views:

598

answers:

7

I'm migrating a piece of code to make use of generics. One argument for doing so is that the for loop is much cleaner than keeping track of indexes, or using an explicit iterator.

In about half the cases, the list (an ArrayList) is being iterated in reverse order by using an index.

Can someone suggest a cleaner way of doing this (since I dislike the indexed for loop when working with collections)?

 for (int i = nodes.size() - 1; i >= 0; i--) {
    final Node each = (Node) nodes.get(i);
    ...
 }

Note: I can't add any new dependencies outside the JDK.

Took accepted answer and turned it into:

class ListReverser<T> implements Iterable<T> {
    private ListIterator<T> listIterator;        

    public ListReverser(List<T> wrappedList) {
        this.listIterator = wrappedList.listIterator(wrappedList.size());            
    }               

    public Iterator<T> iterator() {
        return new Iterator<T>() {

            public boolean hasNext() {
                return listIterator.hasPrevious();
            }

            public T next() {
                return listIterator.previous();
            }

            public void remove() {
                listIterator.remove();
            }

        };
    }

}

Which can be used as:

for (final Node each : new ListReverser<Node>(nodes)) {
   //...
}

Not great, but better.

+3  A: 

Create a custom reverseIterable

nanda
Don't know why this is getting down voted, I might just make a wrapper for the list that does this.
Allain Lalonde
This is no less clean than calling Collections.reverse() IMHO.
Adamski
@Allain: Agreed re. the downvotes although I don't see why you view a call to listIterator(list.size()) as "not clean". Even if you wrap it you're still having to make the same method call somewhere.
Adamski
Suppose not, just not willing to take the performance hit, for cleanliness.
Allain Lalonde
+3  A: 

Have you thought about reversing the List with Collections#reverse() and then using foreach?

Of course, you may also want to refactor your code such that the list is ordered correctly so you don't have to reverse it.


EDIT:

Alternatively, could you use a Deque instead of an ArrayList? It will allow you to iterate forwards and backwards


EDIT:

As others have suggested, you could write an Iterator that will go through the list in reverse:

import java.util.Iterator;
import java.util.List;

public class ReverseIterator<T> implements Iterator<T>, Iterable<T> {

    private final List<T> list;
    private int position;

    public ReverseIterator(List<T> list) {
        this.list = list;
        this.position = list.size() - 1;
    }

    @Override
    public Iterator<T> iterator() {
        return this;
    }

    @Override
    public boolean hasNext() {
        return position >= 0;
    }

    @Override
    public T next() {
        return list.get(position--);
    }

    @Override
    public void remove() {
        throw new UnsupportedOperationException();
    }

}


List<String> list = new ArrayList<String>();
list.add("A");
list.add("B");
list.add("C");
list.add("D");
list.add("E");

for (String s : new ReverseIterator<String>(list)) {
    System.out.println(s);
}
Kevin
Have thought about it, but there cost of reversing it is prohibitive. Also, it only requires this kind of iteration about half the time. Flipping it would just move the problem to the other half.
Allain Lalonde
+1 for repeatedly stabbing this one
Allain Lalonde
+ for Deque; typical implementations have `descendingIterator()`.
trashgod
A: 

I don't think it's possible using the for loop syntax. The only thing I can suggest is to do something like:

Collections.reverse(list);
for (Object o : list) {
  ...
}

... but I wouldn't say this is "cleaner" given that it's going to be less efficient.

Adamski
+7  A: 

Try this:

// Substitute appropriate type.
ArrayList<...> a = new ArrayList<...>();

// Add elements to list.

// Generate an iterator. Start just after the last element.
ListIterator li = a.listIterator(a.size());

// Iterate in reverse.
while(li.hasPrevious()) {
  System.out.println(li.previous());
}
John Feminella
Not bad. Doesn't use the index, but loses the elegance of the for each syntax. +1 anyway.
Allain Lalonde
You want an index on the `listIterator` call, I think.
Tom Hawtin - tackline
Calling listIterator() without an index argument will give an Iterator at the beginning of the list and so hasPrevious() will return false on the first call.
Adamski
Whoops, yes. Thanks Adamski, Tom.
John Feminella
You can write an `Iterator` that uses a `ListIterator` in reverse, but that may not be worth it for one loop.
Tom Hawtin - tackline
It's not one loop, so I've taken this and wrapped it. http://pastebin.ca/1759041 so, now I can do `for (Node each : new ListReverse<Node>(nodes)) { }`
Allain Lalonde
A: 

Here is an (untested) implementation of a ReverseIterable. When iterator() is called it creates and returns a private ReverseIterator implementation, which simply maps calls to hasNext() to hasPrevious() and calls to next() are mapped to previous(). It means you could iterate over an ArrayList in reverse as follows:

ArrayList<String> l = ...
for (String s : new ReverseIterable(l)) {
  System.err.println(s);
}

Class Definition

public class ReverseIterable<T> implements Iterable<T> {
  private static class ReverseIterator<T> implements Iterator {
    private final ListIterator<T> it;

    public boolean hasNext() {
      return it.hasPrevious();
    }

    public T next() {
      return it.previous();
    }

    public void remove() {
      it.remove();
    }
  }

  private final ArrayList<T> l;

  public ReverseIterable(ArrayList<T> l) {
    this.l = l;
  }

  public Iterator<T> iterator() {
    return new ReverseIterator(l.listIterator(l.size()));
  }
}
Adamski
Looks right to me, though exposing it as a static method instead of a public constructor would (in most cases) avoid the need for the client to specify the type parameter. This is how Guava does it..
Kevin Bourrillion
+1  A: 

Also found google collections reverse method.

Allain Lalonde
A: 

You could use the concrete class LinkedList instead of the general interface List. Then you have a descendingIterator for iterating with the reverse direction.

LinkedList<String > linkedList;
for( Iterator<String > it = linkedList.descendingIterator(); it.hasNext(); ) {
    String text = it.next();
}

Don't know why there is no descendingIterator with ArrayList...

tangens