C++ Deleting a node in Queue

c++linked-list

Problem1: Deleting in a node in a list > 3

Description:

Deletion of the sixth node in a list of seven, results in a print of only the first
and last node.

Available Node Pointers:
*next_, *prev_, *data_

Function to delete the specified node is in the LinkedList.cpp
Name: DeleteNode.

Function that traverses through the list to print the nodes is in main.cpp
Name: PrintAllNodes

Possible Solution:

Being able to access Current->prev_ in main when traversing to print the nodes.

Code:

void LinkedList::DeleteNode( Node* node )
{
    Node *Current = first_; // I want this to be my only Node Ptr Varaible Declaration.
    if ( NULL == first_ )
        std::cout << "Cannot delete from an empty list: \n";
//TRAVERSING WAS/IS A BAD IDEA.............................
    while ( Current != NULL )
    {
        if ( Current->data_ == node->data_ )
        {
            //If Current isn't the head of the list, set prev to next
            if ( Current != first_ )
            {
                Current->prev_        = first_; //statement that follows crashes if this is not assigned.
                Current->prev_->next_ = Current->next_;
            }
            else
            {
                first_ = Current->next_;
                if ( first_ != NULL )
                first_->prev_ = NULL;
            }

            //If Current isn't the tail of the list, set next to prev
            if ( Current->next_ != NULL )
                Current->next_ = Current->prev_;

            else if ( Current->prev_ != NULL )
            Current->prev_->next_ = NULL;

          listLen_--;
          delete Current;
          Current = NULL;
        }
        else
        {
            Current->prev_ = Current;
            Current = Current->next_;
        }
    }
    return;
}

Code for PrintAllNodes in main.cpp:

void PrintAllNodes( LinkedList *LinkedObject, long length = 0 )
{
    const char *Names = NULL; 
    length = LinkedObject->GetListLength();
    Node *GetNode = LinkedObject->GetFirstNode();

    for ( signed short x = 0; x < length; x++ )
    {
        Names = static_cast< NameObject* >( GetNode->data_ )->GetName();
        cout << Names << endl;
        GetNode = GetNode->next_; // traversing
    }
    return;
}

Best Solution

This is your problem:

Current->prev_  = first_;

What you're doing is disconnecting all nodes before current and connecting the first to the last! (the seventh in your case)

What you should do there is only:

Current->prev_->next_ = Current->next_;
Current->next_->prev_ = Current->prev_; //I think you forgot this
delete Current;
Current = NULL;

If without the

Current->prev_  = first_;

you get a crash, that's because your Current->prev_ isn't assigned well. But assigning it to first_ is not the solution. You should check your other methods (maybe the AddNode) to see why your Current->prev_ is bad.

Related Question