How to delete a specific value from a doubly linked list?

Question

I was given a task with a DOUBLY linked list to delete a specific number from the list. My code is giving an Access Violation error. Even after multiple dry runs, I can't figure out what is wrong. The task basically is to create a search function which finds a specific number in the linked list, and a deletion function which deletes that specific link.

node* search(int val){
    node* cur=head;
    while(cur!=NULL){
        if(cur->data==val){
            cout<<"value found "<<val<<endl;
            return cur;
        }
        cur=cur->next;
    }
    cout<<"value not exist"<<endl;
    return NULL;
}

bool delspval(int val){
    node*temp=0;
    if(search(val)==NULL){
        return 0;
    }
    else{
        temp=search(val);
        temp->prev->next=temp->next;
        delete temp;
        temp=0;
        cout<<"specific value "<<val<<" deleted"<<endl;
        return 1;
    }
}

In the above given code, the line temp->prev->next=temp->next; is giving the error. I'm pretty much a beginner at linked lists, so any help would be appreciated.

minimal working code:

#include<iostream>
using namespace std;
class dll{
    struct node{
        int data;
        node *next,*prev;
    };
    node *head;
public:
    dll(){
        head=NULL;
    }
    void inatst(int val){
        node *temp=new node;
        temp->data=val;
        temp->next=head;
        head=temp;
    }
    node* search(int val){
        node* cur=head;
        while(cur!=NULL){
            if(cur->data==val){
                cout<<"value found "<<val<<endl;
                return cur;
            }
            cur=cur->next;
        }
        cout<<"value not exist"<<endl;
                    return NULL;
    }
    bool delspval(int val){
        node*temp=0;
        if(search(val)==NULL){
            return 0;
        }
        else{
            temp=search(val);
            temp->prev->next=temp->next;
            delete temp;
            temp=0;
            cout<<"specific value "<<val<<" deleted"<<endl;
            return 1;
                }
            }
    void display(){
        node*cur=head;
        while(cur!=NULL){
            cout<<cur->data<<" ";
            cur=cur->next;
        }
        cout<<endl;
    }
    ~dll(){
        while(head!=NULL){
            node*cur=head;
            head=cur->next;
            delete cur;
            cur=head;
        }
    }
};
void main(){
    dll l1;
    l1.inatst(1);
    l1.inatst(2);
    l1.inatst(3);
    l1.inatst(4);
    l1.inatst(5);
    l1.inatst(6);
    l1.display();
    l1.delspval(3);
    system("pause");
}

Welcome! Please post a Minimal Reproducible Example as text, the shortest complete code that shows what you have tried. The best way to do that is by copy/paste, after you check it compiles and does exhibit the behaviour described. May I suggest you take the Tour and read How do I ask a good question? — Weather Vane
– Weather Vane, Commented Sep 16, 2022 at 18:07
Please don’t use all caps in heading, it amounts to shouting. — DailyLearner
– DailyLearner, Commented Sep 16, 2022 at 18:07
In temp->next->prev you don't check if next is NULL. That would give you an access error. Aside: there is an inefficiency from calling search(val) twice. — Weather Vane
– Weather Vane, Commented Sep 16, 2022 at 18:16
Make the list circular or use a dummy element to avoid that error-prone and inefficient nullptr special case. Next, a doubly linked list must have two links updated. First: temp->next->prev = temp->prev; (Never assign a next into a prev like you did!) Second: temp->prev->next = temp->next; Also, always run it under valgrind; it will tell you when exactly you start using invalid pointers. — Andrej Podzimek
– Andrej Podzimek, Commented Sep 16, 2022 at 18:18
Unrelated: One of the best tools when debugging a linked list is the pencil and paper. Draw the list. redraw the list at every step of a transformation, making absolutely certain that you do not lose a connection before you no longer need it. Then use the pictures as the basis for your code or as a the set of expectations while debugging. If you step through the code with a debugger, redrawing according to what the debugger does as you step, can you draw the exact same pictures? If not, where the pictures diverge is probably your bug. — user4581301
– user4581301, Commented Sep 16, 2022 at 18:22

Vlad from Moscow · Accepted Answer · 2022-09-16 18:41:31Z

3

For starters, the search() function is being called twice within the delspval() function:

if(search(val)==NULL){

and

temp=search(val);

that makes the delspval() function less efficient.

This statement:

temp->next->prev=temp->next;

does not make sense.

The delspval() function can be defined in the following way. I suppose that the class contains only one pointer to the head node. If the class contains also a pointer to the tail node, then the function below must be modified.

bool delspval( int val )
{
    node *temp = search( val );
    bool success = temp != nullptr;

    if ( success )
    {
        if ( temp->next != nullptr )
        {
            temp->next->prev = temp->prev;
        }
        // If the class has a pointer to the tail node
        //   then uncomment the else part  
        /*
        else
        {
            tail = temp->prev;
        }
        */

        if ( temp->prev != nullptr )
        {
            temp->prev->next = temp->next;
        }
        else
        {
            head = temp->next;
        }

        delete temp;
    }

    return success;
}

edited Sep 16, 2022 at 18:41

answered Sep 16, 2022 at 18:21

Vlad from Moscow

313k27 gold badges204 silver badges358 bronze badges

Sign up to request clarification or add additional context in comments.

10 Comments

hadi khan Over a year ago

there was a mistake in the above given code temp->next->prev=temp->next; was supposed to be temp->prev->next=temp->next; i was trying different things and forgot to change but still it is giving access violation error .... and the inefficiency for using the function twice is noted thanks <3

Vlad from Moscow Over a year ago

@hadikhan Do you have a pointer to the tail node in the class?

hadi khan Over a year ago

no i have int data , node*next,*prev in node struct and node *head in link class

Vlad from Moscow Over a year ago

@hadikhan If so then you have a mistake in some other functions.

hadi khan Over a year ago

but if i exclude this funtion in the main body the program works perfectly fine

|

gnasher729 · Accepted Answer · 2022-09-16 19:12:10Z

0

You should absolutely not call search twice. You basically double the runtime.

But the solution if the value was found: If tmp->next is not null then it points to tmp, but it now needs to the item before tmp. And if tmp->prev is not then it points to tmp, but it needs to point to the item after tmp.

And finally, if tmp = head then head must be changed to the item after tmp.

Tmp = search()
If tmp == null then return
Prev = tmp->prev
Next = tmp->next
If prev ≠null then prev->next = next
If next ≠ null then next->prev = prev
If head = tmp then head = next. 
Delete tmp.

answered Sep 16, 2022 at 19:12

gnasher729

53k5 gold badges83 silver badges106 bronze badges

Collectives™ on Stack Overflow

How to delete a specific value from a doubly linked list?

2 Answers 2

10 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

10 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related