CODEWITHSUNDEEP
c++linked-list
HQuser
HQuser

Reputation: 640

Deleting a specific value from singly linked list?

I've been trying to delete a node if the data matches a specific value.

Here's my delete value method:

void del_value(int data)
{
    Node *temp = head;

    int i = 0;

    while(temp!=NULL)
    {
        if(temp->data == data)
        {
            del_index(i);
            i--; // Since the nodes count will be reduced after deleting, reducing the index by one.
        }
        i++;

        temp = temp->next;
    }
}

And here is my del_index method(Which is working correctly):

    int getCount()
{
    int i = -1;

    Node *temp = head;

    while(temp!=NULL)
    {
        temp = temp->next;
        i++;
    }

    return i;
}

void del_index(int pos)
{
    int count = getCount();

    if(pos == 0)
    {
        del_start();
    }
    else if(pos == count)
    {
        del_last();
    }
    else if(pos<0 || pos>count)
    {
        cout<<"Out of range"<<endl;
        return;
    }
    else
    {
        int i = 1;

        Node *temp = head;

        while(i<pos)
        {
            temp = temp->next;
            i++;
        }

        Node *toDel = temp->next;
        Node *forward = toDel->next;

        temp->next = forward;
        delete toDel;
    }
}

And here is my main method:

int main()
{
    Mylist l;

    l.add_start(4);
    l.add_start(4);
    l.add_start(4);

    l.del_value(4);

    l.show();
}

But it stucks when it reaches del_value method inside loop. Any idea where am I missing?

Update: (Added del_first and del_last methods

void del_start()
{
    if(head == NULL)
    {
        cout<<"List is empty"<<endl;
        return;
    }
    else
    {
        Node *temp = head;
        head = head->next;
        delete temp;
    }
}

void del_last()
{
    if(head == NULL)
    {
        cout<<"List is empty"<<endl;
        return;
    }
    else
    {
        Node *temp = head;

        while(temp->next != NULL)
        {
            tail = temp;
            temp = temp->next;
        }

        tail->next = NULL;
        delete temp;
    }
}

Upvotes: 0

Views: 989

Answers (1)

nfactorial
nfactorial

Reputation: 187

Your del_value method will not work because you delete the object being pointed to by 'temp' then you dereference it after (with "temp = temp->next").

For your example code, I would cache the 'next' value before your conditional, for example:

Node *next = temp->next;
if(temp->data == data)
{
    del_index(i);
    i--;
}

i++;
temp = next;

I am assuming you're doing this for practice purposes but I would add the following suggestions:

I wouldn't call del_index here but remove the node inline, within the del_value method. As you have the necessary pointers to be able to remove it. del_index has to traverse your list a second time.

I would also recommend using the containers within the stl instead of rolling your own to avoid encountering issues like this.

Upvotes: 1

Related Questions