0
votes

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;
}
3
Changed it but it still won't delete it..... 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; } - Lauren
Is there a reason why you are not using std::list? - Remy Lebeau

3 Answers

1
votes
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;
1
votes

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.

0
votes

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;
}