288

I have an ArrayList that I want to iterate over. While iterating over it I have to remove elements at the same time. Obviously this throws a java.util.ConcurrentModificationException.

What is the best practice to handle this problem? Should I clone the list first?

I remove the elements not in the loop itself but another part of the code.

My code looks like this:

public class Test() {
    private ArrayList<A> abc = new ArrayList<A>();

    public void doStuff() {
        for (A a : abc) 
        a.doSomething();
    }

    public void removeA(A a) {
        abc.remove(a);
    }
}

a.doSomething might call Test.removeA();

Belphegor
  • 4,456
  • 11
  • 34
  • 59

25 Answers25

426

Two options:

  • Create a list of values you wish to remove, adding to that list within the loop, then call originalList.removeAll(valuesToRemove) at the end
  • Use the remove() method on the iterator itself. Note that this means you can't use the enhanced for loop.

As an example of the second option, removing any strings with a length greater than 5 from a list:

List<String> list = new ArrayList<String>();
...
for (Iterator<String> iterator = list.iterator(); iterator.hasNext(); ) {
    String value = iterator.next();
    if (value.length() > 5) {
        iterator.remove();
    }
}
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 3
    I should have mentioned that i remove the elements in another part of the code and not the loop itself. – RoflcoptrException Nov 12 '11 at 13:28
  • @Roflcoptr: Well it's hard to answer without seeing how the two bits of code interact. Basically, you can't do that. It's not obvious whether cloning the list first would help, without seeing how it all hangs together. Can you give more details in your question? – Jon Skeet Nov 12 '11 at 13:30
  • I know that cloning the list would help, but I don't know if it is a good approach. But I'll add some more code. – RoflcoptrException Nov 12 '11 at 13:32
  • @Roflcoptr Are you using more than one thread? If you are looping while removing in code called in the loop, you need to loop over a copy of the list, or use a decreasing index, or use a list which does get CME. – Peter Lawrey Nov 12 '11 at 13:32
  • @Roflcoptr: So could you just pass the iterator down instead? Why does `removeA` need to remove it *directly* from the collection? Does `removeA` even need to *know* about the rest of the collection? Could it actually just return a `boolean` to say whether or not it *should* be removed? Basically, the design of your code is encouraging problems here - calling arbitrary code which you don't control while iterating over a collection which can also be modified by other code is fundamentally problematic. – Jon Skeet Nov 12 '11 at 13:39
  • Cloning the list *may* be the best bet... but presumably `doSomething` could *also* add something to the list - how would you want that to be handled? Perhaps `doSomething` should be indicating that it wants something removed instead? – Jon Skeet Nov 12 '11 at 13:40
  • @JonSkeet doSomething() does not add something to the list, just possible remove it. I think cloning is a simple solution, but unfortunately i have a ugly cast then. Or is use a for loops that decrements through the list. – RoflcoptrException Nov 12 '11 at 13:44
  • @Roflcoptr: But what if `doSomething()` decided to remove a *different* value from the list? There's a problem of encapsulation here, fundamentally. What's `doSomething` meant to do? Rather than it being able to remove items directly, could it decide whether or not the value *should* be removed, and let the iterating code do the removal? – Jon Skeet Nov 12 '11 at 13:58
  • @JonSkeet What do you mean by a different value? And why should it be a problem when using clone? – RoflcoptrException Nov 12 '11 at 14:01
  • @JonSkeet: Ok I think I've solved the problem. If I just use a for (int i ....) loop then I don't have this problem.. – RoflcoptrException Nov 12 '11 at 14:42
  • @JonSkeet small point: `originalList.removeAll()` has method signature `Collection` (rather than more specifically, `List`). So `valuesToRemove` could be, for example, a `List` or a `Set`. – vikingsteve Jul 16 '15 at 12:51
  • 2
    This solution also leads to java.util.ConcurrentModificationException, see http://stackoverflow.com/a/18448699/2914140. – CoolMind May 31 '16 at 06:41
  • @CoolMind: Um, no it doesn't. The answer you've linked to is equivalent to mine - what difference do you think there is? – Jon Skeet May 31 '16 at 08:40
  • @JonSkeet, you are right, but in my case it raised the exception, but another variant worked. Sorry. – CoolMind May 31 '16 at 09:18
  • 1
    @CoolMind: Without multiple threads, this code should be fine. – Jon Skeet May 31 '16 at 10:00
  • Thanks! My solution was a variation of option 1: cloned the List, iterated through one, changes made in the second, then return the second. – Federico Alvarez Oct 04 '17 at 14:51
  • Cloning the list and make changes on it would work but this is the best option to avoid ConcurrentModificationException ( " Without multiple threads, this code should be fine" ) :-) thanks for share Mr. Skeet. – Jorgesys Jan 31 '19 at 21:29
  • saving life with stack overflow answers since 2011 – pujan kc May 31 '22 at 05:36
31

From the JavaDocs of the ArrayList

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

Varun Achar
  • 14,781
  • 7
  • 57
  • 74
28

You are trying to remove value from list in advanced "for loop", which is not possible, even if you apply any trick (which you did in your code). Better way is to code iterator level as other advised here.

I wonder how people have not suggested traditional for loop approach.

for( int i = 0; i < lStringList.size(); i++ )
{
    String lValue = lStringList.get( i );
    if(lValue.equals("_Not_Required"))
    {
         lStringList.remove(lValue);
         i--; 
    }  
}

This works as well.

suhas0sn07
  • 678
  • 6
  • 7
  • 5
    This is not correct!!! when you remove an element, the next is taking its position and while i increases the next element is not checked in the next iteration. In this case you should go for( int i = lStringList.size(); i>-1; i-- ) – Johntor Dec 05 '16 at 14:30
  • 2
    Agree! Alternate is to perform i--; in if condition within for loop. – suhas0sn07 Mar 30 '17 at 10:08
  • 1
    I think this answer was edited to address the issues in the above comments, so as it is now it works fine, at least for me. – Kira Resari May 20 '20 at 05:58
  • @KiraResari, Yes. I have updated the answer to address the issue. – suhas0sn07 Oct 18 '21 at 04:45
18

In Java 8 you can use the Collection Interface and do this by calling the removeIf method:

yourList.removeIf((A a) -> a.value == 2);

More information can be found here

ggeo
  • 454
  • 6
  • 9
12

You should really just iterate back the array in the traditional way

Every time you remove an element from the list, the elements after will be push forward. As long as you don't change elements other than the iterating one, the following code should work.

public class Test(){
    private ArrayList<A> abc = new ArrayList<A>();

    public void doStuff(){
        for(int i = (abc.size() - 1); i >= 0; i--) 
            abc.get(i).doSomething();
    }

    public void removeA(A a){
        abc.remove(a);
    }
}
Marcus
  • 121
  • 1
  • 3
7

While iterating the list, if you want to remove the element is possible. Let see below my examples,

ArrayList<String>  names = new ArrayList<String>();
        names.add("abc");
        names.add("def");
        names.add("ghi");
        names.add("xyz");

I have the above names of Array list. And i want to remove the "def" name from the above list,

for(String name : names){
    if(name.equals("def")){
        names.remove("def");
    }
}

The above code throws the ConcurrentModificationException exception because you are modifying the list while iterating.

So, to remove the "def" name from Arraylist by doing this way,

Iterator<String> itr = names.iterator();            
while(itr.hasNext()){
    String name = itr.next();
    if(name.equals("def")){
        itr.remove();
    }
}

The above code, through iterator we can remove the "def" name from the Arraylist and try to print the array, you would be see the below output.

Output : [abc, ghi, xyz]

Indra K
  • 114
  • 1
  • 2
  • Else, we can use concurrent list which is available in concurrent package, so that you can perform remove and add operations while iterating. For example see the below code snippet. ArrayList names = new ArrayList(); CopyOnWriteArrayList copyNames = new CopyOnWriteArrayList(names); for(String name : copyNames){ if(name.equals("def")){ copyNames.remove("def"); } } – Indra K Feb 01 '18 at 08:43
  • CopyOnWriteArrayList gonna be costliest operations. – Indra K Feb 01 '18 at 08:46
5

Here is an example where I use a different list to add the objects for removal, then afterwards I use stream.foreach to remove elements from original list :

private ObservableList<CustomerTableEntry> customersTableViewItems = FXCollections.observableArrayList();
...
private void removeOutdatedRowsElementsFromCustomerView()
{
    ObjectProperty<TimeStamp> currentTimestamp = new SimpleObjectProperty<>(TimeStamp.getCurrentTime());
    long diff;
    long diffSeconds;
    List<Object> objectsToRemove = new ArrayList<>();
    for(CustomerTableEntry item: customersTableViewItems) {
        diff = currentTimestamp.getValue().getTime() - item.timestamp.getValue().getTime();
        diffSeconds = diff / 1000 % 60;
        if(diffSeconds > 10) {
            // Element has been idle for too long, meaning no communication, hence remove it
            System.out.printf("- Idle element [%s] - will be removed\n", item.getUserName());
            objectsToRemove.add(item);
        }
    }
    objectsToRemove.stream().forEach(o -> customersTableViewItems.remove(o));
}
serup
  • 3,676
  • 2
  • 30
  • 34
  • I think that you're doing extra work executing two loops, in worst case the loops would be of the entire list. Would be simplest and less expensive doing it in only one loop. – Luis Carlos Nov 09 '16 at 14:08
  • I do not think you can remove object from within first loop, hence the need for extra removal loop, also removal loop is only objects for removal - perhaps you could write an example with only one loop, I would like to see it - thanks @LuisCarlos – serup Nov 11 '16 at 08:45
  • As you say with this code you can't remove any element inside the for-loop because it causes the java.util.ConcurrentModificationException exception. However you could use a basic for. Here I write an example using part of your code. – Luis Carlos Nov 11 '16 at 10:03
  • 1
    for(int i = 0; i < customersTableViewItems.size(); i++) { diff = currentTimestamp.getValue().getTime() - customersTableViewItems.get(i).timestamp.getValue().getTime(); diffSeconds = diff / 1000 % 60; if(diffSeconds > 10) { customersTableViewItems.remove(i--); } } Is important i-- because you don't want skip any elment. Also you could use the method removeIf(Predicate super E> filter) provided by ArrayList class. Hope this help – Luis Carlos Nov 11 '16 at 10:12
  • @LuisCarlos, if this is possible, then why the exception when using the other way? perhaps this is not safe, anyway thanks for your example – serup Nov 11 '16 at 10:45
  • 1
    The exception occurs because in for-loop there as an active reference to iterator of the list. In the normal for, there's not a reference and you have more flexibility to change the data. Hope this help – Luis Carlos Nov 11 '16 at 12:03
5

Do the loop in the normal way, the java.util.ConcurrentModificationException is an error related to the elements that are accessed.

So try:

for(int i = 0; i < list.size(); i++){
    lista.get(i).action();
}
Eric Leschinski
  • 146,994
  • 96
  • 417
  • 335
Tacila
  • 75
  • 1
  • 4
  • 1
    You avoided the `java.util.ConcurrentModificationException` by not removing anything from the list. Tricky. :) You can not really call this "the normal way" to iterate a list. – Zsolt Sky Oct 14 '19 at 14:54
5

You can also use CopyOnWriteArrayList instead of an ArrayList. This is the latest recommended approach by from JDK 1.5 onwards.

Apostolos
  • 10,033
  • 5
  • 24
  • 39
Pathikreet
  • 53
  • 1
  • 3
4

Instead of using For each loop, use normal for loop. for example,the below code removes all the element in the array list without giving java.util.ConcurrentModificationException. You can modify the condition in the loop according to your use case.

for(int i=0; i<abc.size(); i++)  {
       e.remove(i);
 }
Shubham Chopra
  • 1,678
  • 17
  • 30
  • I do not think this works. After removing the element at index 0 the second element becomes the first (index 0). In the next iteration we skip this element and remove the one at index 1. Plus, the original question was not how to remove all elements. – Zsolt Sky Aug 09 '22 at 12:49
4

In my case, the accepted answer is not working, It stops Exception but it causes some inconsistency in my List. The following solution is perfectly working for me.

List<String> list = new ArrayList<>();
List<String> itemsToRemove = new ArrayList<>();

for (String value: list) {
   if (value.length() > 5) { // your condition
       itemsToRemove.add(value);
   }
}
list.removeAll(itemsToRemove);

In this code, I have added the items to remove, in another list and then used list.removeAll method to remove all required items.

Asad Ali Choudhry
  • 4,985
  • 4
  • 31
  • 36
4

Sometimes old school is best. Just go for a simple for loop but make sure you start at the end of the list otherwise as you remove items you will get out of sync with your index.

List<String> list = new ArrayList<>();
for (int i = list.size() - 1; i >= 0; i--) {
  if ("removeMe".equals(list.get(i))) {
    list.remove(i);
  }
}
4

One option is to modify the removeA method to this -

public void removeA(A a,Iterator<A> iterator) {
     iterator.remove(a);
     }

But this would mean your doSomething() should be able to pass the iterator to the remove method. Not a very good idea.

Can you do this in two step approach : In the first loop when you iterate over the list , instead of removing the selected elements , mark them as to be deleted. For this , you may simply copy these elements ( shallow copy ) into another List.

Then , once your iteration is done , simply do a removeAll from the first list all elements in the second list.

Bhaskar
  • 7,443
  • 5
  • 39
  • 51
  • Excellent, I used the same approach, although I loop twice. it makes things simple and no concurrent issues with it :) – Pankaj Nimgade Jun 25 '15 at 04:52
  • 2
    I don't see that Iterator has a remove(a) method. The remove() takes no arguments https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html what am I missing ? – c0der Dec 23 '16 at 08:53
  • 1
    @c0der is right. I mean how did this even get voted 5 times... – Kasasira May 24 '21 at 07:43
3

Do somehting simple like this:

for (Object object: (ArrayList<String>) list.clone()) {
    list.remove(object);
}
XMB5
  • 1,384
  • 1
  • 13
  • 17
2

An alternative Java 8 solution using stream:

        theList = theList.stream()
            .filter(element -> !shouldBeRemoved(element))
            .collect(Collectors.toList());

In Java 7 you can use Guava instead:

        theList = FluentIterable.from(theList)
            .filter(new Predicate<String>() {
                @Override
                public boolean apply(String element) {
                    return !shouldBeRemoved(element);
                }
            })
            .toImmutableList();

Note, that the Guava example results in an immutable list which may or may not be what you want.

Zsolt Sky
  • 527
  • 5
  • 6
2
for (A a : new ArrayList<>(abc)) {
    a.doSomething();
    abc.remove(a);
}
0

"Should I clone the list first?"

That will be the easiest solution, remove from the clone, and copy the clone back after removal.

An example from my rummikub game:

SuppressWarnings("unchecked")
public void removeStones() {
  ArrayList<Stone> clone = (ArrayList<Stone>) stones.clone();
  // remove the stones moved to the table
  for (Stone stone : stones) {
      if (stone.isOnTable()) {
         clone.remove(stone);
      }
  }
  stones = (ArrayList<Stone>) clone.clone();
  sortStones();
}
Arjen Rodenhuis
  • 199
  • 1
  • 3
  • 2
    Downvoters should at least leave a comment before downvoting. – OneWorld Jun 18 '15 at 08:51
  • 2
    There is nothing inherently wrong with this answer expect maybe that `stones = (...) clone.clone();` is superfluous. Would not `stones = clone;` do the same? – vikingsteve Jul 16 '15 at 12:49
  • I agree, the second cloning is unnecessary. You can futher simplify this by iterating on the clone and remove elements directly from `stones`. This way you do not even need the `clone` variable: `for (Stone stone : (ArrayList) stones.clone()) {...` – Zsolt Sky Jun 28 '19 at 12:49
0

I arrive late I know but I answer this because I think this solution is simple and elegant:

List<String> listFixed = new ArrayList<String>();
List<String> dynamicList = new ArrayList<String>();

public void fillingList() {
    listFixed.add("Andrea");
    listFixed.add("Susana");
    listFixed.add("Oscar");
    listFixed.add("Valeria");
    listFixed.add("Kathy");
    listFixed.add("Laura");
    listFixed.add("Ana");
    listFixed.add("Becker");
    listFixed.add("Abraham");
    dynamicList.addAll(listFixed);
}

public void updatingListFixed() {
    for (String newList : dynamicList) {
        if (!listFixed.contains(newList)) {
            listFixed.add(newList);
        }
    }

    //this is for add elements if you want eraser also 

    String removeRegister="";
    for (String fixedList : listFixed) {
        if (!dynamicList.contains(fixedList)) {
            removeResgister = fixedList;
        }
    }
    fixedList.remove(removeRegister);
}

All this is for updating from one list to other and you can make all from just one list and in method updating you check both list and can eraser or add elements betwen list. This means both list always it same size

Alex Taylor
  • 8,343
  • 4
  • 25
  • 40
0

Use Iterator instead of Array List

Have a set be converted to iterator with type match

And move to the next element and remove

Iterator<Insured> itr = insuredSet.iterator();
while (itr.hasNext()) { 
    itr.next();
    itr.remove();
}

Moving to the next is important here as it should take the index to remove element.

Dang Nguyen
  • 1,209
  • 1
  • 17
  • 29
nafics
  • 11
  • 5
0

Very important

ConcurrentModificationException can occur during ArrayList<> sorting, if the ArrayList<> sorting method is called at the same time, ie. parallel (asynchronous).

For example from two background Threads, or Thread and UI, but at the same time. This usually happens by accident, if you drop the order of operations somewhere during programming.

Happened to me just now :-)

-1
List<String> list1 = new ArrayList<>();
list1.addAll(OriginalList);

List<String> list2 = new ArrayList<>();
list2.addAll(OriginalList);

This is also an option.

Delmirio Segura
  • 1,651
  • 1
  • 9
  • 5
-2

If your goal is to remove all elements from the list, you can iterate over each item, and then call:

list.clear()
Gibolt
  • 42,564
  • 15
  • 187
  • 127
-2

What about of

import java.util.Collections;

List<A> abc = Collections.synchronizedList(new ArrayList<>());
joseluisbz
  • 1,491
  • 1
  • 36
  • 58
-2

ERROR

There was a mistake when I added to the same list from where I took elements:

fun <T> MutableList<T>.mathList(_fun: (T) -> T): MutableList<T> {
    for (i in this) {
        this.add(_fun(i))   <---   ERROR
    }
    return this   <--- ERROR
}

DECISION

Works great when adding to a new list:

fun <T> MutableList<T>.mathList(_fun: (T) -> T): MutableList<T> {
    val newList = mutableListOf<T>()   <---   DECISION
    for (i in this) {
        newList.add(_fun(i))   <---   DECISION
    }
    return newList   <---   DECISION
}
-4

Just add a break after your ArrayList.remove(A) statement