How to avoid "ConcurrentModificationException" while removing elements from `ArrayList` while iterating it? [duplicate]
Asked Answered
L

10

400

I'm trying to remove some elements from an ArrayList while iterating it like this:

for (String str : myArrayList) {
    if (someCondition) {
        myArrayList.remove(str);
    }
}

Of course, I get a ConcurrentModificationException when trying to remove items from the list at the same time when iterating myArrayList. Is there some simple solution to solve this problem?

Lelialelith answered 26/8, 2013 at 16:27 Comment(1)
I used cloned object to resolve this issue.Rana
M
635

Use an Iterator and call remove():

Iterator<String> iter = myArrayList.iterator();

while (iter.hasNext()) {
    String str = iter.next();

    if (someCondition)
        iter.remove();
}
Mendoza answered 26/8, 2013 at 16:29 Comment(12)
Thanks, now everything works fine :) I think this answer is the best, because the code is easily readable.Lelialelith
@ErnestasGruodis The tradeoff is that iter is now in scope for the rest of the method.Deplane
What if I wanted to remove something other than the current iteration (say it's on index 2, but I need to remove index 7 at the same time). It gives me a ConcurrentModificationException whenever I try through .remove(index).Leenaleeper
Funny, I got same exception on String str = iter.next(); ! Java with collections sucks !Caloric
iterator initializer may be put in the first section of a c-style for-loop expression.Salutary
What about this, Collections.unmodifiableListBeautician
Got the same exception using this approach as well.Objectify
@AakashPatel Then it's a different problem.Mendoza
For me not work. I use this .ArrayList<OnMessage> f = (ArrayList<OnMessage>) list.clone(); f.remove(onMessage); list = f;Ladyship
@Caloric do a type casting to that statement it will work fine. When ever converting higher to lower type always use typecasting i.e; String str = (String) iter.next();Hardshell
@Caloric (and other): the problem is not Java collections (which are great) but your multi-threaded program where a thread modifies the list that is being iterated in another thread.Acidulent
Iterators are not always helpful when another thread modifies the collection. I had tried many ways but then I realized traversing the collection manually is much safer (backward for removal)Tailpiece
M
224

As an alternative to everyone else's answers I've always done something like this:

List<String> toRemove = new ArrayList<String>();
for (String str : myArrayList) {
    if (someCondition) {
        toRemove.add(str);
    }
}
myArrayList.removeAll(toRemove);

This will avoid you having to deal with the iterator directly, but requires another list. I've always preferred this route for whatever reason.

Misgiving answered 26/8, 2013 at 16:35 Comment(10)
+1 I like this iteratorless solution.Periapt
@KevinDiTraglia Is there some reason to use more resources than you need? It's not like iterators are that hard to work with or make the code messy.Deplane
@EricStein I usually end up in situations where I want to add to the list as well, and the extra resources are mostly trivial. It's just an alternative solution, both have their pros and cons.Misgiving
@KevinDiTraglia I agree that the resources are usually negligible.Deplane
@EricStein If we go an extra step and use immutable lists (like those in the Guava library) then this becomes much more attractive when dealing with multithreaded concurrency issues.Gismo
I'm really sorry I accidentally downvoted this without noticing and now I can't change it.Flashing
This is the slowest possible way.Lecythus
@KevinDiTraglia really appreciate the way you thinkingCuevas
What is wrong with using an Iterator? That said, I like this approach because it does not manipulate state, but creates a new one. Feels kind a immutable ;-)Broadbrim
@KevinDiTraglia any idea about the performance of this method against a method which using iterator? BTW this is coolest solutionCuevas
W
107

Java 8 user can do that: list.removeIf(...)

    List<String> list = new ArrayList<>(Arrays.asList("a", "b", "c"));
    list.removeIf(e -> (someCondition));

It will remove elements in the list, for which someCondition is satisfied

Wiggle answered 22/4, 2014 at 17:0 Comment(5)
Yeah, this is way better if you can use Java 8.Byran
If would be nice if they had also added removeWhileGarage
@damluar I don't understand why the removeWhile, removeIf will remove all the elements matching the condition.Selfexcited
But what if you want to remove only first element until/while a condition is satisfied?Garage
They are adding something like removeWhile in JDK 9.Wiggle
D
70

You have to use the iterator's remove() method, which means no enhanced for loop:

for (final Iterator iterator = myArrayList.iterator(); iterator.hasNext(); ) {
    iterator.next();
    if (someCondition) {
        iterator.remove();
    }
}
Deplane answered 26/8, 2013 at 16:29 Comment(4)
I feel this answer communicates better; the iterator is confined to the for-loop, and the details of the iteration are in the for statement. Less visual noise.Gismo
If you add type parameters to Iterator and assign that iterator.next() to some variable so you can actually do something with it in the if this is the best solutionInflexible
Why you declare the iterator as final ?Dereism
@Dereism I consider it a good habit to declare as final all variables that are not intended to be reassigned. I only wish "final" were the default.Deplane
F
42

No, no, NO!

In single threated tasks you don't need to use Iterator, moreover, CopyOnWriteArrayList (due to performance hit).

Solution is much simpler: try to use canonical for loop instead of for-each loop.

According to Java copyright owners (some years ago Sun, now Oracle) for-each loop guide, it uses iterator to walk through collection and just hides it to make code looks better. But, unfortunately as we can see, it produced more problems than profits, otherwise this topic would not arise.

For example, this code will lead to java.util.ConcurrentModificationException when entering next iteration on modified ArrayList:

        // process collection
        for (SomeClass currElement: testList) {

            SomeClass founDuplicate = findDuplicates(currElement);
            if (founDuplicate != null) {
                uniqueTestList.add(founDuplicate);
                testList.remove(testList.indexOf(currElement));
            }
        }

But following code works just fine:

    // process collection
    for (int i = 0; i < testList.size(); i++) {
        SomeClass currElement = testList.get(i);

        SomeClass founDuplicate = findDuplicates(currElement);
        if (founDuplicate != null) {
            uniqueTestList.add(founDuplicate);
            testList.remove(testList.indexOf(currElement));
            i--; //to avoid skipping of shifted element
        }
    }

So, try to use indexing approach for iterating over collections and avoid for-each loop, as they are not equivalent! For-each loop uses some internal iterators, which check collection modification and throw ConcurrentModificationException exception. To confirm this, take a closer look at the printed stack trace when using first example that I've posted:

Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
    at java.util.AbstractList$Itr.next(AbstractList.java:343)
    at TestFail.main(TestFail.java:43)

For multithreading use corresponding multitask approaches (like synchronized keyword).

Fairground answered 14/11, 2014 at 14:41 Comment(6)
Worth noting that given the way a LinkedList works internally, an Iterator is far more efficient than subsequent get(i) calls with an incremented i.Becker
Great comment and details, thanksMaravedi
Agreed with Angad. One often only has access to the generic List type, and the implementation that was used is then unknown. If the implementation used is LinkedList, using a C style for loop to loop through the List retrieving each one will result in O(n2) complexity.Morris
You can avoid the i--; //to avoid skipping of shifted element by looping downwards: for (int i = testList.size()-1; i >= 0; i--) { ... } Also, instead of testList.remove(testList.indexOf(currElement)); you can simply write testList.remove(i);Qualm
@Becker But using of iterator causes exception mentioned, due to it relies on previous-current-next relation, which is broken in case of removing an element from the collection. Here we should pay by performance hit.Fairground
@MartinRust Yes, thank you for comments!Fairground
M
9

While other suggested solutions work, If you really want the solution to be made thread safe you should replace ArrayList with CopyOnWriteArrayList

    //List<String> s = new ArrayList<>(); //Will throw exception
    List<String> s = new CopyOnWriteArrayList<>();
    s.add("B");
    Iterator<String> it = s.iterator();
    s.add("A");

    //Below removes only "B" from List
    while (it.hasNext()) {
        s.remove(it.next());
    }
    System.out.println(s);
Martyrdom answered 26/8, 2013 at 16:55 Comment(1)
Yes, but Java documentation says that "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."Lelialelith
A
8

If you want to modify your List during traversal, then you need to use the Iterator. And then you can use iterator.remove() to remove the elements during traversal.

Amanuensis answered 26/8, 2013 at 16:28 Comment(0)
B
7
List myArrayList  = Collections.synchronizedList(new ArrayList());

//add your elements  
 myArrayList.add();
 myArrayList.add();
 myArrayList.add();

synchronized(myArrayList) {
    Iterator i = myArrayList.iterator(); 
     while (i.hasNext()){
         Object  object = i.next();
     }
 }
Bonitabonito answered 26/8, 2013 at 16:48 Comment(1)
In this answer where are you removing the item(s) from list? OP asked How to avoid “ConcurrentModificationException” while removing elements. I could not see any reason why others up-voted this answer.Begorra
M
7

One alternative method is convert your List to array, iterate them and remove them directly from the List based on your logic.

List<String> myList = new ArrayList<String>(); // You can use either list or set

myList.add("abc");
myList.add("abcd");
myList.add("abcde");
myList.add("abcdef");
myList.add("abcdefg");

Object[] obj = myList.toArray();

for(Object o:obj)  {
    if(condition)
        myList.remove(o.toString());
}
Marnamarne answered 18/10, 2013 at 10:47 Comment(3)
Why is there a object.toString() while removing? shouldn't it just be 'o'?Gillie
@TheMorfeus It can be just 'o'. But I used toString() method to avoid 'Suspicious method call' bug from IDE. No other specific reasons.Marnamarne
This solution is suitable only for small size of list. Just imagine a list containing thousands of items, converting to array will be very costly.Begorra
F
2

You can use the iterator remove() function to remove the object from underlying collection object. But in this case you can remove the same object and not any other object from the list.

from here

Febrile answered 16/4, 2014 at 13:23 Comment(2)
Links belong in the comment section unless they are supporting your post. You should edit your answer to include an explanation and then just put the link as a reference.Bowse
This one actually works and solves the problem! Thank you!Heinrick

© 2022 - 2024 — McMap. All rights reserved.