Deleting node in a double linked list is not working

Question

This is a basic function that takes an iterator position and deletes the node in this position but it gives me a runtime error. what am i doing wrong?

iterate erase(iterate position)
{
    iterate i;
    Node<T>* temp = head;
    if (head == NULL) {
        cout << "empty list" << endl;
    }
    else if (position.pointer == head) {
        head = temp->next;
        temp->next->previous = NULL;
        delete position.pointer;
    }
    else {
        while (temp != NULL) {
            if (temp == position.pointer->previous) {
                temp->next = position.pointer->next;
                temp->next->previous = temp;
                i.pointer = temp->next;
                delete position.pointer;
                return i;
            }
        }
    }

Answer 1

Your function is lacking adequate return statements. There are multiple flows that can cause the function to exit, but only one of them has a return statement. So the return value will largely be indeterminate, causing undefined behavior for any caller that tries to use the return value.

In any case, your while loop iterates forever, because you are not updating temp on each iteration of the loop. You also have a NULL pointer dereference if position is pointing at the last node in the list, as you are not checking the new temp->next for NULL before accessing temp->next->previous.

But, you really don't need the while loop at all. The thing about a double-linked list is that, given any node in the list, you have direct access to the nodes that are surrounding it on both sides. So there is no need to iterate the list hunting for nodes.

Try something more like this instead:

iterate erase(iterate position)
{
    Node<T> *temp = position.pointer;
    if (!temp) return end();

    Node<T> *next = temp->next;
    Node<T> *previous = temp->previous;

    if (next) next->previous = previous;
    if (previous) previous->next = next;
    if (temp == head) head = next;
    //if (temp == tail) tail = previous;
    delete temp;

    iterate i;
    i.pointer = next;
    return i;
}

Alternatively:

iterate erase(iterate position)
{
    Node<T> *temp = position.pointer;
    if (!temp) return end();

    Node<T> *dummy; // <-- only if no tail ...
    Node<T> **previous = (temp->next) ? &(temp->next->previous) : &dummy/*&tail*/;
    Node<T> **next = (temp->previous) ? &(temp->previous->next) : &head;

    *previous = temp->previous;
    *next = temp->next;
    delete temp;

    iterate i;
    i.pointer = *next;
    return i;
}