An iterable implementation of LinkedList

I am curious as to what improvements to my code can be suggested.

package concepts;

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

public class LinkedList implements Iterable> {

    public static class LinkedListIterator implements Iterator> {
        private LinkedListNode curNode;

        public LinkedListIterator(LinkedList linkedList) {
            curNode = linkedList.head;
        }

        @Override
        public boolean hasNext() {
            if (curNode != null) {
                return true;
            } else {
                return false;
            }
        }

        @Override
        public LinkedListNode next() {
            if (!hasNext()) {
                throw new NoSuchElementException();
            } else {
                LinkedListNode temp = curNode;
                curNode = curNode.next;
                return temp;
            }

        }

    }

    public LinkedListNode head;

    public boolean add(M newData) {
        LinkedListNode newNode = new LinkedListNode();
        newNode.data = newData;
        if (head == null) {
            head = newNode;
        } else {
            LinkedListIterator it = new LinkedListIterator(this);
            LinkedListNode lastNode = null;
            while (it.hasNext()) {
                lastNode = it.next();
            }
            if (lastNode != null) {
                lastNode.next = newNode;
            } else {
                throw new IllegalStateException();
            }
        }
        return true;
    }

    @Override
    public Iterator> iterator() {
        return new LinkedListIterator(this);
    }

}

Here is the LinkedListNode implementation:

package concepts;

public class LinkedListNode {
    public N data;
    public LinkedListNode next;
}

Antipattern

It's not a good idea to expose the internal list nodes, and this is why:

public static void main(String[] args) {
    LinkedList list = new LinkedList<>();
    list.add(123);
    // Make a loop:
    list.head.next = list.head;
    int i = 0;

    for (LinkedListNode node : list) {
        ++i;

        if (i == 100) {
            break;
        }
    }

    System.out.println("i = " + i);
}

As the above snippet demonstrates, a user may restructure your linked list into a cycle in which iteration will never terminate. So the moral here is that:

hide implementation from the user!

First of all, make head a private field. Then, reimplement the iterator that iterates over values and not the internal nodes holding those values.

Additional field tail would not hurt, while keeping your add method a constant time operation.

Summa summarum

All in all, I had this in mind:

package concepts;

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

public class LinkedListCR implements Iterable {

    private static final class LinkedListNode {
        private E element;
        private LinkedListNode next;

        LinkedListNode(E element) {
            this.element = element;
        }
    }

    private class LinkedListIterator implements Iterator {

        private LinkedListNode nextNodeToReturn = head;

        @Override
        public boolean hasNext() {
            return nextNodeToReturn != null;
        }

        @Override
        public E next() {
            if (!hasNext()) {
                throw new NoSuchElementException("Iterator exceeded.");
            }

            E ret = nextNodeToReturn.element;
            nextNodeToReturn = nextNodeToReturn.next;
            return ret;
        }

    }

    private LinkedListNode head;
    private LinkedListNode tail;

    public void add(E element) {
        LinkedListNode newnode = new LinkedListNode<>(element);

        if (head == null) {
            head = newnode;
            tail = newnode;
        } else {
            tail.next = newnode;
            tail = newnode;
        }
    }

    @Override
    public Iterator iterator() {
        return new LinkedListIterator();
    }

    public static void main(String[] args) {
        LinkedListCR list = new LinkedListCR<>();
        list.add("hello");
        list.add("world");
        list.add("y'all");

        for (String s : list) {
            System.out.print(s + " ");
        }

        System.out.println();
    }
}

Hope that helps.

StackExchange Code Review Q#141560, answer score: 6