1

I'm trying to write a custom made Iterator for a Set that I made. I'm a bit confused about the contract for the Interface Iterable. It has three methods: next(), hasNext() and remove(). My set is immutable so I'm planning on throwing an UnsupportedOperationException for the method remove(). It is also so called "lazily generated" ie elements are not stored in a memory but rather created when needed, but that's neither here nor there.

The javadoc for the Iterator's next() method is as follows:

E next()

Returns the next element in the iteration.
Returns:
the next element in the iteration
Throws:
NoSuchElementException - if the iteration has no more elements

and for hasNext() is:

boolean hasNext()

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

Going by these rules, I started implementing my Set and Iterator, getting this:

import java.util.AbstractSet;
import java.util.Iterator;

public class PrimesBelow extends AbstractSet<Integer>{

    int max;
    int size;

    public PrimesBelow(int max) {
        this.max = max;
    }

    @Override
    public Iterator<Integer> iterator() {
        return new SetIterator<Integer>(this);
    }

    @Override
    public int size() {
        if(this.size == -1){
            System.out.println("Calculating size");
            size = calculateSize();
        }else{
            System.out.println("Accessing calculated size");
        }
        return size;
    }

    private int calculateSize() {
        int c = 0;
        for(Integer p: this)
            c++;
        return c;
    }

    public static void main(String[] args){
        PrimesBelow primesBelow10 = new PrimesBelow(10);
        for(int i: primesBelow10)
            System.out.println(i);
        System.out.println(primesBelow10);
    }
}

.

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

public class SetIterator<T> implements Iterator<Integer> {
    int max;
    int current;
    public SetIterator(PrimesBelow pb) {
        this.max= pb.max;
        current = 1;
    }

    @Override
    public boolean hasNext() {
        if(current < max) return true;
        else return false;
    }

    @Override
    public Integer next() {
        while(hasNext()){
            current++;
            if(isPrime(current)){
                System.out.println("returning "+current);
                return current;
            }
        }
        throw new NoSuchElementException();
    }

    private boolean isPrime(int a) {
        if(a<2) return false;
        for(int i = 2; i < a; i++) if((a%i)==0) return false;
        return true;
    }
}

This seemed fine by my, next() return the next value and throws an Exception when there is no more. hasNext() should return true if there are more values to iterate over and false otherwise. The output from the main loop however is this:

returning 2
2
returning 3
3
returning 5
5
returning 7
7
Exception in thread "main" java.util.NoSuchElementException
    at SetIterator.next(SetIterator.java:27)
    at SetIterator.next(SetIterator.java:1)
    at PrimesBelow.main(PrimesBelow.java:38)

So it seems the Exception isn't handled in the for each loop. How do I write a custom Iterator that I can use so that this works? I tried returning null instead of throwing an Exception but that just gives a NullPointerException.

Should I return null when the Iterator is finished, or throw an Exception? Javadoc says next() should throw an Exception but when I hover over the overriden method next() in Eclipse the signature doesn't show throws NoSuchElementException so I'm very confused as to what the contract says. It seems very weird to me to return null when I'm done since the collection could potentially contain null elements.

Thank for the help.

3
  • Why do you loop over hasNext() in your implementation of next()? You should only check if there is a next element without a while-loop. Commented Aug 18, 2017 at 12:18
  • I think I did it because that's where the logic is to check if the Iterator can return more elements, to avoid code duplication and writing if(current < max) two different times. current iterates over all natural numbers, so it needs to be checked that current is a prime, and that it is smaller than the max value. Since not all natural numbers are prime, I need to keep increasing current until I hit a prime number or the limit, that's why the while loop. Commented Aug 18, 2017 at 12:22
  • @user1661303 the logic doesn't make sense mate, next() should return a single value (not looping through it). and this code will always throw an exception because after looping through all elements u end up throwing an exception regardless Commented Aug 18, 2017 at 12:26

2 Answers 2

1

Change

while(hasNext()) {
    //...
}
throw new NoSuchElementException();

To

if(hasNext()) {
    //...
} else {
    throw new NoSuchElementException();
}
Sign up to request clarification or add additional context in comments.

8 Comments

Ah. Ok. I almost see what you're saying, but I don't see how the while loop in my example iterate over the elements in the Set. Doesn't it just iterate over the natural numbers until it hits an element that is in the Set, or reaches the limit and then throws an Exception? Say for example current is 3. A call to next() then should just increase it to 4, then increase it to 5, then see it's a prime and return it right? Not iterate over the entire Set. Maybe I'm missing something here.
This was my first thought as well, but this won't fix the problem. The loop is needed to compute the next prime.
@dpr but loop should be outside of iteratator isn't it?
This code seemed right, but returns [2, 3, 4, 5, 6, 7, 8, 9, 10] as the set when I run it. -edit
@user1661303 u can unaccept my answer if it is too principal for dpr. I just like to help people)
|
1

Suppose the last prime returned by your code is 7. Then current will be 7 which is obviously < 10. So hasNext() will return true. However there is no more prime greater than 7 but smaller than 10, so the next call to next() will produce the NoSuchElementException. Currently your code will only work, if max is a prime.

You will need to validate, that there is an additional prime available in hasNext().

1 Comment

Ah. Got it. Thanks for corner case explanation. Now I understand :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.