Simple Example of an Iterable and an Iterator in Java

I was a TA for a first-year Java programming course this year. As a very simple example of iterables/iterators, I wrote the following code for the students. I am curious if any styling or other improvements can be made.

import java.util.NoSuchElementException;
import java.util.Iterator;

public class Range implements Iterable {
private int start, end;

public Range(int start, int end) {
this.start = start;
this.end = end;
}

public Iterator iterator() {
return new RangeIterator();
}

// Inner class example
private class RangeIterator implements
Iterator {
private int cursor;

public RangeIterator() {
this.cursor = Range.this.start;
}

public boolean hasNext() {
return this.cursor it = range.iterator();
while(it.hasNext()) {
int cur = it.next();
System.out.println(cur);
}

// Shorter, nicer way:
// Read ":" as "in"
for(Integer cur : range) {
System.out.println(cur);
}
}
}


Variables

I understand why you have the Range.this.end and Range.this.start to avoid confusion about where those variables come from... If you need the Range.this as part of the teaching exercise, then sure. Otherwise, I would recommend three things....

• add range as a prefix, even though it is slightly redundant

• Make them final...

• one variable per line... (it makes revision-control diffs/patches easier to read)

The code would look like:

private final int rangeStart;
private final int rangeEnd;

Then, all the Range.this.start would be just rangeStart, etc.

Nested classes

Your iterator class is a non-static class, so it can reference the outer class's range start/end.

In this case, the nested class can be changed to a static class very easily. This has the potential of simplifying memory management because the iterator does not need a reference to the enclosing Range.

Consider a private-static Iterator instance:

// Inner class example
private static final class RangeIterator implements
                Iterator {
    private int cursor;
    private final int end;

    public RangeIterator(int start, int end) {
        this.cursor = start;
        this.end = end;
    }

    public boolean hasNext() {
        return this.cursor < end;
    }

    public Integer next() {
        if(this.hasNext()) {
            int current = cursor;
            cursor ++;
            return current;
        }
        throw new NoSuchElementException();
    }

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

This static class removes the need for the back-references to Range.this entirely....

The new iterator is called simply with:

public Iterator iterator() {
    return new RangeIterator(start, end);
}

Pre-Validate

It is better to pre-validate state, than to fall-through to an error... This code:

public Integer next() {
        if(this.hasNext()) {
            int current = cursor;
            cursor ++;
            return current;
        }
        throw new NoSuchElementException();
    }

would be better as:

public Integer next() {
        if(!this.hasNext()) {
            throw new NoSuchElementException();
        }
        int current = cursor;
        cursor ++;
        return current;
    }

Post-Increment

This block can be simplified:

int current = cursor;
        cursor ++;
        return current;

to just:

return cursor++;

although I imagine this is done as an education ploy.

Integer as the example

Because of the int auto-bocxing I worry that Integer may not be the right choice for data type. You may want to consider a non-primitive as the data.

Autoboxing is the sort of thing that will confuse.

Conclusion

Otherwise, I don't see much in the way of problems.

StackExchange Code Review Q#48109, answer score: 36