Деструктор связанного списка - PullRequest
1 голос
/ 04 июля 2011

Я изучаю C ++ в свое свободное время и пишу связанный список, чтобы попытаться освоить его.Я беспокоюсь о том, как я пришел, чтобы удалить объект.Это односвязный список.Вот деструктор:

template <typename T>
LinkedList<T>::~LinkedList() 
{
  Node<T> * current = this->first;
  do {
    Node * temp = current->next;
    delete current; // THIS JUST MIGHT BE A TERRIBLE IDEA!!!
    Node * current = temp; // new current-- might work with the current
                           // delete a line above
  } while (current->next != 0); // need to leave this->last so that I don't
                                // delete it twice in the next line.
                                // Just realized I'm deleting this->first, then 
                                // in the next line [implicitly] deleting it again!
                                // 
  delete this;
}

Я создаю указатель на первый узел в списке, создаю временный указатель на следующий узел, удаляю первый указатель, создаю новый указатель с тем же именем, который затемвозвращается назадПосле этого он удаляет указатель «this».

Я уверен, вы понимаете, почему я обеспокоен тем, как я создаю новый указатель с тем же именем, что и удаленный указатель.

Ответы [ 4 ]

1 голос
/ 04 июля 2011

Я не вижу вопроса, но вижу много ошибок:

template <typename T>
LinkedList<T>::~LinkedList() 
{
    Node<T>* current = this->first; // you never check if this->first is 0
    do {
        Node * temp = current->next;
        delete current; // THIS is not a problem
        Node * current = temp; /* this line has no effect -
                     you define new variable and it disappears when reaches
                     the end of its scope next line */
    } while (current->next != 0); /* 'current' is always 'this->first' but
              even if you would assign it 'temp' like you intended few lines
              above you never check if 'current' is 0 so you will
              dereference 0 when you reach the end of the list */

    delete this; /* this line is total nonsense
            if your LinkedList is created with 'new LinkedList' then
            you have infinite recursion (you call destructor from destructor)
            otherwise you 'delete' pointer that you never 'new'-ed */
}

Правильный код:

template <typename T>
LinkedList<T>::~LinkedList() 
{
    Node<T>* current = this->first;
    while (current != 0)
    {
        Node<T>* temp = current->next;
        delete current;
        current = temp;
    }
}
1 голос
/ 04 июля 2011
  1. Не delete this в деструкторе.
  2. Если Node является шаблоном, вам нужно написать Node<T> во всех этих определениях.
  3. Не переопределяйте current, просто присвойте ему новое значение.

Кроме этого, я не вижу других проблем в этом фрагменте.

1 голос
/ 04 июля 2011

Почему бы не скомпилировать код, попробовать и посмотреть, что получится?Худшее, что может случиться, - это сбой вашей программы, и вы должны выяснить, почему.

Ваш код должен в основном работать, за исключением того, что вам нужно проверить current в состоянии цикла while вместо current->nextи излишне (и, вероятно, неправильно) писать delete this в деструкторе, и есть еще несколько ошибок, которые Cat Plus Plus указал в своем ответе.

Если вы пытаетесь изучать C ++, тогдавам следует больше узнать об этом, чтобы понять, какие ошибки вы допустили, и уверены, что исправленный код будет работать.

Вот моя фиксированная версия функции:

template <typename T> LinkedList<T>::~LinkedList() 
{
    Node<T> * current = this->first;
    while(current != 0) {
        Node<T> * temp = current->next;
        delete current;
        current = temp;
    }
    delete this;
}
0 голосов
/ 04 июля 2011
~LinkedList
{
//...
  delete this;
}

delete this; в деструкторе похоже на код-самоубийство .Ваш объект уже разрушается, а вы снова разрушаетесь с помощью delete this;.Это неопределенное поведение.Вы можете удалить это.В остальном все выглядит хорошо (при условии, что this->first дает голову Node).

Редактировать : я пропустил, вы переопределили current.Удалить это.(должно быть просто current = temp;)

...