7

Is it safe to remove multiple items from an ArrayList while iterating through it with an iterator?

Iterator<String> iterator = nameList.iterator();
 while(iterator.hasNext()){
     String s = iterator.next();
     List<String> list = work(s);
     for (String s1 : list) {
        nameList.remove(s1);   
    }
}

The work() method gives back a list of names that should be removed from the nameList, during the runtime of while loop.

Eran
  • 387,369
  • 54
  • 702
  • 768
Thommy
  • 5,070
  • 2
  • 28
  • 51

4 Answers4

12

No, it's not safe, and can throw ConcurrentModificationException. You can collect all the elements to be removed in a temporary List, and then call list.removeAll(tmpList) after the while loop to perform the removal.

Iterator<String> iterator = nameList.iterator();
List<String> removed = new ArrayList<>();
while(iterator.hasNext()){
    String s = iterator.next();
    removed.addAll(work(s));
}
list.removeAll(removed);

I realize this can be less efficient, since you might be calling work(s) on Strings the should have been removed from the List earlier. This can be improved by changing tempList to a Set, and only calling work(s) for Strings not in the Set:

Iterator<String> iterator = nameList.iterator();
Set<String> removed = new HashSet<>();
while(iterator.hasNext()){
    String s = iterator.next();
    if (!removed.contains(s)) {
        removed.addAll(work(s));
    }
}
list.removeAll(removed);
Eran
  • 387,369
  • 54
  • 702
  • 768
  • Yes, avoiding to call work(s) on a String that should have been removed in a previous loop is exactly what I want to do. – Thommy Feb 14 '18 at 12:32
  • I will try your idea using a "removed" set. Thanks for the suggestion. – Thommy Feb 14 '18 at 12:33
  • Of course, then you don’t need to deal with an `Iterator` manually; `for(String s: nameList) { if(!removed.contains(s)) removed.addAll(work(s)); } list.removeAll(removed);` – Holger Feb 14 '18 at 13:02
2

You can implement your logic if you use a ListIterator which is fail-safe. Below is a basic example :

Set<String> removed = new HashSet<>();
    ArrayList<String> nameList = new ArrayList<String>();

            ListIterator<String> iterator = nameList.listIterator();
             while(iterator.hasNext()){
                 String s = iterator.next();
                 if (!removed.contains(s)) {
        removed.addAll(work(s));
    }

            }


 nameList.removeAll(removed);
                 System.out.println(nameList);

With your logic, you have to account the performance. If performance is not a factor, you can go ahead and add/remove from a list through ListIterator.

Jayanth
  • 746
  • 6
  • 17
  • That still throws a ConcurrentModificationException – Thommy Feb 14 '18 at 12:30
  • just check with sysout statements that the list you are getting is empty or anything. – Jayanth Feb 14 '18 at 12:50
  • 2
    @Jayanth calling [remove()](https://docs.oracle.com/javase/8/docs/api/java/util/ListIterator.html#remove--) on the `ListIterator` exactly once per iteration step would be ok, but calling `removeAll` on the backing `List` is not. – Hulk Feb 14 '18 at 13:02
  • Hi @Thommy, may be check whether you are removing an element from namelist which is already been removed or something...because my post is just to give you an idea of making use of ListIterator. – Jayanth Feb 14 '18 at 13:03
  • @Jayanth Yes that is exactly the problem, removing one works but I want to remove multiple in one iteration. I solved it using the "workaround" from Eran's answer. – Thommy Feb 14 '18 at 13:16
0

As a previous answer said, removing an item from a iterated list is not safe. Here is a code that throws ConcurrentModificationException:

List<String> list = new ArrayList<>(Arrays.asList("1", "2", "3"));
Iterator<String> iterator = list.iterator();
while (iterator.hasNext()) {
    String s = iterator.next();
    list.remove("1");
}
senerh
  • 1,315
  • 10
  • 20
0

You can use Iterator.remove() (as opposed to nameList.remove(s)).

Iterator<String> iterator = nameList.iterator();
while(iterator.hasNext()){
    String s = iterator.next();
    iterator.remove();
}

Also consider nameList.clear() and nameList.removeAll(elementsToRemove).

Iterators are generally invalid after any modification to their underlying collections, except via the iterator itself. - When I sort a List what happens to its iterators?

https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html

Curtis Yallop
  • 6,696
  • 3
  • 46
  • 36