Iterator implementation causing infinite loop

Question

I am trying to do a foreach loop as followed for a linkedlist-esque class I built:

boolean contains = false;
for(Thing t : front) {
    if(t.equals(something)) {
        t.doSomething();
        contains = true;
        break;
    } 
}

This are the iterator methods I've implemented:

@Override
public Iterator<Thing> iterator() {
    Thing current = this;
    return new Iterator<Thing>() {
        public boolean hasNext() {
            return current.hasNext();
        }

        public Thing next() {
            return current.next;
        }
    };
}

I've tried debugging this and it seems it isn't returning the next element in the linkedlist, thus causing an infinite loop when I run it and try and do a foreach loop.

Sorry if this question has been answered or there's a really stupid mistake, I searched for a while and couldn't find an answer. This is my first time writing an iterator so please be gentle. :)

Eran · Accepted Answer · 2015-11-10 10:37:15Z

6

You have to advance the Iterator's state in next() :

    public Thing next() {
        current = current.next;
        return current;
    }

Otherwise, all calls to next() would return the same element.

EDIT :

You should move your local variable declaration Thing current = this; inside your anonymous class instance (i.e. turn it into an instance variable).

@Override
public Iterator<Thing> iterator() {
    return new Iterator<Thing>() {

        private Thing current = Thing.this;

        public boolean hasNext() {
            return current.hasNext();
        }

        public Thing next() {
            current = current.next;
            return current;
        }

    };
}

edited Nov 10, 2015 at 10:37

answered Nov 10, 2015 at 10:19

Eran

395k57 gold badges726 silver badges793 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Tbs95 Over a year ago

When I do this, I get an error in Eclipse: "Local variable current defined in an enclosing scope must be final or effectively final". I tried changing the next method to the following: public Thing next() { Thing tmp = current; tmp = current.next; return tmp; } But it is still not advancing to the next element in the list.

khelwood Over a year ago

@Tbs95 An anonymous class in a method does not have alteration rights on variables inside the enclosing method. You need to make the current variable an instance variable of the Iterator, because it is part of the iterator's state.

Tbs95 Over a year ago

Thank you @Eran, but now that I have changed the variable declaration location, I am getting a Type Mismatch error converting new Iterator<Thing>(){} to Thing.

Tbs95 Over a year ago

Gosh, I swear I already tried that and it gave me a different error... Worked flawlessly though, thank you!

Collectives™ on Stack Overflow

Iterator implementation causing infinite loop

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related