Iterator for a linkedlist

Question

My project should implement two classes. A basic linked list and a sorted linked list. Everything seems to be working fine except for some reason I can't iterate through the sorted linked list. The class structure is as follows:

public class BasicLinkedList<T> implements Iterable<T> {
    public int size;

    private class Node {
        private T data;
        private Node next;

        private Node(T data) {
            this.data = data;
            next = null;
        }
    }

    private Node head;
    private Node tail;

    public BasicLinkedList() {
        head = tail = null;
    }
//Add, remove method 

public Iterator<T> iterator() {
        return new Iterator<T>() {

            Node current = head;

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

            @Override
            public T next() {
                if(hasNext()){
                    T data = current.data;
                    current = current.next;
                    return data;
                }
                return null;
            }

            @Override
            public void remove(){
                throw new UnsupportedOperationException("Remove not implemented.");
            }

        };

Now when I test this class it works just fine. The iterator works and I can test it all. The problem is in the sorted linked list class which extends this one. Here's its implementation and a comparator class that I'm using in the constructor:

public class SortedLinkedList<T> extends BasicLinkedList<T>{
    private class Node{
        private T data;
        private Node next;

        private Node(T data){
            this.data = data;
            next = null;
        }
    }

    private Node head;
    private Node tail;
    private Comparator<T> comp;

    public SortedLinkedList(Comparator<T> comparator){
        super();
        this.comp = comparator;
    }

Here's the comparator class and the test I ran in a separate class:

public class intComparator implements Comparator<Integer>{
        @Override
        public int compare(Integer o1, Integer o2) {
            return o1 - o2;
        }
}

public static void main(String[] args) {
System.out.println("---------------SortedLinkedList--------------");
        SortedLinkedList<Integer> sortedList = new SortedLinkedList<Integer>(new intComparator());
        sortedList.add(3);
        sortedList.add(5);
        sortedList.add(2);
        for(int i: sortedList){
            System.out.println(i);
        }
    }   

Nothing prints out. I assumed the iterator that was inherited would help me traverse this no problem and clearly its legal because the for-each loop compiles. It's just that nothing gets printed out. I debugged it and all the adding, removing stuff works as expected. It's just that the iterator isn't doing what it's supposed to. Should I create a separate new iterator for this class? But wouldn't that be redundant code since I already inherit it? Help appreciated!

EDIT: Here's the add method for the sorted list

public SortedLinkedList<T> add(T element){
        Node n = new Node(element);
        Node prev = null, curr = head;
        if(head == null){
            head = n;
            tail = n;
        }
        //See if the element goes at the very front
        else if(comp.compare(n.data, curr.data) <= 0){
            n.next = head;
            head = n;
        }
        //See if the element is to be inserted at the very end
        else if(comp.compare(n.data, tail.data)>=0){
            tail.next = n;
            tail = n;
        }
        //If element is to be inserted in the middle
        else{
            while(comp.compare(n.data, curr.data) > 0){
                prev = curr;
                curr = curr.next;
            }
            prev.next = n;
            n.next = curr;
        }

        size++;
        return this;
    }

Solution

1) SortedLinkedList extends BasicLinkedList but both have

private Node head; 
private Node tail

this is wrong. If you want to inherit those field in the sub class, you should mark the variables as protected in the super class and remove them from the subclass.

2) Same goes for private class Node. You are declaring the Node class in both the SortedLinkedList and BasicLinkedList. What you should do is declare it once, (maybe in the super class?) and use the same class in both places. If you do this, the constructor, and the fields should be accessible to both classes. So you will have to change the access modifier (private is what you have now).

I will post below code that works, but I haven't spent any time on the design. Just posting it to demonstrate how you could change the code to make it work. You will have to decide which access modifiers to use and where to put the classes.

import java.util.Comparator;
import java.util.Iterator;

public class Test {
    public static void main(String[] args) {
        System.out.println("---------------SortedLinkedList--------------");
        SortedLinkedList<Integer> sortedList = new SortedLinkedList<Integer>(new intComparator());
        sortedList.add(3);
        sortedList.add(5);
        sortedList.add(2);
        for (int i : sortedList) {
            System.out.println(i);
        }
    }
}

class BasicLinkedList<T> implements Iterable<T> {
    public int size;

    class Node {
        T data;
        Node next;

        Node(T data) {
            this.data = data;
            next = null;
        }
    }

    protected Node head;
    protected Node tail;

    public BasicLinkedList() {
        head = tail = null;
    }

    // Add, remove method

    public Iterator<T> iterator() {
        return new Iterator<T>() {

            Node current = head;

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

            @Override
            public T next() {
                if (hasNext()) {
                    T data = current.data;
                    current = current.next;
                    return data;
                }
                return null;
            }

            @Override
            public void remove() {
                throw new UnsupportedOperationException("Remove not implemented.");
            }

        };

    }
}

class SortedLinkedList<T> extends BasicLinkedList<T> {


    private Comparator<T> comp;

    public SortedLinkedList(Comparator<T> comparator) {
        super();
        this.comp = comparator;
    }

    public SortedLinkedList<T> add(T element) {
        Node n = new Node(element);
        Node prev = null, curr = head;
        if (head == null) {
            head = n;
            tail = n;
        }
        // See if the element goes at the very front
        else if (comp.compare(n.data, curr.data) <= 0) {
            n.next = head;
            head = n;
        }
        // See if the element is to be inserted at the very end
        else if (comp.compare(n.data, tail.data) >= 0) {
            tail.next = n;
            tail = n;
        }
        // If element is to be inserted in the middle
        else {
            while (comp.compare(n.data, curr.data) > 0) {
                prev = curr;
                curr = curr.next;
            }
            prev.next = n;
            n.next = curr;
        }

        size++;
        return this;
    }
}

class intComparator implements Comparator<Integer> {
    @Override
    public int compare(Integer o1, Integer o2) {
        return o1 - o2;
    }
}

Comments

[move_slow_break_things]

Perfect. Thanks. That did the trick. I was just mindlessly copy-pasting instead of thinking about inheritance.

[Gangnus]

Upvoting both question and answer. A very nice example on inheritance.