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

Question

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

No. Only to others. But I can have a document A first in the list and B after in the list. I first create relation A to B, but when I find B I create again relation B - A, which is essentially the same relation. I tried only removing B instead of both, but I get the same error. This could be solved maybe by replacing the document with null value in its position and verifying that is not null in each iteration but is not very straightforward. — César Castro Aroche
– César Castro Aroche, Commented Feb 23, 2021 at 9:16

Sweeper · Accepted Answer · 2021-02-23 09:30:36Z

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.

Ioan M · Accepted Answer · 2021-02-23 09:55:39Z

1

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(..);
    }
}

answered Feb 23, 2021 at 9:55

Ioan M

1,2178 silver badges17 bronze badges

2 Comments

Tobias Otto Over a year ago

Prefer for-each loops to traditional for loops

Ioan M Over a year ago

Here we need more control over the indices so I prefer doing less iterations

Tobias Otto · Accepted Answer · 2021-02-23 09:22:09Z

0

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.

answered Feb 23, 2021 at 9:22

Tobias Otto

1,68614 silver badges21 bronze badges

1 Comment

César Castro Aroche Over a year ago

Yeah this is a good solution but then I will have to add another condition and I will still have to iterate over the whole collection again even though all pairs have already been found.

Collectives™ on Stack Overflow

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

3 Answers 3

Comments

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related