390

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?

ʇolɐǝz ǝɥʇ qoq
  • 669
  • 1
  • 14
  • 30
Ernestas Gruodis
  • 8,069
  • 14
  • 48
  • 111

10 Answers10

625

Use an Iterator and call remove():

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

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

    if (someCondition)
        iter.remove();
}
arshajii
  • 123,543
  • 24
  • 232
  • 276
217

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.

Eugene
  • 9,617
  • 4
  • 42
  • 62
Kevin DiTraglia
  • 25,012
  • 19
  • 91
  • 135
  • 27
    +1 I like this iteratorless solution. – Terry Li Aug 26 '13 at 16:39
  • 5
    @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. – Eric Stein Aug 26 '13 at 16:45
  • 2
    @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. – Kevin DiTraglia Aug 26 '13 at 16:47
  • @KevinDiTraglia I agree that the resources are usually negligible. – Eric Stein Aug 26 '13 at 16:52
  • 4
    @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. – Nobbynob Littlun Aug 19 '14 at 18:34
  • I'm really sorry I accidentally downvoted this without noticing and now I can't change it. – Script Kitty Mar 22 '17 at 13:10
  • This is the slowest possible way. – Faraz Jun 11 '17 at 07:50
  • @KevinDiTraglia really appreciate the way you thinking – Nibras Oct 23 '18 at 06:24
  • 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 ;-) – Christian May 28 '19 at 15:43
  • @KevinDiTraglia any idea about the performance of this method against a method which using iterator? BTW this is coolest solution – Nibras Sep 17 '19 at 11:09
103

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

Mikhail Boyarsky
  • 2,771
  • 3
  • 19
  • 33
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();
    }
}
sa2000
  • 33
  • 7
Eric Stein
  • 12,431
  • 2
  • 35
  • 51
  • 10
    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. – Nobbynob Littlun Aug 19 '14 at 18:26
  • 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 solution – sscarduzio Apr 15 '17 at 10:16
  • Why you declare the iterator as final ? – kh.tab Apr 19 '17 at 08:55
  • 1
    @kh.tab 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. – Eric Stein Apr 19 '17 at 14:27
41

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).

Dima Naychuk
  • 443
  • 4
  • 4
  • 19
    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`. – Angad Aug 17 '15 at 11:12
  • Great comment and details, thanks – OlivierM Feb 20 '16 at 00:45
  • 1
    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. – mdewit Mar 30 '16 at 19:29
  • 7
    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);` – Martin Rust Nov 15 '16 at 14:59
  • @Angad 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. – Dima Naychuk May 08 '17 at 14:54
  • @MartinRust Yes, thank you for comments! – Dima Naychuk May 08 '17 at 14:58
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);
Prashant Bhate
  • 10,568
  • 7
  • 44
  • 81
  • 2
    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." – Ernestas Gruodis Aug 26 '13 at 17:05
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.

Juned Ahsan
  • 66,028
  • 11
  • 91
  • 129
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();
     }
 }
Prabhakaran Ramaswamy
  • 24,936
  • 10
  • 54
  • 63
  • 2
    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. – Shailesh Saxena Oct 30 '18 at 12:40
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());
}
Allan Pereira
  • 2,561
  • 4
  • 19
  • 27
CarlJohn
  • 727
  • 2
  • 9
  • 20
  • 1
    Why is there a object.toString() while removing? shouldn't it just be 'o'? – themorfeus Feb 11 '14 at 20:02
  • @TheMorfeus It can be just 'o'. But I used toString() method to avoid 'Suspicious method call' bug from IDE. No other specific reasons. – CarlJohn Feb 12 '14 at 09:27
  • 2
    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. – Shailesh Saxena Oct 30 '18 at 12:43
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

Gilo
  • 503
  • 2
  • 20