CODEWITHSUNDEEP
javalistiteratorconcurrentmodification
César Castro Aroche
César Castro Aroche

Reputation: 775

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;
   }

Upvotes: 0

Views: 612

Answers (3)

Ioan M
Ioan M

Reputation: 1207

I would also improve the algorithm and instead of checking for one element all of them all the time try to play a bit with the indexes and base the second loop index(j) on the index of the first one(i). Don't do any removals and use a set in case you think you may have duplicates in the list as suggested already here.

for (int i = 0; i < documentsToIterate.size() - 1; i++) {
    for (int j = i + 1; j < documentsToIterate.size(); j++) {
        if (related(doc[i],doc[j]);
           addPair(..);
    }
}

Upvotes: 1

Sweeper
Sweeper

Reputation: 271555

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.

Upvotes: 1

Tobias Otto
Tobias Otto

Reputation: 1676

Maybe your problem could easily be solved when switching from pairDocList to pairDocSet.

You do not need to remove any element from the list, when you make a Set of PairDocuments. It would be OK to add the same PairDocument twice or more to the Set, because there are no duplicates in a Set. You have to make some effort to identify same PairDocuments with correct equals() and hashCode(), but it is worth.

Upvotes: 0

Related Questions