Java Iterator implementation - next() and hasNext() enforcing order

Question

I have an implementation of java.util.Iterator which requires that the call to next() should always be proceeded by a call to hasNext(). (This is because results are returned asynchronosly in a multi threaded environment and it is never clear how many more results there might be).

Would it be 'correct' to properly document this in the JavaDoc and then throw a RuntimeException if this was violated. Or is this stretching the Iterator interface a little too far?

All thoughts appreciated?

Answer 1

I might be missing something here, but why not call hasNext() internally in your implementation?

Answer 2

If your hasNext() and next() calls aren't in a synchronized block/method, it is not guaranteed that you will have elements even if you call hasNext() before next().

The contract of the Iterator interface is that NoSuchElementException should be thrown if there are no more elements. So proceed with the next() method until such an exception arises.

That said, take a look at the java.util.concurrent package - it has concurrent collections whose iterators may help you - i.e. you can use these collections and iterators instead of implementing your own.

Answer 3

I would rather throw an exception from next(), when there are no more elements. In a multi-threaded environment hasNext() is pretty useless anyway.

Answer 4

I imagine you're doing something like this:

class IteratorImpl<T> implements Iterator<T> {
  private Source<T> source = ...
  private T next = null;

  public boolean hasNext() {
    if(next == null) {
      next = source.poll();
    }
    return next != null;
  }

That sounds OK to me. I can't imagine a situation where you'd want to use next without hasNext - it would be a recipe for exceptions.

---

EDIT:

The doc for hasNext() says:

Returns true if the iteration has more elements. (In other words, returns true if next would return an element rather than throwing an exception.)

To me, the implementation does not violate the contract. However, I would (as Fabian Steeg implies) still implement next() as:

  public T next() {
    if(!hasNext()) {
      throw new NoSuchElementException();
    }
    T ret = next;
    next = null;
    return ret;
  }

I mean, what does that check really cost you?

You must check and throw a NoSuchElementException as per the API contract. Either testing on !hasNext() or next == null will meet this criteria, I believe, but I would favour the former.

If someone is catching NoSuchElementException instead of calling hasNext(), you probably have bigger problems.

Answer 5

Requiring that hasNext() be called before next() violates the iterator contract. You really should rewrite it so that next() simply throws a NoSuchElementException if there is no element to return.

Answer 6

You could do something similar to the below, whereby you delegate the underlying data fetch to a private method, and implement hasNext() and next() to react differently to the absence of data. This has the advantage that you can repeatedly call next() without calling hasNext() first, and hence do not violate the contract of Iterator.

public class IteratorImpl<T> implements Iterator<T> {
  private final Source<T> source;
  private T next;

  public synchronized boolean hasNext() {
    tryGetNext();
    return next != null;
  }

  public synchronized T next() {
    tryGetNext();

    if (next == null) {
      throw new NoSuchElementException();
    } 

    return next;
  }

  private void tryGetNext() {
    if (next != null) {
      next = source.poll();
    }
  }
}

Answer 7

I would also suggest implementing the iterator as fail-fast

Answer 8

What you can do is implement the Iterator interface yourself (if you need to interface with other API) and ask your clients to implement your own stricter interface.

I created such a class in my functional programming library in order to easily implement an Iterator around a ResultSet in a project at work.

My class EasierIterator implements the Iterator interface while requiring that the client implements a simpler interface based on moveNext()/getCurrent(). It actually does the caching of the current item for you.