Клонирование односвязного списка с использованием перегруженного оператора присваивания

Question

Я пытаюсь реализовать класс одиночно связанных списков в C++. Я перегрузил оператор присваивания, чтобы клонировать список. Сам клонирование, похоже, работает нормально, но программа вылетает во время удаления клонированного списка, что заставляет меня подозревать, что что-то не так во время процедуры копирования.

Любая помощь очень ценится. Вот код для перегруженного оператора =:

DLList& DLList::operator=(const DLList& iList) 
{ 
    Node *t = iList.head(); 
    while(t != NULL) 
    { 
     push(t->data); 
     t = t->next; 
    } 
    return *this; 
} 

Вот код для операции нажимной:

void DLList::push(int data) 
{ 
    Node *n = new Node; 
    n->data = data; 
    n->next = NULL; 
    //n->random = NULL; 
    n->next = _head; 
    _head = n; 
    //cout << "_head" <<_head->data<< "head->next" << (_head->next ? _head->next->data : 0)<<endl; 
} 

Вот основной код (где авария происходит):

DLList *d = new DLList(); 
d->push(10); 
d->push(20); 
d->push(30); 
d->push(40); 
d->setRandom(); 
d->display("Original list"); // Displays the original list fine 
DLList *d1 = new DLList(); 
d1 = d; 
d1->display("Cloned list"); //Displays the cloned list fine 
delete d; // works fine 
delete d1; // Crashes here due to segmentation fault, invalid pointer access during delete called as part of destructor) 

Код для деструктора (если это помогает):

~DLList() 
{ 
    while(_head != NULL) 
    { 
     Node *temp = _head; 
     _head = _head->next; 
     delete temp; // crashes here during first iteration for the cloned list 
    } 
} 

Answer 1

вы фактически не используете оператор присваивания! Вы просто копируете указатель на другой указатель. отладку кода (или просто добавить следы) бы показала, что

1. вы не называли свою копию кода
2. адресом d и d1 являются теми же

Итак, когда вы удалите во втором списке вы дважды удаляете один и тот же адрес.

Вам не нужно new здесь:

DLList d; 
d.push(10); 

DLList d1 = d; // assignment is REALLY called 
d1.display("Cloned list"); 

объекты будут удалены, когда вы выходите за рамки ваших переменных

Если вы хотите проверить в вашем контексте, сохраняя new , изменить

d1 = d; 

по

*d1 = *d; 

для активации оператора присваивания.

Еще один совет: факторизуйте свой код копирования, чтобы он был разделен между оператором присваивания и конструктором копирования, а код удаления должен делиться между деструктором и оператором присваивания (чтобы избежать утечки памяти при назначении дважды): не проверено, не пылать меня, если он не компилируется, просто скажите мне, что я буду это исправить, я уже overanswering здесь :)

DLList& DLList::operator=(const DLList& iList) 
{ 
    destroy(); // or you'll get memory leaks if assignment done twice 
    copy(iList); 
    return *this; 
} 
DLList& DLList::DLList(const DLList& iList) 
{ 
    _head = NULL; // set _head to NULL in all your constructors!! 
    copy(iList);  
} 
~DLList() 
{ 
    destroy(); 
} 

void DLList::copy(const DLList& iList) // should be private 
{ 
    Node *t = iList.head(); 
    while(t != NULL) 
    { 
     push(t->data); 
     t = t->next; 
    } 
} 
void DLList::destroy() // private 
{ 
    while(_head != NULL) 
    { 
     Node *temp = _head; 
     _head = _head->next; 
     delete temp; // crashes here during first iteration for the cloned list 
    } 
    _head = NULL; // calling destroy twice is harmless 
    } 

Answer 2

d1 = d; нЕ копирование DLList. Вместо этого вы просто делаете d1 так же, как и указатель d. Итак, оба d1 и d указывают на тот же список. В конце вашей программы вы дважды удалите этот список и сделаете сбои в работе программы.

Answer 3

Ваша проблема заключается в заявлении

d1 = d; 

который делает назначение указателя, и делает d1 точку на тот же объект, как d. Последующие операторы delete вызывают один и тот же объект (*d). Это неопределенное поведение (одним из симптомов является крах, как вы описываете).

Вышеупомянутое также не вызывает конструктор копирования DLList. Если это ваша цель, вы должны сделать

*d1 = *d; 

, что делает объект, на который указывает d1 копию одного, на который указывает d. В этом случае также подходят два оператора delete.

Answer 4

Если вы хотите использовать оператор присваивания, вам нужно разыменовать указатели. Смотрите здесь:

DLList *d1 = new DLList(); 
d1 = d; // <------------- This line 
d1->display("Cloned list"); //Displays the cloned list fine 

На выделенной линии, вы устанавливаете указательd1, чтобы указать на то же место, как d. Это означает, что когда вы вызываете delete в конце вашего кода, вы пытаетесь удалить один и тот же объект (d в этом случае) дважды.

Чтобы исправить код, вы должны разыменовать ваши указатели:

DLList *d1 = new DLList(); 
*d1 = *d; // <------------- Note the dereference applied to BOTH lists. 
d1->display("Cloned list"); //Displays the cloned list fine 

Кроме того, следует избегать использования указателей полностью, если вы можете. Для вашего тривиального примера вы можете просто создать свои объекты напрямую.