Doubly Linked List Java Remove method does not work for any node besides head node

Question

All of my methods work except for the remove method. Although it only works when the node to be removed is the head, it does not work for any other node to be removed after the head. After debugging for hours, I found out that the removing something other than the head only works when 'head' itself is referenced (not through storing the head value in a variable) such as head.next, head.next.next, etc. But statements such as curNode.next.prev = curNode.prev seem to not change the order of the list at all. Attached below is my entire doubly linked class as well as the node class, and a main method I use to check the code.

Doubly Linked Node Class

public class DLNode {
    Object data;
    DLNode next;
    DLNode prev;

    DLNode(Object o) {
        data = o;
        next = null;
        prev = null;
    }
    public String toString() {
        return "[" + data + "]";
    }

}

Doubly Linked List Class

public class DLList {
    DLNode head;
    DLList(){
        head = null;
    }
    public void append(DLNode newNode) {
        if (head == null ) {
            head = newNode;
        }
        else {
            DLNode curNode = head;
            while (curNode.next != null) {
                curNode.prev = curNode;
                curNode = curNode.next;             
            }
            curNode.next = newNode;
            curNode.prev = newNode.prev;
        }

    }
    public void prepend(DLNode newNode) {
        if (head == null) {
            head.prev = newNode;
            head = newNode;
            newNode.prev = null;
        }
        else {
            DLNode n = head;
            head.prev = newNode;
            head = newNode;
            newNode.next = n;
            newNode.prev = null;

        }
    }
    public void insertAfter(DLNode curNode, DLNode newNode) {
        DLNode sucNode = head.next;
        if (head == null) {
            head = newNode;
        }
        else {
            sucNode = curNode.next;
            newNode.next = sucNode;
            newNode.prev = curNode;
            curNode.next = newNode;
            sucNode.prev = newNode;
        }

    }
    public void remove(DLNode curNode) {
        DLNode sucNode = curNode.next; //these variables don't seem to work without
        DLNode predNode = curNode.prev; //the head node directly referenced
        if (head == null) {
            curNode = null;
        }
        else if (curNode == head) { //only block that works
            head = sucNode;
        }
        else if (sucNode != null) {
            sucNode.prev = predNode; //where the problem is apparently
        }
        else if (predNode != null) {
            predNode.next = sucNode;
        }

    }
    public DLNode search(Object key) {
        DLNode curNode = head;
        while (curNode != null) {
            if (curNode.data == key) {
                return curNode;
            }
            curNode = curNode.next;
        }
        return curNode;
    }

    public void insertAfterNew(DLNode curNode, DLNode newNode) {
        DLNode sucNode = head.next;
        if (head == null) {
            head = newNode;
        }
        else if (curNode == null) {
            newNode.next = sucNode;
            head = newNode;
            sucNode.prev = newNode;
        }
        else {
            sucNode = curNode.next;
            newNode.next = sucNode;
            newNode.prev = curNode;
            curNode.next = newNode;
            sucNode.prev = newNode;
        }
    }

    public String toString() {
        String finalString = "X<-";
        DLNode curNode = head;
        if (head == null) {
            return "X";
        }
        while (curNode != null) {
            if (curNode.next == null) {
                finalString += curNode;
                curNode = curNode.next;
            }
            else {  
                finalString += curNode + "<=>";
                curNode = curNode.next;
            }
        }
        return finalString + "->X";
    }
    }

Test Class with main

public class TestList {
    static final int N = 4;

    public static void main(String[] args) {
        testDLList();
    }

    static void testDLList() {

        System.out.println("Doubly-Linked List");

        DLList list2 = new DLList();

        for (int i = 0; i < N; i++) 
            list2.append(new DLNode(i));
        for (double d = N; d < 2 * N; d++)
            list2.append(new DLNode(d));

        System.out.println(list2);

        DLNode temp = list2.search(1); //remove works when the value in search is 0
        System.out.println(temp); // since 0 is head, but not with other values in list

        list2.insertAfter(temp, new DLNode(2000));
        System.out.println(list2);

        list2.remove(temp);
        System.out.println(list2);

        System.out.println();
    }

Answer 1

You have to apply user7's answer together with the suggestions I made in my comment.

public void append(DLNode newNode) {
    if (head == null ) {
        head = newNode;
    }
    else {
        DLNode curNode = head;
        while (curNode.next != null)
            curNode = curNode.next; 
        curNode.next = newNode;
        newNode.prev = curNode;
    }

}

public void remove(DLNode curNode) {
    DLNode sucNode = curNode.next;
    DLNode predNode = curNode.prev;
    if (sucNode != null)
        sucNode.prev = predNode;
    if (predNode != null)
        predNode.next = sucNode;
    else if (curNode == head)
        head = sucNode;
    else
        throw new IllegalArgumentException();

}

See user7's answer for the explanation on the changes to append(). As for remove(), there are four scenarios:

• If the list looks like predNode <-> curNode <-> sucNode, i.e. predNode != null && sucNode != null, you need to set sucNode.prev = predNode and predNode.next = sucNode
• If the list looks like predNode <-> curNode, i.e. predNode != null && sucNode == null, you need to set predNode.next = null
• If the list looks like curNode <-> sucNode, i.e. predNode == null && sucNode != null, you need to set sucNode.prev = null and head = sucNode
• If the list looks like curNode, i.e. predNode == null && sucNode == null, you need to set head = null

The fixed remove() method handles these four cases correctly and also sanity checks that curNode == head when predNode == null.

---

Your insertAfter() is also broken. For one thing, the head == null check is useless because head.next will always throw a NullPointerException before the head == null check is reached. I'm not quite sure I follow the intended logic, but here's a fixed version and an explanation:

public void insertAfter(DLNode curNode, DLNode newNode) {
    DLNode sucNode;

    if (curNode == null) {
        sucNode = head;
        head = newNode;
    } else {
        sucNode = curNode.next;
        curNode.next = newNode;
    }
    newNode.prev = curNode;

    newNode.next = sucNode;
    if (sucNode != null)
        sucNode.prev = newNode;
}

• If the list is empty, i.e. curNode == null && head == null, you need to set newNode.prev = null, newNode.next = null, and head = newNode
• If newNode should be inserted at the head of the list, i.e. curNode == null && head != null, you need to set newNode.prev = null, newNode.next = head, and head = newNode
• If newNode is being appended to the tail of the list, i.e. curNode != null && curNode.next == null, you need to set newNode.prev = curNode, newNode.next = null, and curNode.next = newNode
• If newNode is being inserted between two nodes, i.e. curNode != null && curNode.next != null, you need to set newNode.prev = curNode, newNode.next = curNode.next, and curNode.next = newNode

---

Your prepend() is also broken since head.prev will always throw a NullPointerException in the head == null branch. The head != null branch works correctly but uses an unnecessary temporary variable. Here's a fixed version:

public void prepend(DLNode newNode) {
    if (head == null) {
        newNode.next = null;
        head = newNode;
        newNode.prev = null;
    } else {
        head.prev = newNode;
        newNode.next = head;
        newNode.prev = null;
        head = newNode;
    }
}

Answer 2

The problem is in your append. I have commented the lines to be removed with the reason and also added few ones.

public void append(DLNode newNode) {
    if (head == null ) {
        head = newNode;
    }
    else {
        DLNode curNode = head;
        while (curNode.next != null) {
           // curNode.prev = curNode; Shouldn't be updating the prev when traversing
            curNode = curNode.next;     //Traverse         
        }
        curNode.next = newNode;
        newNode.prev = curNode;
        //curNode.prev = newNode.prev; This also must be removed. This cuts off the nodes from the list
    }
 }