CODEWITHSUNDEEP
c++linked-list
user1816546
user1816546

Reputation: 313

Deleting a node in a linked list

Hi I'm trying to delete a node in a linked list. I am first experimenting on how to delete the head and the tail nodes. The head deletion seems to work, however the tail the deletion does not. When I run the code, the place where the tail used to be is replaced with garbage values. Can anyone figure out why? Many thanks!

void CList :: Remove() {

    int data = NULL;

    std::cout<<"Enter value you wish to remove ";
    std:: cin>> data;

    cNode *pMyPointer = m_pHead;

    while (pMyPointer != NULL)
    {
        if (pMyPointer->m_nValue == data) {
            std::cout << "Element found";
            goto del;
        }

        else {
            pMyPointer = pMyPointer->m_pNext;
        }   
    }

    del:

    //removing the head
    if (pMyPointer == m_pHead)
        m_pHead= m_pHead->m_pNext;
    //removing the tail
    else if (pMyPointer == m_pTail)
        m_pTail = m_pTail->m_pPrev;

    delete pMyPointer;
}

Upvotes: 2

Views: 364

Answers (4)

Ashalynd
Ashalynd

Reputation: 12573

And what if your tail and head pointer are the same? You don't check for it. Therefore you might be deleting the pointer which you assume to be the Head, which is also a Tail. Plus what if it's Next for a Head or Prev for a Tail?

void CList :: Remove() {

    int data = NULL;

    std::cout<<"Enter value you wish to remove ";
    std:: cin>> data;

    cNode *pMyPointer = m_pHead;

    while (pMyPointer != NULL)
    {
        if (pMyPointer->m_nValue == data) {
            std::cout << "Element found";
            goto del;
        }

        else {
            pMyPointer = pMyPointer->m_pNext;
        }   
    }

    del:

     //taking care of the neighbors
    if (pMyPointer->m_pPrev)
        pMyPointer->m_pPrev->m_pNext = pMyPointer->m_pNext;
    if (pMyPointer->m_pNext)
        pMyPointer->m_pNext->m_pPrev = pMyPointer->m_pPrev;
    // removing the head
    if (pMyPointer == m_pHead)
        m_pHead= m_pHead->m_pNext;
    //removing the tail
    if (pMyPointer == m_pTail)
        m_pTail = m_pTail->m_pPrev;

    delete pMyPointer;
}

Upvotes: 0

MAG
MAG

Reputation: 3075

consider node_1 points to node_2 (just a 2 node case) Just look at this code 

else if (pMyPointer == m_pTail)
        m_pTail = m_pTail->m_pPrev;

node_1 points to node_2 . It still points there . once you deleted node_2 , node_1 will still point to node_2 (or garbage once node_2 is deleted) & so you must make sure node_1 points to NULL . ie last but one should point to null .

something like 

else if (pMyPointer == m_pTail)
    m_pTail->m_pPrev->next=NULL;
    m_pTail = m_pTail->m_pPrev;

Upvotes: 2

Abhishek Bansal
Abhishek Bansal

Reputation: 12705

With this statement

 while (pMyPointer != NULL)

Your pointer may be pointing to NULL when it exits the loop and hence it will skip the tail pointer.

Instead try

while (pMyPointer->m_pNext != NULL)

You also need to make the second last node point to NULL.

else if (pMyPointer == m_pTail) {
  m_pTail = m_pTail->m_pPrev;
  m_pTail->m_pNext = NULL;
}
delete pMyPointer;

Also, instead of goto del, just use break;

Upvotes: 1

Nick N.
Nick N.

Reputation: 13578

Stay one node ahead of the node you want to delete

Upvotes: 0

Related Questions