5

I am having an issue removing elements of a list while iterating through the list. Code:

For (WebElement element: list){
    if (!element.isEnabled() || !element.isSelected()){
        list.remove(element);
    }
}

I get a ConcurrentModificationException, which I totally understand. I am removing an item from a list while in the loop that goes through the list. Intuitively, that would screw up the indexing of the loop.

My question is, how else should I remove elements that are either not enabled or selected from this list?

Oleg Mikheev
  • 17,186
  • 14
  • 73
  • 95
jamesfzhang
  • 4,403
  • 8
  • 32
  • 43

4 Answers4

8

The easiest way to remove elements from a list in a loop is to use an ListIterator and remove elements using the routine iterator.remove()

Johan Sjöberg
  • 47,929
  • 21
  • 130
  • 148
  • I don't know if it is necessarily the easiest. `remove()` is an optional piece of functionality on the `Iterator` interface. It's also worth noting that `remove()` is on `Iterator`, and is merely inherited by `ListIterator`. – corsiKa Nov 18 '11 at 21:30
8

Modifying a list while iterating through it, in a way outside of using the iterator, results in undefined behavior. You'll have to use an iterator explicitly:

Iterator<WebElement> iter = list.iterator();
while (iter.hasNext()) {
    WebElement element = iter.next();
    if (!element.isEnabled() || !element.isSelected()) {
        iter.remove();
    }
}

See this question for more.

Community
  • 1
  • 1
Claudiu
  • 224,032
  • 165
  • 485
  • 680
  • Hmm I see. So if I want to convert the `Iterator` back to a `List`, is there an easier way than just adding each element one by one in a loop? – jamesfzhang Nov 18 '11 at 21:11
  • 2
    This doesn't convert the `List` to an `Iterator` - an `Iterator` is just an object that acts on that list itself - it's an interface for iterating through the list. When you call `iter.remove()`, it really is modifying the underlying list. – Claudiu Nov 18 '11 at 21:14
3

Others have suggested using the list iterator. That has proven useful to me, but unfortunately it relies on a method, remove(), which is considered optional by the Iterable<E> interface.

Quoth the Javadoc, nevermore (emphasis mine):

void remove()

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

To get around that has proven more useful to me is a removal list.

List<E> removed = new ArrayList<E>();
for(E element : list) {
    if(someCondition) removed.add(element);
}
list.removeAll(removed);

This has the added benefit of giving you a history of what you removed, just like the remove method does.

corsiKa
  • 81,495
  • 25
  • 153
  • 204
  • I like this one very much (+1). However, the element E should have a proper override of the equals method – GETah Nov 18 '11 at 21:26
  • 1
    @GETah not necessarily. It will work just fine without it, and may even be preferred. You may have them coming from a factory method, in which you have much less of a need for the equals method, relying strictly on reference for equality. – corsiKa Nov 18 '11 at 21:29
  • 1
    Wow I like this a lot too! Very good outside the box thinking. – jamesfzhang Nov 18 '11 at 21:29
  • Updated to include my reasons why I prefer this method over the `remove` on the iterator. – corsiKa Nov 18 '11 at 21:34
0

The ConcurrentModificationException results from the fact that the for-each syntax is just syntactic sugar for using the Iterator interface.

List iterators have what is known as the "fail-fast" attribute, meaning that any change made to the list aside from the interface provided by the iterator, immediately invalidates said iterator. Trying to use an invalidated iterator triggers your exception.

@Claudiu has already posted this code, but for clarity, I will put it here as well. In order to do what you're trying to do, you'll have to drop the fancy syntax and use a bare Iterator.

Iterator<WebElement iter = list.iterator();
while (iter.hasNext()) {
    WebElement element = iter.next();
    if (!element.isEnabled() || !element.isSelected()) {
        iter.remove();
    }
}
jpm
  • 3,165
  • 17
  • 24