I am trying to make a copy constructor for an Object that contains a pointer, that pointer refers to other pointers etc. The following code is a Binary Tree.
BTree.h
{
public:
vertex* root;
BTree()
{
root = NULL;
};
~BTree() {
delete root;
root = nullptr;
};
BTree(const BTree& p_BTree) //copy constructor
{
root = new vertex(*p_BTree.root);
}
BTree& operator= (const BTree& other) //assignment operator
{
// Delete the existing class A instance
delete root;
// and create a new as a copy of other.attribute
root = new vertex(*other.root);
}
Node.h
class vertex{
public:
int key;
string data;
vertex* leftChild;
vertex* rightChild;
vertex* parent;
int height;
vertex(string data){
key = ID;
this->data = data;
leftChild = NULL;
rightChild = NULL;
parent = NULL;
ID ;
};
vertex(){
key = 0;
leftChild = NULL;
rightChild = NULL;
parent = NULL;
};
vertex(vertex* node){
key = ID;
leftChild = node->leftChild;
rightChild = node->rightChild;
parent = node->parent;
};
~vertex(){
delete leftChild;
delete rightChild;
leftChild = nullptr;
rightChild = nullptr;
};
void print(string indent){
string indent2 = indent;
cout <<indent << " " << data <<endl;
if(leftChild != nullptr || rightChild != nullptr)
{
if(leftChild != nullptr){
indent = "|" indent;
leftChild->print(indent);
}else{
cout << indent << endl;
}
if(rightChild != nullptr){
indent2 = "|" indent2;
rightChild->print(indent2);
}else{
cout << indent2 << endl;
}
}
}
};
#include "BTree.h"
int main() {
// Aufgabe 1
BTree B;
B.main();
// Aufgabe 2
BTree C = B; //Call copy constructor
C.print();
// Aufgabe 3
BST D;
D.main();
D.print(D.root);
D.sortvector(); //neu hinzugefügt
// Aufgabe 4
D.PrintLayers(D.root, 1);
}
The problem is that when the destructor is called, the program crashes because it tries to free memory that has already been freed.
Root in Object B and C (in main) have a different memory address, the problem is the leftchilds and the rightchilds in Object C. These are copied shallow and not deep. I don't know how to do this in the copy constructor for these attributes.
This is how it looks in the Debugger:
CodePudding user response:
I don't have time to check your complete code.
But the root = new vertex(*p_BTree.root);
in your copy constructor would be a problem if root
is nullptr
.
And your vertex
manages resources (it has a delete
in the destructor), but does not follow the rule of three/five.
So as soon as a copy of an instance of vertex
is created, the pointers will be copied, and two instances of vertex
manage the same pointers which results in double free.
And using this in a node of a tree is problematic:
~vertex(){
delete leftChild;
delete rightChild;
leftChild = nullptr;
rightChild = nullptr;
};
If you have a tree with a large depth you could encounter a stackoverflow. The reason for that is because the deletion is done recursively. You want to collect the nodes of the tree in the root and delete them there.
The same is true for the deep copying in the copy constructor and copy assignment operator you have to create for vertex
.
Further notes:
- doing
root = nullptr;
after thedelete
in the destructor is something you don't need to do. Same forvertex
. - be consistent and don't mix
NULL
andnullptr
. You should stick withnullptr
. root = NULL;
in your constructor should be either done with a default value for the member variable or through the member initializer lists of the constructor. Same forvertex
.