c++destructorcopy-constructoroctree

Copy constructor and destructor octree c++


I have created an Octree data structure but it's not perfect yet. I'm struggled with the copy constructor and the destructor. Here is my header file:

class Octree
{
public:
static int lastbranch;
static bool utolsoelotti;

struct node
{
    int value;
    node *child[8];
};

Octree();
~Octree();
Octree(const Octree& oct);

void clear(node* node);
node* searchandset(int dec, int value);
node* search(int dec);
node* step(node *node, int k);
node* copy(node *n);
void Print(node *n)const;
void deletebranch(int branch);
node *root;
};

Constructor,destructor,copy contrsuctor

Octree::Octree()
{
root = new node;
root->value = 0;

for (int i = 0; i < 8; i++)
    root->child[i] = 0;
}
Octree::~Octree()
{
clear(root);
}

Octree::Octree(const Octree& oct) {
root = copy(oct.root);
}

void Octree::clear(node *node){
    for (int i = 0; i < 8; i++)
        if (node->child[i])
            clear(node->child[i]);

    delete node;
}

Octree::node*Octree::copy(node *n) {
node* n2 = new node;
if (n) {
    for (int i = 0; i < 8; i++) {
        n2->child[i] = copy(n->child[i]);
    }
}
return n2;
}

And here is how I created objects in the main:

int main() {

Octree tree;
Octree tree2(tree);

tree.searchandset(8, 2);
tree2.Print(tree2.search(8));
return 0;
}

In the searchandset function I'm giving a value for node number 8 at the first tree. After that I'm calling the copy constructor and print the 8th node of the second tree. The value is the same what I gave for the first tree, but when the desctructor called I always got this exception:

Exception thrown: read access violation. node was 0xDDDDDDDD.

As I know it means that I tried to delete the nodes which I have already deleted. The object 'tree2' is a different object from 'tree' with the same values and nodes isn't it? Then I don't understand that exception above. I'm new in c++ and I know it's basic stuff, so if somebody would direct me into the right direction I would very appreciate it.


Solution

  • The problem lies the in copy function. Let's go it through step by step:

    node* n2 = new node;
    
    if (n) {
        for (int i = 0; i < 8; i++)
            n2->child[i] = copy(n->child[i]);
    }
    return n2;
    

    For an empty Octree oct, constructed with the default constructor, and copied to another Octree:

    1. A new node is created, n2
    2. n is the root of oct, so the condition if true
    3. child[i] of n2 has a value of a copy of the corresponding child, so call copy again
    4. A new node n2 is created
    5. n is nullptr (because the children where all nullptr in oct), so don't execute condition
    6. Return n2
    7. Repeat steps 3 to 6 8 times
    8. Return the root n2
    9. Assign new pointer (n2) to root of the copied object

    But wait! Did you notice that in step 6, you are returning a new pointer, even though the child is supposed to be nullptr!

    That's the problem, because then, in clear, you will loop through each child. That's still ok, right? But then, you try to access the children, which are uninitialized (they have random value, the condition will evaluate to true), so you get a Read access violation, because it's not your memory.

    So the solution? Only allocate memory for n2 if n is not nullptr.