CODEWITHSUNDEEP
c++linked-list
Lauren
Lauren

Reputation: 1

C++ removing node from back --singly linked list

Why won't my code delete the last element of the linked list? I create a current pointer to transverse through my list and break out of the loop..(next is the point that is within my struct called Card_Node). It should be simple to answer, just not sure why it won't delete the last node in the list"

    Card_Node *current; 
    current = front;
    while ( current->next->next != NULL){
    {   
        current = current-> next;
    }   
    Card a = current->next->card;
    return a;
    delete current->next;
    current->next = NULL;
}

Upvotes: 0

Views: 106

Answers (3)

4386427
4386427

Reputation: 44274

return current->next->card;   // return !!
delete current->next;         // so this will never be executed
current->next = NULL;

Update

As the comment below ask for further input, here is an update where I tried to keep the original principles.

if (front == nullptr)  // Special handling of empty list
{
    // Nothing to return - add error handling - throw exception perhaps
    // or:
    return ???; // A default card perhaps
}
if (front->next == nullptr)  // Special handling of list with one element
{
    // Only one element
    Card a = front->card;
    delete front;
    front = nullptr;
    return a;
}

Card_Node *current; 
current = front;
while ( current->next->next != NULL)  // Iterate to find last element
{   
    current = current-> next;
}

// Now current->next is last element, i.e. the one to remove   
Card a = current->next->card;
delete current->next;
current->next = NULL;
return a;

Upvotes: 1

Remy Lebeau
Remy Lebeau

Reputation: 595557

There are several problems with your code:

  1. You are not taking into account if the list contains less than two nodes.
  2. You are calling return before delete, thus skipping delete.
  3. You have one too many open braces in your while loop.
  4. You are not setting the previous node's next pointer to NULL when there are two or more nodes in the list.
  5. You not setting front to NULL if there is only one node in the list.

Try this instead:

if (!front) {
    // no cards in the list, do something...
    return Card();
}
Card_Node *current = front;
Card_Node *previous = NULL;
while (current->next != NULL) {
    previous = current;
    current = current->next;
}   
Card a = current->card;
delete current;
if (previous != NULL) {
    previous->next = NULL;
}
if (front == current) {
    front = NULL;
}

Upvotes: 0

1201ProgramAlarm
1201ProgramAlarm

Reputation: 32732

You're checking for NULL in two different ways; this should be done only once. If you think about what your code does when your list only has one element in it (walk thru it in the debugger or on paper), then you should realize what the problem is.

Upvotes: 1

Related Questions