1

This little annoyance caused me to lose an hour of sleep, and I don't understand why.

I have an ArrayList array which I want to iterate over and conditionally remove items. This was my first attempt:

for (int i = 0; i < array.size(); i++) {
   if (array.get(i) == conditionMet) array.remove(i);
}

And that did not work. The following did:

for (Iterator<T> i = array.iterator(); i.hasNext();) {
   if (i.next() == conditionMet) i.remove();
}

Why?

Jonik
  • 80,077
  • 70
  • 264
  • 372
ledhed2222
  • 675
  • 6
  • 11
  • possible duplicate of [loop on list with remove](http://stackoverflow.com/questions/1921104/loop-on-list-with-remove) – McDowell May 15 '11 at 15:33
  • you should have included which exception you are getting, most probably "Exception in thread "main" java.lang.IndexOutOfBoundsException". – Bhushan May 16 '11 at 23:02

6 Answers6

7

You didn't specify how it didn't work but when you're iterating over the ArrayList in the for loop and removing the element at the current index i, the size of the collection and indices for subsequent elements, both change which is probably what you were not expecting.

From the documentation for ArrayList#remove(int index):

Removes the element at the specified position in this list. Shifts any subsequent elements to the left (subtracts one from their indices).

no.good.at.coding
  • 20,221
  • 2
  • 60
  • 51
  • Valiant attempt no.good, but I don't think that's what's happening. In the first example, `array.remove()` never even removes elements. This broke when I gave it two different conditions, the first tested whether the element at i was unique: `if (array.indexOf(array.get(i)) != array.lastIndexOf(array.get(i)))` The other returned a boolean from another function: `if (!(isCity(array.get(i))))` – ledhed2222 May 15 '11 at 15:39
  • 1
    ...in other words if the documentation you mention was causing the bug in the first case (which could make sense), I don't see how it could in the second case. – ledhed2222 May 15 '11 at 15:46
  • @ledhed2222 So the failure is that elements that ought to be removed (because they pass the conditional test) aren't removed? And the same conditional statements are used in the second case? Could you post the actual code that fails? This might not matter but what is the type of the data in the `ArrayList`? – no.good.at.coding May 15 '11 at 15:55
1

What type is the ArrayList and conditionMet? You can't compare objects with == only with the method .equals (which you should override).

edit: (haven't done Java in a while and not 100% sure if this will be correct, you should test it)

Iterator<E> it = list.getIterator();
while(it.hasNext()) {
  E obj = it.next();
  if(obj.equals(VARIABLE)) {
    it.remove();
  }
}
Sir Troll
  • 105
  • 2
  • 7
1

The problem is that you're trying to change the list over which you're iterating. if you did something like this:

int listSize = array.size();
for (int i = 0; i < list; i++) {
    if (array.get(i) == conditionMet) {
        array.remove(i);
        i--;
        listSize--;
    }
}

This way, you're just iterating up to an int, rather than using the size of an ArrayList that you're changing in the loop body, which Java doesn't allow.

Hope that works for you.

Ed.
  • 176
  • 4
0

Simple. In the second case, i is a class of type iterator, and in the first, i is an Integer. When you remove the values, while iterating, the indices will change (basically, skip one). So each time you remove, you have to decrement i by one (which will be again incremented by the for loop). So, modifying the first one to 

for (int i = 0; i < array.size(); i++) {
    if (array.get(i) == conditionMet) array.remove(i--);
}
Raze
  • 2,175
  • 14
  • 30
0

Collections don't like having members removed mid-loop except when it's done through an Iterator.

Iterators allow the caller to remove elements from the underlying collection during the iteration with well-defined semantics.

Andrew Lazarus
  • 18,205
  • 3
  • 35
  • 53
0

In the first for loop,

for (int i = 0; i < array.size(); i++) {
   if (array.get(i) == conditionMet) array.remove(i);
}

the condition ; i < array.size(); is evaluated in the very beginning which will take the size of the original list. So while iterating over it, if you remove any element from it, the size has reduced, but you are still iterating up to the original size, which is obviously wrong.

In the for second loop, 

for (Iterator<T> i = array.iterator(); i.hasNext();) {
   if (i.next() == conditionMet) i.remove();
}

since you are using iterator, you are checking if there is anymore element is left in the list or not. hence you do not overstep and it works properly.

(You should have included the exception in the question. "Exception in thread "main" java.lang.IndexOutOfBoundsException")

Bhushan
  • 18,329
  • 31
  • 104
  • 137