Как правильно освободить память, выделенную связанным списком?

Question

Я пишу программу, которая использует связанный список, и в моем коде я создаю указатель на объект, используя «новое» ключевое слово. Насколько я понимаю, в любое время, когда вы используете новое ключевое слово, вы также должны иметь удаление, и мне интересно, правильно ли я делаю это.

#include"gameClass.h" 
#include"card.h" 
#include"list.h" 
#include"node.h" 

using namespace std; 

List::List() 
{ 
    head = NULL; 
} 

List::~List() 
{ 
    delete head; 
} 

void List::add(Card* tmpCard) 
{ 
    if (head == NULL) 
{ 
    Node* tmpNode; 
    tmpNode = new Node; 
    tmpNode->setNext(NULL); 
    tmpNode->setData(tmpCard); 
    head = tmpNode; 
    delete tmpNode; 
} 
else 
{ 
    Node* tmpNode; 
    tmpNode = new Node; 
    tmpNode->setNext(head->getNext()); 
    tmpNode->setData(tmpCard); 
    head = tmpNode; 
    delete tmpNode; 
} 
} 

Card * List::remove() 
{ 
    if (head != NULL) 
{ 
    //Card* tmpCard; 
    Node* tmpNode; 

    tmpNode = new Node; 
    tmpNode->setNext(head->getNext()); 
    tmpNode->setData(head->getData()); 
    head->setNext(tmpNode->getNext()); 
    delete tmpNode; 
} 
    return nullptr; 
} 

Answer 1

Вы делаете это совершенно неправильно.

• Вы внедряете новую head, а затем delete новый head немедленно. Это очень плохо.
• Ваша remove() функция практически ничего не делает. Он создает новый Node, устанавливает для него некоторый параметр, затем извлекает набор данных, который предполагается для него, и delete новый Node.

Попробуйте это:

#include"gameClass.h" 
#include"card.h" 
#include"list.h" 
#include"node.h" 

using namespace std; 

List::List() 
{ 
    head = NULL; 
} 

List::~List() 
{ 
    // delete all node currently have instead of only head 
    while (head != NULL) 
    { 
     Node* tmpNode = head; 
     delete head; 
     head = tmpNode; 
    } 
} 

void List::add(Card* tmpCard) 
{ 
    if (head == NULL) 
    { 
     Node* tmpNode; 
     tmpNode = new Node; 
     tmpNode->setNext(NULL); 
     tmpNode->setData(tmpCard); 
     head = tmpNode; 
     // do not delete the new node here! 
    } 
    else 
    { 
     Node* tmpNode; 
     tmpNode = new Node; 
     tmpNode->setNext(head); // the new head should be linked to current head, not the next node of current head 
     tmpNode->setData(tmpCard); 
     head = tmpNode; 
     // do not delete the new node here! 
    } 
} 

Card * List::remove() 
{ 
    if (head != NULL) 
    { 
     //Card* tmpCard; 
     Node* tmpNode; 

     // no creating new nodes to remove the first node 
     tmpNode = head->getNext(); // remember where the next node is 
     delete head; // remove the head 
     head = tmpNode; // move the head 
    } 
    return nullptr; 
} 

UPDATE: Если вы хотите вернуть указатель на карту в узле должны быть удалены, функция remove() будет выглядеть следующим образом:

Card * List::remove() 
{ 
    if (head != NULL) 
    { 
     Card* tmpCard; 
     Node* tmpNode; 

     tmpCard = head->getData(); // remember where the card pointed by head is 
     tmpNode = head->getNext(); // remember where the next node is 
     delete head; // remove the head 
     head = tmpNode; // move the head 
     return tmpCard; // return the card 
    } 
    return nullptr; // there are no cards in this list 
} 

Answer 2

Чтобы избежать проблем, просто используйте std::shared_ptr<Node>. Указатель будет освобождать то, на что указывает, как только он выходит из области видимости (если на него нет другого общего указателя).

Если вы хотите попрактиковаться в использовании нового/удалить, вы не поняли, что он делает: вы создаете новый узел, используя new, а затем каким-то образом работаете с этим узлом. Только когда узел больше не задан, вы его удаляете. Сейчас происходит следующее:

Node* tmpNode; 
tmpNode = new Node;  // You get a new Node on heap. 
tmpNode->setNext(NULL);  
tmpNode->setData(tmpCard); 
head = tmpNode;   // Head now points to the new Node. All is well. 
delete tmpNode;   // The Node both tmpNode and head point to is deleted! 

Теперь, как голова и tmpNode содержат указатель на нераспределенную память. Когда вы получаете доступ к нему, вы получаете неопределенное поведение. Вы, вероятно, хотите удалить все узлы в деструкторе List и нигде больше.