Search code examples
javadoubly-linked-list

double linked list method in java


this method for the double linked list in java runs the while loop once then then gets stuck and doesn't continue

    public static DoubleLinkedList reverse(DoubleLinkedList DL) {
    DoubleLinkedList result = new DoubleLinkedList();
    Node tempDL = DL.head;
    if(DL.head==null || DL.head.next==null){
        result.head=DL.head;
        result.tail = DL.head;
    }else{
        while(tempDL != null){
            result.insertNode(tempDL.data, 0);
            tempDL = tempDL.next;
        }
    }
    return result;
}

this is the insert node method for reference:

public void insertNode(int data, int pos) {
    Node newNode = new Node(data);
    if (head == null)
        head = tail = newNode;
    if (pos > length())
        return;
    if (pos == 0) { // comment #1
        newNode.next = head;
        head.prev = newNode;
        head = newNode;

    } else {
        Node temp = head;
        for (int i = 0; i < pos - 1 && temp.next != null; i++)
            temp = temp.next;

        if (temp.next == null) {
            tail.next = newNode;
            newNode.prev = tail;
            tail = newNode;
        } else {
            newNode.prev = temp;
            newNode.next = temp.next;
            temp.next.prev = newNode;
            temp.next = newNode;
        }
    }
}

as you see in comment #1 here is the part that works when i called the method in the reverse method

i tried making the tail of the new list go backwards while the head of the main list go forwards but then it mirrors itself as follows:

main list: 1, 2, 3, 4, 5

after calling:

1, 2, 3, 2, 1


Solution

  • Your insertNode method creates a cycle. This is quite evident when you run the code with a debugger, stepping through the code...

    When the list is empty and pos is 0, then the following is executed:

        if (head == null)
            head = tail = newNode;
    

    So far so good, but execution continues here:

        if (pos == 0) {
            newNode.next = head;
            head.prev = newNode;
            head = newNode;
        }
    

    This is not good: a cycle is created as now head.next == head. The first if should include a return:

        if (head == null) {
            head = tail = newNode;
            return;
        }
    

    It would then also make more sense to put the length check at the top of your function, so it is always executed. And if it is true, there is no reason to have a node instance created:

        if (pos > length())
            return;
    

    Not your question, but there is also another remark to make: the reverse method should not make the new list reference nodes from the original list, like is happening in the first if block. This will lead to undesired behaviour when one of the two lists will get a new node: then depending on the position of that new node, the other list will or will not have it.

    So I suggest you remove that case, and always create the nodes for the new list like you do in the general case:

    public static DoubleLinkedList reverse(DoubleLinkedList DL) {
        DoubleLinkedList result = new DoubleLinkedList();
        Node tempDL = DL.head;
        while(tempDL != null){
            result.insertNode(tempDL.data, 0);
            tempDL = tempDL.next;
        }
        return result;
    }