deleting an item in Doubly-linked list

Question

my program is supposed to do 3 operations: 1.insert 2.delete 3.show using a Doubly-linked list... I have a problem in delete function. here is the code:

void List::del()
{
    int num;
    Node *Aux1=first;
    Node *Aux2=NULL;
    if(isempty())
        cout<<"List is Empty!"<<endl;

    else
    {
        cout<<"Enter the number that you want to DELETE:"<<endl;
        cin>>num;
        while(Aux1->info!=num && Aux1 != NULL)
        {
            Aux2=Aux1;
            Aux1=Aux1->left;
        }
        if(Aux1!=NULL)
        {
            if(Aux2!=NULL)
            {
                if(Aux1->left==NULL)
                    Aux2->left=NULL;
                else
                {
                Aux2->left=Aux1->left;
                Aux1=Aux1->left;
                Aux1->right=Aux2;
                }
            }
            else
            {
                first=Aux1->left;
                //first->right=NULL;
            }
        }
        system("pause");
    }
}

In delete function I want to find the number that the user wants to remove and then remove it from the list... the problem is when the user enters a number that does not exist in the list!! In this case, I want my program not to do nothing on the list and not to remove any item from the list. but when it happens, I encounter this error:

what's wrong with my code? ;-?

Answer 1

Your while loop is dereferencing the NULL pointer before it checks if it is NULL. You need to change your loop to this:

while(Aux1 != NULL && Aux1->info!=num)

Answer 2

The problematic part is the following part:

    while(Aux1->info!=num && Aux1 != NULL)
    {
        Aux2=Aux1;
        Aux1=Aux1->left;
    }

You dereference Aux1 before checking if it is NULL. Hence, you will get a segmentation fault in this loop.

However, there is a more serious problem with your data structure.

Firstly, your field names are a bit weird. Usually, doubly linked lists have a prev and next pointer, for the preceding and following element, respectively.

Secondly, you don't actually count the items but rely on their info field. But you never update this field, so after a while, this ID will become meaningless and fragmented.

Consider the case where a user has entered five nodes:

0 --> 1 --> 2 --> 3 --> 4

Now you call delete(2):

0 --> 1 --> 3 --> 4

Now your user has to either get all the IDs presented and choose the right one, in which case your segmentation fault will occur -- what happens now if the user calls delete(2) again? Usually, one would not delete nodes by magic ID number, but instead have a function that takes a pointer to the node which needs to be deleted.

NodePayload* delete(Node* node)

This would then delete the appropriate node and optionally return a pointer to the actually deleted node/or its payload. Because usually for memory-management reasons, your List class might allocate Nodes, but not their payloads, so users can use the List with their allocated custom payloads.