How to fix NullPointerException? - java doubly linked list

Question

I'm implementing a doubly linked list with a Previous() function that raises a NullPointerException, I would appreciate help if someone knows how to fix this. The error is in line 37 of the parent class (DoublyLinkedList).

I'm a beginner taking (a beating from) a course on data structures and java. A friend helped me by saying the following "It says that your null pointer exception is at your .Previous() function [line 18]. I looked at your Previous() function within your DoublyLinkedList class and it seems that you don't check if current == null before testing if current != head. YOu must have a null case so that you don't get a nullpointer like in your case. If your current == null, you should set it to the tail, otherwise keep the rest of our code." I tried fixing that but still have the same problem

class Node
{
    // Public references to previous and next nodes
    public Node next;
    public Node previous;

    // Private data field
    Object data;

    public Node(Object data)
    {
        this.data = data;
    }

    public Object GetData()
    {
        return data;
    }
}

class DoublyLinkedList
{
    Node head;
    Node tail;
    Node current;
    Node next;
    Node previous; 

    // Go to the first element in the list 
    public void Head()
    {
        current = head;
        previous = null;    
    }

    //Go to the last element in the list
    public void Tail()
    {
        current = tail;
        next = null;
    }

    // Go to the next element if the current element is not past-the-end
    public void Next()
    {
        if (current != null)
        {
            previous = current;
            current = current.next;
        }
    }

    public void Previous()
    {
        if (current == null && current != head)
            next = null;
            Tail();
        if (current != head)
        {
            next = current;
            current = current.previous;
        }
    }

    // Return the data associated with the current element
    // or null if the current element is past-the end
    public Object Get()
    {
        return current == null ? null : current.GetData();
    }

    // Insert a new element before the current element
    // or at the end if the current element is past-the-end
    public void Insert(Object data)
    {
        // Create new node
        Node node = new Node(data);

        // Set 'next' reference of new node
        node.next = current;

        // Set 'next' reference for 'previous' node.
        // Treat the special case where the current node is the head.
        if (current == head)
            head = node;
        else
            previous.next = node;

        // Update current node
        current = node;
    }   

    public void Print()
    {
        // Traverse list
        for (Head(); Get() != null; Next())
            System.out.println(Get());
    }
}

class Test
{
    public static void main(String args[])
    {
        // Create list
        DoublyLinkedList doubly_linked_list = new DoublyLinkedList();

        // Insert elements at the end
        doubly_linked_list.Insert("a");
        doubly_linked_list.Next();
        doubly_linked_list.Insert("b");
        doubly_linked_list.Next();
        doubly_linked_list.Insert("c");
        doubly_linked_list.Next();

        // Set second-to-last as current element. Insert string "d"
        doubly_linked_list.Tail();
        doubly_linked_list.Previous();
        doubly_linked_list.Insert("d");

        // Set current element as past-the-end. Insert string "e"
        doubly_linked_list.Tail();
        doubly_linked_list.Next();
        doubly_linked_list.Insert("e");

        // Print
        doubly_linked_list.Print();
    }
}

Answer 1

Your friend is incorrect. If current were null, that is not an obstacle to comparing it to something else. As you have the line right now, it is only true if the current equals null AND it is not equal to head. That's what you're testing, so you're never going to enter the then clause unless it is null and head isn't.

Also, I'm not sure what you were trying to do there, but either your indenting is incorrect or you are missing some curly braces. The call to Tail() there will always be called whether or not the if above it is true.

Finally, the real problem with your design is that you have conflated two things - the concept of a List and the concept of the thing that walks through it one element at a time, which (in Java, at least) is called an Iterator. You should have a separate class representing the Iterator, which has a concept of the current place where it is looking in the list, and the idea of previous and next as it moves through the list. Imagine if you wanted to have two different pointers moving through the list at the same time (say, to find duplicates). With your approach, that would be impossible. If Iterator is a separate class, you could always make two of them.

OK. Given that you will separate out the Iterator functionality into a separate class, what does that mean for the insert process? The answer is that the only insert operations you can do on the List itself is insertFront and insertBack. Then the Iterator class has an insert, as well, which means insert in the current location.

You have next and previous pointers in Node, which is perfect. That's the only place, though, that needs next and previous pointers. The List itself needs first and last pointers, and the Iterator only needs a current pointer. When it is asked to move to next or previous from where it is, then it consults current to get that value. So the only member data in iterator are to the List itself and to its "current" Node. (It needs the List pointer because its insert function might need to call the List's insertFront or insertBack methods, if you call insert when it is pointing at the first or off the list. The List has to do those, because it has to change its own first or last pointer.)

Be clear in your own mind what it means for the Iterator if current is null. If it is null, does that mean you've fallen off the end with too many next calls, can it happen from falling off the front with too many previous calls, or what? One solution is never to allow it to fall off the front. Add the methods hasNext and hasPrevious and declare that it is an error to call previous when you are pointing at the head element.

Finally, you should follow the Java convention that method names start with lowercase.