Search code examples
data-structureslinked-listdoubly-linked-list

can't debug this code of deleting node from the doubly linked list


I am trying to write code for deleting a node from a doubly linked list, but something is going wrong. I can't seem to delete the first node, while any node from the middle or end node is being deleted. Also, my head and the tail pointer are not being updated after deleting the respective node positions. code -

class Node{
    public:
    int data;
    Node* prev;
    Node* next;

    //constructor 
    Node(int d) {
        this -> data = d;
        this -> prev = NULL;
        this -> next = NULL;
    }

    // destructor
    ~Node() {
        int val = this -> data;
        if(next != NULL) {
            delete next;
            next = NULL;
        }
        cout << "memory free for node with data " << val << endl;
    }
};

void deleteNode(int position, Node* head) {

    //deleting the first node
    if(position == 1) {
        Node* temp = head;
        temp -> next -> prev = NULL;
        head = temp -> next;
        temp -> next = NULL;
        delete temp;

    } else {
        // deleteing middle or last node
        Node* curr = head;
        Node* prev = NULL;

        int cnt = 1;
        while(cnt < position) {
            prev = curr;
            curr = curr -> next;
            cnt++;
        }

        curr -> prev = NULL;
        prev -> next = curr -> next;
        curr -> next = NULL;
        delete curr;
    }
}
int main() {

    Node* head = NULL;
    Node* tail = NULL;
    insertAtHead(head, tail, 11);
  
    insertAtHead(head, tail, 18);
    insertAtHead(head, tail, 9);
    print(head);
    insertAtTail(head, tail, 21);
    insertAtPosition(head, tail, 2, 34);
    print(head);

    deleteNode(1, head);
  
    cout << "head - " << head -> data << endl;
    cout << "tail - " << tail -> data << endl;
    print(head);

Solution

  • A few issues:

    • deleteNode might delete the tail node, and so may need to adapt the value of tail. This never happens. tail should be an argument.

    • deleteNode currently has code that modifies head, but this will only affect the local variable with that name, not the caller's variable. So this parameter should be call-by-reference: &head. Same for the missing &tail parameter.

    • There are unsafe references, like temp -> next -> prev, when temp -> next could be nullptr.

    • In the else case there is code missing to set curr->next->prev = prev

    Assuming that the provided position is a valid one, the code could be corrected to this:

    void deleteNode(int position, Node* &head, Node* &tail) {
        //deleting the first node
        if(position == 1) {
            Node* temp = head;
            if (temp -> next != nullptr) {
                temp -> next -> prev = nullptr;
            } else {
                tail = nullptr;
            }
            head = temp -> next;
            temp -> next = nullptr;
            delete temp;
        } else {
            // deleteing middle or last node
            Node* curr = head;
            Node* prev = nullptr;
    
            int cnt = 1;
            while(cnt < position) {
                prev = curr;
                curr = curr -> next;
                cnt++;
            }
    
            prev -> next = curr -> next;
            if (curr->next != nullptr) {
                curr->next->prev = prev;
            } else {
                tail = prev;
            }
            prev -> next = curr -> next;
            curr -> prev = NULL;
            curr -> next = NULL;
            delete curr;
        }
    }