Self-made linked list iterator

I recently made a linked list for myself, the code of which was posted here, if anyone would be interested. Now, as the next task, I had to make an Iterator for my list, which is working properly.

public Iterator iterator() {
    final MyLinkedList list = this;
    return new Iterator() {
        final Node firstNode = list.firstNode;
        Node currentNode = null;
        @Override
        public boolean hasNext() {
            if (list.isEmpty()) {
                return false;
            } else if (currentNode == null){
                return true;
            } else if (currentNode == list.lastNode){
                return false;
            }
            return true;
        }
        @Override
        public T next() {
            if (list.isEmpty()){
                throw new NoSuchElementException();
            } else if (currentNode == null){
                this.currentNode = firstNode;
                return currentNode.data;
            } else if (currentNode.nextNode == null) {
                throw new NoSuchElementException();
            }
            this.currentNode = currentNode.nextNode;
            return currentNode.data;
        }
    };
}

Here are the variables of the LinkedList class:

private Node lastNode;
private Node firstNode;
private int size;

This is a Node:

private static class Node{
    private Node nextNode = null;
    private Node previousNode = null;
    private int index;
    private U data;

    private Node(U data){
        this.data = data;
    }
}

And this is how I add a new Node:

```
public boolean add(T data) {
if (data == null) {
return false;
}

Node currentNode = new Node(data);

if (this.isEmpty()){
currentNode.previousNode = null;
currentNode.index = 0;
this.firstNode = currentNode;
} else {
currentNode.previousNode = lastNode;
lastNode.nextNode = currentNode;
currentNode.index = lastNode.index + 1;

Does the code comply with the Java coding conventions?

I think it does comply quite well. A tiny thing is that it would be better to put a space between ){, for example in if (list.isEmpty()){. More importantly, it can be simplified.

There is no need for the inner list variable pointing to this.
The inner (non-static) class has direct access to the containing class' fields and methods: it can access firstNode directly from the containing list. So in this code:

final MyLinkedList list = this;
return new Iterator() {
    final Node firstNode = list.firstNode;

You can drop both the list variable and the firstNode variable.

It's a minor thing, but currentNode can be private.

The hasNext method can be simplified by a lot:

@Override
public boolean hasNext() {
    if (list.isEmpty()) {
        return false;
    } else if (currentNode == null){
        return true;
    } else if (currentNode == list.lastNode){
        return false;
    }
    return true;
}

This is equivalent:

@Override
public boolean hasNext() {
    return !isEmpty() && currentNode != lastNode;
}

Could this be made significantly faster?

Slightly. Notice that the isEmpty() check is pointless to perform for every iteration of a non-empty list. So as a minor optimization,
you could check for emptiness once, in the beginning,
and then omit all the isEmpty() checks from the rest of the code.

Suggested implementation

Putting it all together:

public Iterator iterator() {
    if (isEmpty()) {
        return Collections.emptyList().iterator();
    }
    return new Iterator() {
        private Node currentNode = null;

        @Override
        public boolean hasNext() {
            return currentNode != lastNode;
        }

        @Override
        public T next() {
            if (currentNode == null) {
                currentNode = firstNode;
                return currentNode.data;
            }
            if (currentNode.nextNode == null) {
                throw new NoSuchElementException();
            }
            currentNode = currentNode.nextNode;
            return currentNode.data;
        }
    };
}

Unit testing

I didn't just refactor aggressively. I wrote unit tests first to know that I'm not breaking anything. After that I could go ahead and refactor aggressively:

@Test
public void testEmpty() {
    MyLinkedList list = new MyLinkedList<>();
    assertFalse(list.iterator().hasNext());
}

@Test(expected = NoSuchElementException.class)
public void throwIfNextOnEmpty() {
    MyLinkedList list = new MyLinkedList<>();
    list.iterator().next();
}

@Test(expected = NoSuchElementException.class)
public void throwIfIterateBeyond() {
    MyLinkedList list = new MyLinkedList<>();
    list.add(1);
    list.add(2);
    Iterator iter = list.iterator();
    iter.next();
    iter.next();
    iter.next();
}

@Test
public void testStandardIteration() {
    MyLinkedList list = new MyLinkedList<>();
    Integer[] items = { 12, 3, 4 };
    for (int i : items) {
        list.add(i);
    }
    Iterator iter = list.iterator();
    for (Integer item : items) {
        assertTrue(iter.hasNext());
        assertEquals(item, iter.next());
    }
    assertFalse(iter.hasNext());
}

These may not be perfect, and might not cover all corner cases, but I hope they are enough to get you started.

StackExchange Code Review Q#66938, answer score: 7