Search code examples
javalistiteratorconcurrentmodification

Java - Remove from single list over nested loop avoiding concurrent modification exception


So I have this method that is supposed to find pairs inside a collection, for this purpose I use a nested loop. However I always get concurrent modification exception even though I'm using an iterator. I guess as both iterators iterate over the same collection, they are both trying to modify it at the same time and this is why I get this exception. Can you please help me avoid this error by accomplishing the same results.

private List<Pair<Document, Document>> createPairDocument(List<Document> documentsToIterate){
       List<Pair<Document, Document>> pairDocList = new ArrayList<>();
       //iterators are used to avoid concurrent modif exception
       Iterator<Document> iterator0 = documents.iterator();
       while(iterator0.hasNext()){
           Document dl0 = iterator0.next();
           Iterator<Document> iterator1 = documents.iterator(); //returns new instance of iterator
           while(iterator1.hasNext()){
               Document dl1 = iterator1.next();
               if (dl1.relatedTo(dl0) && dl0.relatedTo(dl1)){
                   pairDocList.add(Pair.of(dl0, dl1));
                   //these docs should be removed to avoid creating the same relation again
                   iterator0.remove();
                   iterator1.remove();
                   break;
               }
           }
       }
       return pairDocList;
   }

Solution

  • ConcurrentModificationException occurs because when an iterator is iterating through a collection, it has no idea that the collection is modified, so when the collection is actually modified, the iterator becomes very confused (has an invalid state). By using the Iterator.remove method, you are letting the iterator know that you are removing elements, so that the iterator can adjust its state accordingly.

    In this particular case however, the exception occurs because iterator1 isn't told about the removal that iterator0 just did, in the line iterator0.remove();. When iterator1 tries to remove its element, it finds that its list has changed.

    It's not a good idea to use two iterators that iterates over the same list. I think you can use a regular for loop to loop over the list's indices, and each time getting a list iterator from that index + 1, since a document can't be related to itself.

    for (int i = 0 ; i < documentsToIterate.size() ; i++) {
        var iteratorFromI = documentsToIterate.listIterator(i + 1);
        var dl0 = documentsToIterate.get(i);
        while (iteratorFromI.hasNext()) {
            var dl1 = iteratorFromI.next();
            if (dl1.relatedTo(dl0) && dl0.relatedTo(dl1)){
                pairDocList.add(Pair.of(dl0, dl1));
                iteratorFromI.remove();
                documentsToIterate.remove(i);
                i--; // so that the next one doesn't get skipped
                break;
            }
        }
    }
    

    Now we don't have a concurrent modification exception because we are doing documentsToIterate.remove(i); after iteratorFromI.remove(), and we are throwing the iterator away after that, so it never knows that we modified the list :)

    Alternatively, just use 2 regular for loops.