up vote 3 down vote favorite

I'm trying to make use of the for each loop with the arraylist, but when i use it, it gives me error, but when I use normal for loop, it works perfectly, what could be the problem?

The code is here: 

for (Pair p2 : R) {
            if ((p2.getFirstElm() == p.getSecondElm()) && (p2.getFirstElm() != p2.getSecondElm())) R.add(new Pair (p.getFirstElm(), p2.getSecondElm()));
            else if ((p2.getSecondElm() == p.getFirstElm()) && (p2.getFirstElm() != p2.getSecondElm())) R.add(new Pair (p2.getFirstElm(), p.getSecondElm()));
            //else
            //  There are no transitive pairs in R.
        }

this is the loop that doesnt work, and here is the one that works:

for (int i = 0; i < R.size(); i++) {
            if ((R.get(i).getFirstElm() == p.getSecondElm()) && (R.get(i).getFirstElm() != R.get(i).getSecondElm())) R.add(new Pair (p.getFirstElm(), R.get(i).getSecondElm()));
            else if ((R.get(i).getSecondElm() == p.getFirstElm()) && (R.get(i).getFirstElm() != R.get(i).getSecondElm())) R.add(new Pair (R.get(i).getFirstElm(), p.getSecondElm()));
            //else
            //  There are no transitive pairs in R.
        }

the error im getting when using the for each loop is:

Exception in thread "main" java.util.ConcurrentModificationException at java.util.AbstractList$Itr.checkForComodification(Unknown Source) at java.util.AbstractList$Itr.next(Unknown Source) at set.problem.fourth.PoSet.makeTransitive(PoSet.java:145) at set.problem.fourth.PoSet.addToR(PoSet.java:87) at set.problem.fourth.PoSetDriver.typicalTesting(PoSetDriver.java:35) at set.problem.fourth.PoSetDriver.main(PoSetDriver.java:13)

share|improve this question
79% accept rate
possible duplicate of Concurrent Modification Exception : adding to an ArrayList – Steve Kuo Mar 21 '12 at 15:02
feedback

3 Answers

up vote 5 down vote accepted

Java Collection classes are fail-fast which means that if the Collection will be changed while some thread is traversing over it using iterator, the iterator.next() will throw a ConcurrentModificationException.

This situation can come in case of multithreaded as well as single threaded environment. - www.javacodegeeks.com

You can't modify a List in a for/each loop, which is syntactic sugar around the Iterator as an implementation detail. Using the Iterator directly won't do you any good either, you can only safely call .remove() when using the Iterator directly as well.

Note that Iterator.remove is the only safe way to modify a collection during iteration; the behavior is unspecified if the underlying collection is modified in any other way while the iteration is in progress. - Java Collections Tutorial

Calling .add() inside the for/each loop modifies the contents, and the Iterator that is used behind the scenes sees this and throws this exception.

A more subtle concern is the that the second way you list, the .size() is increasing every time you .add() so you will end up processing all the things you .add(), this could possibly cause an endless loop depending on what the input data is. I am not sure if this is what you desire.

Solution

I would create another ArrayList and .add() to it all the new things, and then after the loop, use .addAll() on the original ArrayList to combine the two lists together. This will make things explicit in what you are trying to do, that is unless your intention is process all the newly added things as you add them.

share|improve this answer
feedback
up vote 0 down vote

Under the hood, the for-each loop in Java uses an Iterator for traversing the collection (see this article for a detailed explanation.) And the Iterator will throw a ConcurrentModificationException if you modify the collection while iterating over it, see this post.

share|improve this answer
feedback
up vote 0 down vote

The problem is that you're doing the R.add() in the first line of the loop.

In the first situation you have an iterator open to the arraylist. When you do an add and then try to iterate again the iterator notices that the data structure has changed underneath you.

In the case of the for look you're just getting a new element each time and don't have the concurrent modification issue, though the size is changing as you add more elements.

To fix the problem you probably want to add to a temporary location and add that after the loop or make a copy of the initial data and add to the original.

share|improve this answer
feedback

Your Answer

 
or
required, but never shown
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.