I use, the ICircularDoubleDirectedList for a class structure, then my List inherits from this and that file can you find further down. I use a file to test the deep copying on the list, but the thing is that it passes all the test except the one at the end where it's supposed to delete all the data.
I have tested all my functions and they pass the test so I have no clue where I can find the solution :/
I know it might be a lot to look at and I can't really describe how happy I would be if someone would help me out! :D
This is the design structure for my List:
#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
___________________________________________________________________________
Here is my definition and declarations. Yes, I know they should be in two different header and .cpp files, but it's easier to test if they are in the same I think.
#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
And this is the file I use to test my List:
#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;
}
There are several issues with your code
The first is style-based. There is no need to specify this-> in all the lines you are referring to the current object. Doing so muddles the code up and is not necessary.
Second you've written your copy/assignment operators wrong and in bad style. However, I would give you credit for writing your copy constructor by using the addAtCurrent function. Usually I see persons writing the copy constructor with all sorts of pointer logic, duplicating the code they've already written in the add member function they wrote. You didn't make that mistake, so I commend you for that.
Your copy constructor for CircularDoubledirectList does not need to do any if statements. Whenever I see if statements in a copy constructor, that sets off a red flag. If there is too much decision-based logic within a copy constructor, there is a good chance that what you wind up with is not a real copy, but a half-baked copy. Half-baked copies floating around in a program are bugs that are very hard to find, as it can cause undefined behavior to occur.
So here is a rewrite of the copy constructor without using 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);
}
}
The code above makes use of the member initialization list to initialize certain members to their "empty" values. Note that the loop doesn't check for size -- there is no need to. If the size of other is 0, then the loop doesn't execute.
Now given the above, let's rewrite the assignment operator in terms of the copy constructor. The huge issue with your current implementation of the assignment operator is that you're returning bool. What you should be returning is a reference to a CircularDoubleDirectedList.
The second issue with the assignment operator is that it is redundant. All of that code you wrote is already in the copy constructor. The way I would fix it is to use the copy/swap idiom, where you would be using the copy constructor to "help" with the assignment.
Here is an implementation of using this 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;
}
So what happened here? Well, all I did was to use the copy constructor to create a temporary object. Then I called the swap function to switch the variables between the temp copy and *this. Then I return the current object (this). This not only tests the copy constructor, the destructor for temp is also tested. So if there are any bugs in the copy constructor or destructor, you may be able to detect them right here.
I added a swap function to call std::swap that switches each of the members.
Since your code tests the assignment operator, and you did not implement it correctly with the proper return type, please change your code to the above and test again.
With the changes above, I did not come across any memory corruption issues and the program completed successfully. Not to say there may not be a bug somewhere (I didn't go through the logic of adding and removing items), but I had no issues running the code after the changes applied to your program was made.