Implementing multiple iterators in same class

Question

I was looking back over at some code that I wrote where I'd needed to different iterators, and I noticed that I'd written the first iterator as normal, like this:

private class Traverse implements Iterator {
    int pos = 0;
    boolean hasNextCalled = false;
    @Override
    public boolean hasNext() {
        hasNextCalled = true;
        if(pos<size()) {
            return true;
        } else {
            return false;
        }
    }

    @Override
    public Card next() {
        return cards.get(pos++);
    }

    @Override
    public void remove() {
        if (!hasNextCalled) {
            throw new IllegalStateException("hasNext() must be called before remove()");
        }
        if(pos<1) {
            throw new IllegalStateException();
        }
       cards.remove(--pos);
       hasNextCalled = false;
    }

}

and then implemented the second iterator by creating a new Traverse object but then overriding all the methods:

public Iterator OddEvenIterator() {
    Iterator it = new Traverse(){
        private int pos = 0;
        boolean hasNextCalled = false;

        @Override
        public boolean hasNext() {
            hasNextCalled = true;
            if (pos < size()) {
                return true;
            } else {
                return false;
            }
        }

        @Override
        public Card next() {
            Card nextCard = cards.get(pos);
            pos+=2;
            boolean moreCards = hasNext();
            if(moreCards==false) {
                pos=1;
            }
            return nextCard;
        }

        @Override
        public void remove() {
            if (!hasNextCalled) {
                throw new IllegalStateException("hasNext() must be called before remove()");
            }
            if (pos < 1) {
                throw new IllegalStateException();
            }
            pos-=2;
            cards.remove(pos);
            hasNextCalled = false;
        }
    };
    return it;      
}

I can't remember why I did this but it feels like a bad way to do it. I was wondering if there was any effective difference between these two methods?

yshavit · Accepted Answer · 2014-01-19 22:47:22Z

The second method creates an anonymous class that extends Traverse, meaning that you get access to its members (taking into account normal visibility rules, of course). But you then shadow both members that Traverse declares, meaning that the anonymous class won't use them.

Given that you define every Iterator method and that the return value is typed to Iterator, there's no reason to extend a class rather than implementing Iterator directly, unless you need access to some members that that class provides. Since you don't in this case, there's no reason to extend Traverse.

Furthermore, Traverse doesn't define any methods other than the Iterator methods, so there wouldn't even be any use in downcasting the iterator returned from OddEvenIterator to Traverse. If there were, that might be a reason to extend it -- though it would also be a code smell.

The only other reason to extend Traverse would be if some other code uses reflection to check for subtypes of Traverse (including using instanceof). That, again, would probably be code smell -- but it's a possibility.

There are a few stylistic things I'd suggest, btw:

You should use generics rather than the raw Iterator type
Most Java conventions have method names start lower-cased (so, oddEvenIterator())
in hasNext, rather than the if-else, you can just return pos < size(). This is simpler, and always preferable.

Collectives™ on Stack Overflow

Implementing multiple iterators in same class

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related