An Iterator in Java is a special object, a mean of iteration, that allows to retrieve elements sequentially one by one from a particular source.
There are two method that are mandatory to implement while creating a custom iterator: hasNext()
(returns true
if the next element exists) and next()
(retrieves the next element).
You didn't provide your class with implementation of hasNext()
without that your code will not compile.
And there's a logical flaw in the next()
method, it will not compile because you didn't provide the return statement or throws clause that will get executed when control cant enter the loop. But more important you don't need loops and any conditional logic inside this method, it has to be covered by the hasNext()
which normally has to be invoked before next()
. If the client code doesn't respect it, method next()
might produce an exception. You might add if (hasNext())
at the very beginning of the next()
method to issue a particular exception with your custom message.
Method iterator()
is accessible with arrays. You can use it with classes that implement Iterable
interface, like collections, and you also can invoke iterator()
on a stream. So you can reimplement your method range()
like that:
IntStream.iterate(min, i -> i < max, i -> i + step).iterator();
And that is how you can fix your iterator:
public class Generator implements Iterator<Integer> {
private final Iterator<Integer> xIterator;
private final Iterator<Integer> yIterator;
public Generator(int minX, int minY, int maxX, int maxY, int stepX, int stepY) {
this.xIterator = range(minX, maxX, stepX);
this.yIterator = range(minY, maxY, stepY);
}
public static Iterator<Integer> range(int min, int max, int step) {
return IntStream.iterate(min, i -> i < max, i -> i + step).iterator();
}
@Override
public boolean hasNext() {
return xIterator.hasNext() && yIterator.hasNext();
}
@Override
public Integer next() {
return foo(xIterator.next(), yIterator.next());
}
}
But my suggestion is to favor efficiency and simplicity over conciseness. Because all the values that iterator produces can be easily calculated on the fly, there's no need to occupy memory by allocating them in advance.
Instead, you can maintain two variables curX
and curY
. This solution is simple and also gives more control over your iterator because you're not delegating the process of iteration. As a consequence of this, you can implement a reset()
feature (which is not possible which previous solution, Iterator
becomes useless when it reaches the end of the source of data).
public class Generator implements Iterator<Integer> {
private final int minX;
private final int minY;
private final int maxX;
private final int maxY;
private final int stepX;
private final int stepY;
private int curX;
private int curY;
public Generator(int minX, int minY, int maxX, int maxY, int stepX, int stepY) {
this.minX = minX;
this.minY = minY;
this.maxX = maxX;
this.maxY = maxY;
this.stepX = stepX;
this.stepY = stepY;
this.curX = minX;
this.curY = minY;
}
@Override
public boolean hasNext() {
return curX < maxX && curY < maxY;
}
@Override
public Integer next() {
int result = foo(curX, curY);
curX += stepX;
curY += stepY;
return result;
}
public void reset() { // reset the iterator to the initial coordinates
this.curX = minX;
this.curY = minY;
}
}