-2

I have following code:

public static void main(String[] args) {
 
    List<String> input = new ArrayList<>();
    List<String> output = new ArrayList<>();
    for(int i=0; i< 1000 ;i++){
        input.add(i+"");
    }
    
    
    for(int i=0 ; i<input.size(); i++){
        String value = input.get(i);
        if(Integer.parseInt(value) % 2 == 0){
            output.add(value);
            input.remove(value);
        }
    }
    
    input.stream().forEach(System.out::println);
    System.out.println("--------------------------------------");
    output.stream().forEach(System.out::println);

}

I expected it to throw ConcurrentModificationException but it is working fine. Can some explain the reason?

Pshemo
  • 122,468
  • 25
  • 185
  • 269
Kumar
  • 1,536
  • 2
  • 23
  • 33
  • 2
    "*I expected it to throw `ConcurrentModificationException`*" why? You are not using Iterator explicitly nor implicitly via enhanced for-loop *while modifying* your list. – Pshemo Jun 04 '21 at 13:51
  • 1
    You would only expect that exception if you modified the collection whilst traversing it using an iterator or an enhanced for loop. Your code does neither. – stridecolossus Jun 04 '21 at 13:53
  • @stridecolossus: Then why this is not prefered way to loop when we have to modify the list? – Kumar Jun 04 '21 at 13:55
  • 1
    @Abhinav you should take a look a this article https://www.baeldung.com/java-concurrentmodificationexception – Pilpo Jun 04 '21 at 13:55
  • 1
    @Abhinav I'm not sure there is a *preferred* way to loop when modifying the list being traversed. Generally I would avoid the need to mutate the list at all, e.g. by creating 2 results (for your example) or using streams. Concurrent mod then wouldn't be an issue, and the code would almost certainly be simpler than a loop with random access. – stridecolossus Jun 04 '21 at 14:01

1 Answers1

5

The reason is you are not technically iterating the List. Instead you are random accessing the list using a incrementing index, and removing some values. If you change to code like this to iterate the list it will throw ConcurrentModificationException

public static void main(String[] args) {
    List<String> input = new ArrayList<>();
    List<String> output = new ArrayList<>();
    for(int i=0; i< 1000 ;i++){
        input.add(i+"");
    }
    
    for (String value : input) {
        if(Integer.parseInt(value) % 2 == 0){
            output.add(value);
            input.remove(value);
        }
    }

    input.stream().forEach(System.out::println);
    System.out.println("--------------------------------------");
    output.stream().forEach(System.out::println);
}

A follow up on why this might not be a preferred way compared to an iterator. One reason is performance. Here is some benchmark code using JMH to test this out.

package bench;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Level;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.TimeUnit;

import static java.util.concurrent.TimeUnit.SECONDS;

@State(Scope.Benchmark)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Warmup(iterations = 1, time = 3, timeUnit = SECONDS)
@Measurement(iterations = 3, time = 2, timeUnit = SECONDS)
public class JmhBenchmark {
    private List<String> input;

    @Param({"100", "1000", "10000"})
    public int length;

    @Setup(Level.Invocation)
    public void createInputList() {
        input = new ArrayList<>();
        for (int i = 0; i < length; i++) {
            input.add(i + "");
        }
    }

    @Benchmark
    public void iterateWithVariable() {
        for (int i = 0; i < input.size(); i++) {
            String value = input.get(i);
            if (Integer.parseInt(value) % 2 == 0) {
                input.remove(value);
            }
        }
    }

    @Benchmark
    public void iterateWithIterator() {
        final Iterator<String> iterator = input.iterator();
        while (iterator.hasNext()) {
            String value = iterator.next();
            if (Integer.parseInt(value) % 2 == 0) {
                iterator.remove();
            }
        }
    }

}

The results of the benchmark on my system were

Benchmark                         (length)  Mode  Cnt   Score    Error  Units
JmhBenchmark.iterateWithIterator       100  avgt   15   0.002 ±  0.001  ms/op
JmhBenchmark.iterateWithIterator      1000  avgt   15   0.033 ±  0.001  ms/op
JmhBenchmark.iterateWithIterator     10000  avgt   15   1.670 ±  0.017  ms/op
JmhBenchmark.iterateWithVariable       100  avgt   15   0.005 ±  0.001  ms/op
JmhBenchmark.iterateWithVariable      1000  avgt   15   0.350 ±  0.014  ms/op
JmhBenchmark.iterateWithVariable     10000  avgt   15  33.591 ±  0.455  ms/op

So we can see using an iterator to remove some items from a list is a lot (>20x) faster than the approach posed by this question. Which makes sense you need to perform a random lookup in the list then determine if it needs to be removed and then do another lookup to find and remove it.

James Mudd
  • 1,816
  • 1
  • 20
  • 25
  • @Kumar I have updated the answer to include a "why this is not prefered way to loop when we have to modify the list?" – James Mudd Jun 04 '21 at 16:54