0

Why people recommend use Iterator if we get ConcurrentModificationException with forEach ? In my case the best solution for this exception is just use fori instead of foreach.
So I have right ? Please explain to me if I'm wrong

example:

with exception

   private void init() {
        addToCollection("Dog");
        addToCollection("Cat");
        showCollection(stringArrayList);
    }

    private void showCollection(List<String> collection) {

        for (String s : collection) {
            if (s.equals("Dog")){
                collection.remove(s);
            }
        }

    }

    private void addToCollection(String value) {
        stringArrayList.add(value);
    }
}

without:

private void init() {
    addToCollection("Dog");
    addToCollection("Cat");
    showCollection(stringArrayList);
}

private void showCollection(List<String> collection) {

    for (int i = 0; i < collection.size(); i++) {
        if (collection.get(i).equals("Dog")){
            collection.remove(i);
        }
    }

}

private void addToCollection(String value) {
    stringArrayList.add(value);
}
androidmanifest
  • 289
  • 1
  • 4
  • 9
  • 4
    When you use an iterator, it is spotting your mistake; and when you use indexes, it is unable to spot your mistake. If you remove the element at `i`, and then increment `i` by one, you are skipping over an element. Things like this are the reason concurrent modification causes problems. – khelwood Jun 29 '18 at 13:14
  • There is an optional `remove` method on iterator (https://docs.oracle.com/javase/7/docs/api/java/util/Iterator.html#remove()). It could help in your case, if the iterator supports it. Also you would have to use the iterator manually. – Korrat Jun 29 '18 at 13:18
  • `for(Iterator iter = collection.iterator(); iterator.hasNext();) if(iter.next().equals("Dog")) iter.remove();` – Lino Jun 29 '18 at 13:20
  • Not every collection allows index access or doesn't guarantee doing it efficiently. Also mutating a collection while iterating it is a code smell and leads to hard to spot errors - like the one @khelwood pointed out. – jannis Jun 29 '18 at 13:26
  • Possible duplicate of [Remove elements from collection while iterating](https://stackoverflow.com/questions/10431981/remove-elements-from-collection-while-iterating) – vincrichaud Jun 29 '18 at 13:43

3 Answers3

2

There is"rule" when you are developping : "Never modify something you are looping through". If you loop through your collection you should not modify it inside your loop ! If you want to modify your collection inside the loop, find a way to loop through something else (like a copy of the collection or an iterator).

The reason behind respecting this rule is to avoid bug. You could create some hacks/tricks to allow you to modify the list you are looping through. But forcing you to not do so, avoid the time you will miss your hack and create a bug.


In your case, when you do a for-each loop, the system is able to see that you don't respect the above rule. So it throws you an Exception.

When you do a for loop with an index. The system is unable to detect that you modify your collection in the loop. But you will still have a problem, because you will skip some value since you remove item in the collection but does not modify the index accordingly. Here you could write a "hack" by modifying the index every time you remove an item, But I strongly recommend to not do so. Just to respect the above rule if you loop with an index do not modify it in the loop.

The only good way to do so is to use an Iterator. This object is designed to allow modification during a loop.

Iterator it = collection.iterator();
while (it.hasNext()){
    String value= it.next();
    if (value.equals("Dog")){
       it.remove();
    }
}
vincrichaud
  • 2,218
  • 17
  • 34
1

For i

As you iterate with the help of the index, you do not get a ConcurrentModificationException as this is not detectable in this case. But what happens is that the loop will skip one object, ever time you remove an object as the index gets adjusted when you remove but i does not. If you would decrease i after removing an object it would work as expected.

for (int i = 0; i < collection.size(); i++) {
    if (collection.get(i).equals("Dog")){
        collection.remove(i);
        i--;
    }
}

Use an iterator

The Iterator has a nice solution for this problem. It allows you to remove an object from the collection you are iterating.

remove() Removes from the underlying collection the last element returned by this iterator (optional operation).

Iterator it = collection.iterator();
while (it.hasNext()){
    String value= it.next();
    if (value.equals("Dog")){
       it.remove();
    }
}
Herr Derb
  • 4,977
  • 5
  • 34
  • 62
1
// BROKEN - throws NoSuchElementException!
for (Iterator i = suits.iterator(); i.hasNext(); )
    for (Iterator j = ranks.iterator(); j.hasNext(); )
        sortedDeck.add(new Card(i.next(), j.next()));

Following the Example. Here is broken because you are calling too many times the next iterator.

Versus the ForEach

for (Suit suit : suits)
    for (Rank rank : ranks)
        sortedDeck.add(new Card(suit, rank));

Here will not be broken. And adding a Point for ForEach

As you can see, the for-each construct combines beautifully with generics. It preserves all of the type safety, while removing the remaining clutter. Because you don't have to declare the iterator, you don't have to provide a generic declaration for it.

And as a personal Opinion. It depends when to use for and foreach. Everything is relative. But ForEach saves you a lot of time of debugging when this bugs happens.

If you want to modify a Collection inside a Loop use CopyOnWriteArrayList<E>

A thread-safe variant of ArrayList in which all mutative operations (add, set, and so on) are implemented by making a fresh copy of the underlying array.

This is ordinarily too costly, but may be more efficient than alternatives when traversal operations vastly outnumber mutations, and is useful when you cannot or don't want to synchronize traversals, yet need to preclude interference among concurrent threads.

From Java Docs.

Gatusko
  • 2,503
  • 1
  • 17
  • 25