Java Iterator vs for loop

Question

Good evening,

I'm trying to call the "ricolora" method on each pixel inside the arraylist (change the color of the pixel), the first method (the one with the iterator) doesn't work.

It gives me an exception (java.lang.reflect.InvocationTargetException).

The second method (for loop) works just fine. Can you please help me understand why the first method doesn't work, i thought that the for loop and the iterator were almost the same thing.

Thank you for your help.

public class DisegnoManoLibera {
       protected final ArrayList<Pixel> pixel;

       final public void ricolora(Color c) {
          Iterator<Pixel> it = this.pixel.iterator();
          int i=0;
          while(it.hasNext()){
              Pixel pi =(Pixel) it.next();
              Pixel gi = new Pixel(pi.getX(), pi.getY(), c);
              pixel.remove(i);
              pixel.add(i, gi);
              i++;
          }
       }

    final public void ricolora(Color c) {
         for(int i=0; i<this.pixel.size();i++){
            Pixel pip = pixel.get(i);
            Pixel gin = new Pixel(pip.getX(), pip.getY(), c);
            pixel.remove(i);
            pixel.add(i, gin);
         }
    }
public class Pixel {
  final int x;
  final int y;
  final Color c;

@redFIVE Enhanced `for` loop won't allow updating the collection during the iteration, which is what OP is doing. OP is just doing it the wrong way. — Andreas, Jan 21 '16 at 18:34
You're sidestepping the iterator in your implementation using it. By creating the `int i` variable and accessing the list by index you've broken the whole idea of `Iterator`s. Also, there is an `Iterator#Remove()` method if you want to remove an element. — Draco18s no longer trusts SE, Jan 21 '16 at 18:36

score 1 · Answer 1 · edited May 23 '17 at 12:15

Hava a look here, this explains what differant ways are out there to iterate through a list. There you see that you are not supposed to manipulate the underlying list while using an iterator (your pixel.remove(i); and pixel.add(i, gi);).

As an alternative, you could use a ListIterator which has methods for remove() and add() (or in you case set() to replace the element).

for (ListIterator<E> iter = list.listIterator(); iter.hasNext(); ) {
    E element = iter.next();
    // 1 - can call methods of element
    // 2 - can use iter.remove() to remove the current element from the list
    // 3 - can use iter.add(...) to insert a new element into the list
    //     between element and iter->next()
    // 4 - can use iter.set(...) to replace the current element

    // ...
}

And to cite the other post again:

Note: As @amarseillan pointed out, this form is a poor choice for iterating over Lists because the actual implementation of the get method may not be as efficient as when using an Iterator

Andreas · Accepted Answer · 2016-01-21T18:38:27.300

Why are you removing and adding, when you can simply replace the value? You should use either List.set(int index, E element), or ListIterator.set(E e).

While iterating a collection, you are generally not allowed to modify the collection, except through the iterator. The iterators of most collection objects implement fail-fast logic to prevent accidentally violating that rule. The exception are for collections specifically designed for multi-threaded concurrent access.

So, if using an Iterator, only modify through that iterator.

// Using ListIterator
public final void ricolora(Color c) {
    for (ListIterator<Pixel> it = this.pixel.listIterator(); it.hasNext(); ) {
        Pixel pi = it.next();
        Pixel gi = new Pixel(pi.getX(), pi.getY(), c);
        it.set(gi);
    }
}

// Using index
public final void ricolora(Color c) {
    for (int i = 0; i < this.pixel.size(); i++) {
        Pixel pi = this.pixel.get(i);
        Pixel gin = new Pixel(pi.getX(), pi.getY(), c);
        this.pixel.set(i, gin);
    }
}

The Iterator version is generally preferred, because it performs well regardless of List implementation, e.g. the index version will perform badly if the List is a LinkedList.

score 0 · Answer 3 · answered Jan 21 '16 at 18:26

The point in code where Iterator is obtained for a collection, the structural modifications caused by the operations like resize, add or remove to the collection are not recommended. You can try using CopyOnWriteArrayList if you need that functionality.

Java Iterator vs for loop

3 Answers3