Ошибка в некоторых моих функциях относительно списка C++

Question

Я использую ICircularDoubleDirectedList для структуры класса, тогда мой список наследует от этого и этого файла, вы можете найти дальше. Я использую файл, чтобы проверить глубокое копирование в списке, но дело в том, что он проходит весь тест, кроме того, который находится в конце, где предполагается удалить все данные.

Я проверил все мои функции, и они проходят тест, так что я понятия не имею, где я могу найти решение:/

Я знаю, что это может быть много, чтобы посмотреть и я не могу описать, как я был бы счастлив, если бы кто-то помог мне! : D

Это конструкция структуры для моего списка:

#ifndef ICDDLIST_H 
#define ICDDLIST_H 


template <typename T> 
class ICircularDoubleDirectedList 
{ 
public: 
    static enum direction{FORWARD, BACKWARD}; 
    virtual ~ICircularDoubleDirectedList() {};      //gjord 
    virtual void addAtCurrent(const T& element) = 0;    //gjord 
    virtual T& getElementAtCurrent() const = 0;      //gjord 
    virtual bool removeAtCurrent(const T& element) = 0;        
    virtual int size() const = 0;         //gjord 
    virtual void changeDirection() = 0;        //gjord  
    virtual void moveCurrent() = 0;         //gjord 
    virtual typename direction getCurrentDirection() const = 0;  //gjord 
}; 

#endif 

___________________________________________________________________________ 

Вот мое определение и декларации. Да, я знаю, что они должны быть в двух разных файлах заголовков и .cpp, но легче проверить, находятся ли они в одном и том же, я думаю.

#ifndef DOUBLELIST_H 
#define DOUBLELIST_H 

#include "ICircularDoubleDirectedList.h" 

using namespace std; 

template <typename T> 
class CircularDoubleDirectedList : public ICircularDoubleDirectedList <T>{ 

private: 

    class Node{ 
    public: 

     T data; 
     Node *neighbors[2]; 

     Node(const T& element); 
     Node(){}; 
     ~Node(){}; 


    }; 

    direction NodeDirection = FORWARD; 
    Node *current; 
    int sizeOfList; 

public: 

    CircularDoubleDirectedList(); 
    CircularDoubleDirectedList(const CircularDoubleDirectedList &other); 
    ~CircularDoubleDirectedList(); 

    virtual void addAtCurrent(const T& element); 

    T& getElementAtCurrent() const; 
    bool removeAtCurrent(const T& element); 
    int size() const; 
    void changeDirection(); 
    void moveCurrent(); 
    typename direction getCurrentDirection() const; 


    bool operator=(const CircularDoubleDirectedList &other); 


}; 




template <typename T> 
CircularDoubleDirectedList<T>::Node::Node(const T& element){ 

    data = element; 

} 

template <typename T> 
CircularDoubleDirectedList<T>::~CircularDoubleDirectedList(){ 

    if (this->size() != 0){ 
     while (0 < this->sizeOfList){ 
      this->removeAtCurrent(this->getElementAtCurrent()); 
     } 
    } 
    //this->current = nullptr; 
} 

template <typename T> 
CircularDoubleDirectedList<T>::CircularDoubleDirectedList(){ 

    NodeDirection = FORWARD; 
    sizeOfList = 0; 
    current = nullptr; 

} 

template <typename T> 
CircularDoubleDirectedList<T>::CircularDoubleDirectedList(const CircularDoubleDirectedList &other){ 

    this->NodeDirection = other.NodeDirection; 
    this->current = other.current; 
    this->sizeOfList = 0; 

    if (other.sizeOfList != 0){ 
     Node *counter = other.current; 

     for (int x = 0; x < other.sizeOfList; x++){ 
      counter = counter->neighbors[BACKWARD]; 
      this->addAtCurrent(counter->data); 
     } 
    } 
    else{ 
     this->current = nullptr; 
    } 

} 




template <typename T> 
void CircularDoubleDirectedList<T>::addAtCurrent(const T& element){ 

    Node *temp = new Node(element); 

    if (current == nullptr){ 
     current = temp; 
     temp->neighbors[FORWARD] = temp; 
     temp->neighbors[BACKWARD] = temp; 


    } 

    else{ 
     temp->neighbors[FORWARD] = current; 
     temp->neighbors[BACKWARD] = current->neighbors[BACKWARD]; 
     temp->neighbors[BACKWARD]->neighbors[FORWARD] = temp; 

     current->neighbors[BACKWARD] = temp; 

     current = temp; 
    } 

    ++sizeOfList; 

} 

template <typename T> 
T& CircularDoubleDirectedList<T>::getElementAtCurrent() const{ 

    if (sizeOfList <= 0){ 
     throw "Exeption: call of getElementAtCurrent on empty list"; 
    } 

    else{ 
     return current->data; 
    } 

} 



template <typename T>     //INTE FEL PÅ 
bool CircularDoubleDirectedList<T>::removeAtCurrent(const T& element){      
    bool success = false; 

    Node *temp = this->current; 

    int x = 0; 
    if(sizeOfList <= 0){ 
     throw "Exeption: call of removeAtCurrent on empty list"; 
    } 

    else if (sizeOfList==1 && current->data==element){ 
     delete current; 
     this->current = nullptr; 
     this->sizeOfList--; 
     success = true; 
    } 

    while(x<this->sizeOfList && success==false) { 
     if (temp->data == element){ 
      if (temp == this->current){ 
       this->moveCurrent(); 
      } 

      temp->neighbors[BACKWARD]->neighbors[FORWARD] = temp->neighbors[FORWARD]; 
      temp->neighbors[FORWARD]->neighbors[BACKWARD] = temp->neighbors[BACKWARD]; 

      delete temp; 
      this->sizeOfList--; 
      success = true; 


     } 
     else{ 
      temp = temp->neighbors[FORWARD]; 
     } 
     x++; 
    } 

    return success; 
} 

template <typename T> 
int CircularDoubleDirectedList<T>::size() const{ 

    return sizeOfList; 

} 

template <typename T> 
void CircularDoubleDirectedList<T>::changeDirection(){ 


    if (NodeDirection == FORWARD){ 
     NodeDirection = BACKWARD; 
    } 
    else 
     NodeDirection = FORWARD; 

} 


template <typename T> 
void CircularDoubleDirectedList<T>::moveCurrent(){ 

    if (NodeDirection == FORWARD && sizeOfList>0){ 
     current = current->neighbors[FORWARD]; 
    } 
    else if (NodeDirection == BACKWARD && sizeOfList>0){ 
     current = current->neighbors[BACKWARD]; 
    } 
    else{ 
     throw "Exception: call of moveCurrent on empty list"; 
    } 
} 

template <typename T> 
typename ICircularDoubleDirectedList<T>::direction CircularDoubleDirectedList<T>::getCurrentDirection() const{ 

    return NodeDirection; 

} 



template <typename T> //inte fel på 
bool CircularDoubleDirectedList<T>::operator=(const CircularDoubleDirectedList &other){ 


    if (&other == this){ 
     return true; 
    } 

    if (this->size() != 0){ 
     Node *temp1 = this->current; 
     T temp2; 

     while (0 < this->sizeOfList){ 
      temp2 = temp1->data; 
      temp1 = temp1->neighbors[FORWARD]; 
      this->removeAtCurrent(temp2); 
     } 
     this->current = nullptr; 
    } 

    this->NodeDirection = other.NodeDirection; 

    if (other.size() > 0){ 

     Node *counter = other.current; 

     for (int x = 0; x < other.size(); x++){ 
      counter = counter->neighbors[BACKWARD]; 
      this->addAtCurrent(counter->data); 
     } 
    } 
    else{ 
     this->current = nullptr; 
    } 

    return true; 

} 




#endif 

---

И это файл я использую, чтобы проверить мой список:

#include "CircularDoubleDirectedList.h" 
#include <iostream> 

using namespace std; 
template <typename T> 
void printList(CircularDoubleDirectedList<T>& list) 
{ 
    for (int i=0; i<list.size(); i++) 
    { 
     cout<<list.getElementAtCurrent()<<" "; 
     list.moveCurrent(); 
    } 
} 

template <typename T> 
void test(CircularDoubleDirectedList<T> list) 
{ 
    list.addAtCurrent(55); 
} 

int main() 
{ 
    _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); 
    CircularDoubleDirectedList<int> aList; 
    CircularDoubleDirectedList<int> bList = aList; 

    cout<<"******** Testing copy constructor on empty list ********"<<endl; 
    cout<<endl<<"Expected output: \nElements in aList: \nElements in bList"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in aList "; 
    printList(aList); 

    cout<<endl<<"Elements in bList "; 
    printList(bList); 
    cout<<endl; 
    system("pause"); 
    cout<<endl<<"******** Testing copy constructor on list with content ********"<<endl; 

    aList.addAtCurrent(10); 
    aList.addAtCurrent(20); 

    CircularDoubleDirectedList<int> cList = aList; 

    cout<<endl<<"Expected output: \nElements in aList 20 10\nElements in cList 20 10"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in aList "; 
    printList(aList); 

    cout<<endl<<"Elements in cList "; 
    printList(cList); 

    aList.removeAtCurrent(20); 
    cList.addAtCurrent(5); 

    cout<<endl<<endl<<"Expected output: \nElements in cList 5 20 10"<<endl<<endl<<"Your output: "<<endl; 

    test(cList); 

    cout<<"Elements in cList "; 
    printList(cList); 

    cout<<endl; 
    system("pause"); 

    CircularDoubleDirectedList<int> dList; 
    CircularDoubleDirectedList<int> eList; 


    cout<<endl<<endl<<"******** Testing assignment operator on empty list ********"<<endl; 
    dList = eList; 
    cout<<endl<<"Expected output: \nElements in dList \nElements in eList"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 

    cout<<endl<<"Elements in eList "; 
    printList(eList); 
    cout<<endl; 
    system("pause"); 

    cout<<endl<<endl<<"**** Testing assignment operator on list with content assigned empty list****"<<endl; 
    eList.addAtCurrent(20); 
    eList.addAtCurrent(10); 

    eList = dList; 

    cout<<endl<<"Expected output: \nElements in dList\nElements in eList"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 

    cout<<endl<<"Elements in eList "; 
    printList(eList); 
    cout<<endl; 
    system("pause"); 
    cout<<endl; 

    cout<<endl<<"***** Testing assignment operator on empty list assigned list with content *****"<<endl; 
    eList.addAtCurrent(20); 
    eList.addAtCurrent(10); 

    dList = eList; 

    cout<<"Expected output: \nElements in dList 10 20 \nElements in eList 10 20"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 

    cout<<endl<<"Elements in eList "; 
    printList(eList); 
    cout<<endl; 

    system("pause"); 

    dList.addAtCurrent(5); 
    eList.removeAtCurrent(20); 
    cout<<endl<<"Expected output: \nElements in dList 5 10 20 \nElements in eList 10"<<endl<<endl<<"Your output: "<<endl<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 

    cout<<endl<<"Elements in eList "; 
    printList(eList); 
    cout<<endl; 
    system("pause"); 

    cout<<endl<<"***** Testing assignment operator on lists with content *****"<<endl; 

    eList = dList; 

    cout<<"Expected output: \nElements in dList 5 10 20 \nElements in eList 5 10 20"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 

    cout<<endl<<"Elements in eList "; 
    printList(eList); 
    cout<<endl; 

    system("pause"); 

    eList.addAtCurrent(1); 
    dList.removeAtCurrent(10); 

    cout<<endl; 
    cout<<"Expected output: \nElements in dList 5 20 \nElements in eList 1 5 10 20"<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 

    cout<<endl<<"Elements in eList "; 
    printList(eList); 
    cout<<endl; 

    system("pause"); 

    cout<<endl<<"***** Testing assignment operator on a list assigned itself *****"<<endl; 

    dList = dList; 

    cout<<endl<<"Expected output: \nElements in dList 5 20 "<<endl<<endl<<"Your output: "<<endl; 
    cout<<"Elements in dList "; 
    printList(dList); 
    cout<<endl; 

    system("pause"); 


    cout<<endl<<"***** Testing destructor of list *****"<<endl; 


    ICircularDoubleDirectedList<int>* fList = new CircularDoubleDirectedList<int>(); 


    fList->addAtCurrent(100);   
    fList->addAtCurrent(50); 


    delete fList;    //NÅgot fel här 

    return 0; 
} 

Answer 1

Есть несколько проблем с вашим кодом

Первый стиль основе. Нет необходимости указывать this-> во всех строках, которые вы ссылаетесь на текущий объект. Делать это путает код и не нужно.

Во-вторых, вы написали свои операторы копирования/назначения неправильно и в плохом стиле. Тем не менее, я бы посоветовал написать свой конструктор копий, используя функцию addAtCurrent. Обычно я вижу людей, которые пишут конструктор копий со всеми видами логики указателя, дублируя код, который они уже записали в функции-члене, которую они написали. Вы не допустили этой ошибки, поэтому я благодарю вас за это.

Ваш конструктор копирования для CircularDoubledirectList не должен делать никаких заявлений if. Всякий раз, когда я вижу if инструкции в конструкторе копирования, который устанавливает красный флаг. Если в конструкторе копирования слишком много логики, основанной на решениях, есть хороший шанс, что то, что вы закончите, - это не настоящая копия, а полупеченная копия. Полупрозрачные копии, плавающие вокруг в программе, являются ошибками, которые очень трудно найти, поскольку это может вызвать неопределенное поведение.

Так вот переписывание конструктор копирования без использования if:

template <typename T> 
CircularDoubleDirectedList<T>::CircularDoubleDirectedList(const CircularDoubleDirectedList &other) : 
         NodeDirection(other.NodeDirection), 
         sizeOfList(0), current(nullptr) 
{ 
    Node *counter = other.current; 
    for (int x = 0; x < other.sizeOfList; ++x) 
    { 
     counter = counter->neighbors[BACKWARD]; 
     addAtCurrent(counter->data); 
    } 
} 

Код выше использует member initialization list для инициализации некоторых членов в их «пустые» значения. Обратите внимание, что цикл не проверяет размер - нет необходимости. Если размер other равен 0, то цикл не выполняется.

Теперь, учитывая вышеизложенное, давайте перепишем оператор присваивания в терминах конструктора копирования. Огромная проблема с вашей текущей реализацией оператора присваивания заключается в том, что вы возвращаете bool. То, что вы должны возвращать, является ссылкой на CircularDoubleDirectedList.

Вторая проблема с оператором присваивания состоит в том, что она избыточна. Весь этот код, который вы написали, уже находится в конструкторе копирования.Как я исправит это, нужно использовать copy/swap idiom, где вы бы использовали конструктор копирования для «помощи» с назначением.

Вот реализация с помощью этой идиомы:

#include <algorithm> 
//... 
template <typename T> 
class CircularDoubleDirectedList : public ICircularDoubleDirectedList <T>{ 
private: 
    void swap(CircularDoubleDirectedList<T>& left, 
       CircularDoubleDirectedList<T>& right); 
//... 
}; 
//... 
template <typename T> 
void CircularDoubleDirectedList<T>::swap(CircularDoubleDirectedList<T>& left, CircularDoubleDirectedList<T>& right) 
{ 
    std::swap(left.NodeDirection, right.NodeDirection); 
    std::swap(left.current, right.current); 
    std::swap(left.sizeOfList, right.sizeOfList); 
} 

template <typename T> 
CircularDoubleDirectedList<T>& CircularDoubleDirectedList<T>::operator=(const CircularDoubleDirectedList &other) 
{ 
    CircularDoubleDirectedList<T> temp(other); 
    swap(*this, temp); 
    return *this; 
} 

Так что здесь произошло? Ну, все, что я сделал, это использовать конструктор копирования для создания временного объекта. Затем я вызвал функцию swap для переключения переменных между временной копией и * этим. Затем я возвращаю текущий объект(). Это не только проверяет конструктор копирования, но и тестирует деструктор для temp. Поэтому, если в конструкторе или деструкторе копии есть ошибки, вы можете обнаружить их прямо здесь.

Я добавил функцию swap для вызова std::swap, которая переключает каждый из элементов.

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

С приведенными выше изменениями я не сталкивался с проблемами с повреждением памяти, и программа завершилась успешно. Не сказать, что где-то может быть ошибка (я не прошел логику добавления и удаления элементов), но у меня не было проблем с запуском кода после внесения изменений в вашу программу.