Можно ли использовать «удалить это» для удаления текущего объекта? - PullRequest
7 голосов
/ 11 августа 2009

Я пишу связанный список и хочу, чтобы деструктор структуры (структура Node) просто удалил себя и не имел побочных эффектов. Я хочу, чтобы деструктор моего списка итеративно вызывал деструктор Node для себя (временно сохраняя следующий узел), например:

//my list class has first and last pointers
//and my nodes each have a pointer to the previous and next
//node
DoublyLinkedList::~DoublyLinkedList
{
    Node *temp = first();

    while (temp->next() != NULL)
    {
        delete temp;
        temp = temp->next();
    }
}

Так что это будет мой деструктор Node:

Node::~Node
{
   delete this;
}

Это приемлемо, особенно в этом контексте?

Ответы [ 8 ]

17 голосов
/ 11 августа 2009

Если вызывается деструктор Node, то он уже находится в процессе удаления. Таким образом, удаление не имеет смысла в вашем деструкторе Node.

Также это неправильно:

while (temp->next() != NULL)
{
     delete temp;
     temp = temp->next();
}

Вместо этого вы должны получить temp-> next () во временной переменной. В противном случае вы получаете доступ к удаленной памяти.

Так что больше похоже на это:

DoublyLinkedList::~DoublyLinkedList
{
  Node *temp = first();
  while (temp != NULL)
  {
       Node *temp2 = temp->next();
       delete temp;
       temp = temp2;
  }
}
4 голосов
/ 11 августа 2009

В настоящее время ваш код может вызвать нарушение прав доступа, поскольку вторая из следующих строк явно обращается к освобожденной памяти:

delete temp;
temp = temp->next();

Если вы хотите рекурсивно удалить структуру, вам нужно что-то вроде этого:

DoublyLinkedList::~DoublyLinkedList
{
    Node *temp = first();
    delete temp;
}

Node::~Node
{
   if(this->next() != NULL) delete this->next();
}
4 голосов
/ 11 августа 2009

Нет, вы не должны delete this от деструктора. Деструктор вызывается из-за оператора delete (или выходит из области видимости), и это, скорее всего, приведет к некоторой аварии

У вас также есть пара проблем в деструкторе DoublyLinkedList. Во-первых, вы удаляете temp, а затем получаете доступ к temp после ее удаления. Во-вторых, код фактически не удалит последний элемент в связанном списке.

2 голосов
/ 11 августа 2009

Прежде всего: я действительно, очень надеюсь, что это домашнее задание, назначенное вам для того, чтобы понять двусвязный список. В противном случае нет причин использовать это вместо std::list. С этим из пути:

Нет, delete this в dtor всегда неверен, так как dtor вызывается, когда this находится в состоянии удаления.

Также пока

delete temp;
temp = temp->next();

между прочим может работать, это, безусловно, неправильно, поскольку, когда вы пытаетесь получить доступ к temp->next(), temp уже удален, поэтому вы должны вызвать функцию-член для него. Это вызывает так называемое «неопределенное поведение». (Короче говоря: он может делать то, что вы хотите, но он также может терпеть неудачу всегда или время от времени, или только тогда, когда пятница, 13-е, сталкивается с новолунием. Это также может вызвать на вас очень неприятных носовых демонов. )

Обратите внимание, что вы можете решить обе проблемы, удалив следующий узел в dtor вашего узла:

Node::~Node()
{
   delete next();
}

Таким образом, ваш список dtor тоже становится очень простым:

DoublyLinkedList::~DoublyLinkedList()
{
    delete first();
}

Мне кажется, для этого и были изобретены dtors, поэтому, за исключением того факта, что в наше время никто больше не должен писать свои собственные типы связанных списков, мне кажется, что это решение C ++ вашей проблемы ,

1 голос
/ 11 августа 2009

удали это; вызовет деструктор текущего объекта. В этом случае, если вы звоните, удалите это; в деструкторе деструктор будет вызываться бесконечно до краха.

0 голосов
/ 11 августа 2009

И то, и другое никогда не должно быть сделано.

Это

DoublyLinkedList::~DoublyLinkedList
{
    Node *temp = first();
    while (temp->next() != NULL)
    {
        delete temp;
        temp = temp->next();
    }
}

вызовет неопределенное поведение - вам не разрешен доступ к памяти, которую вы вернули в кучу. Вместо этого должно быть:

DoublyLinkedList::~DoublyLinkedList
{
    Node *temp = first();
    while( temp != NULL)
    {
        Node* next = temp->next();
        delete temp;
        temp = next;
    }
}

Вызов delete this приведет к так называемому двойному освобождению, что также приведет к неопределенному поведению. Деструктор должен вызывать delete только для переменных-указателей, но не для this. Вызов delete this из других методов оправдан для освобождения текущего объекта, но не из деструктора.

0 голосов
/ 11 августа 2009

В общем, деструктор должен просто беспокоиться об удалении (или освобождении, если вы используете C или malloc) любой памяти, выделенной специально для вашего объекта. Удаление указателя на ваш объект всегда будет управляться ОС, и вам не нужно беспокоиться об этой части.

Стоит помнить одну вещь: при создании вы сначала создаете объект (когда поток управления входит в тело конструктора), а затем объекты внутри; для уничтожения вы должны сделать это в обратном порядке, потому что, если вы сначала удалите внешний объект, у вас не будет доступа к внутренним указателям, чтобы удалить их. Вместо этого вы удаляете внутренние объекты, используя деструктор, затем ОС управляет фактическим освобождением памяти, когда поток управления выпадает из деструктора.

Кстати, то же самое происходит с подклассами: если у вас есть класс A и класс B: public A, то когда вы делаете новый B (), сначала выполняется конструктор A, затем конструктор B; при уничтожении сначала выполняется деструктор B, а затем A. Я вполне уверен, что вам не нужно беспокоиться об этом, хотя - C ++ позаботится об этом за вас. Поэтому не пытайтесь найти способ вызвать delete в суперклассе.

0 голосов
/ 11 августа 2009

Код выше вызовет Node :: ~ Node () дважды. (в "delete temp" и в Node :: ~ Node ())

Node :: ~ Node () не должен вызывать "delete this" (иначе ваша программа аварийно завершится)

пс. Цикл while в вашем коде не будет работать. Будет разыменован неверный указатель. Сначала вы должны скопировать значение temp-> next, а затем уничтожить указатель temp.

...