class Node{
private:
bool IsLeaf;
Node* child1, child2;
public:
Node();
void DeleteChildren();
};
void Node::DeleteChildren()
{
if(!IsLeaf)
{
child1->DeleteChildren();
child2->DeleteChildren();
delete child1;
delete child2;
}
}
class Tree{
private:
Node *Root;
public:
Tree();
~Tree();
};
Tree::~Tree
{
Root->DeleteChildren();
delete Root;
}
"push_back" calls destructor?
Author
I have a class, let's call it Tree that contains a pointer to the root of a binary tree. (the root has pointers to its kids who have pointers to theirs...). Now, in the destructor of Tree I have a function that recursively deallocates the memory used by the tree. Something like this:
Now, I wanted to make a std vector of trees so I did this:
Tree Tree1;
vector<Tree> Trees;
Trees.push_back(Tree1);
But for some reason the program crashed.
Sp I ran the debugger and found that in the first line Tree1 was initialized and set up properly, but for some reason, as soon as I called the third line Tree1.~Tree was called and it was all erased. Why does this happen?
Thanks.
push_back may copy-construct and destruct any number of elements of the vector any number of times. Therefore, it is imperative that copy construction of your objects is safe.
In your case, it isn't, because your ownership policy is sloppy (the sub-trees are owned by the Tree object as well as any of its copies, which makes it impossible to copy your objects without a crash).
In your case, it isn't, because your ownership policy is sloppy (the sub-trees are owned by the Tree object as well as any of its copies, which makes it impossible to copy your objects without a crash).
As std::vector resizes, it will use the copy constructor to create new instances of the objects it contains before destructing the 'old' instances. As you delete the tree in the destructor, the new instance doesn't have good memory to listen to.
I'm guessing the easiest fix would be to drop the manual memory-handling and use smart pointers instead (as, I think, there would then be no need for writing a custom copy-constructor, which is the other option).
...
Also, note that I might be completely wrong (but I think I'm right).
EDIT: Ninja-ed by one minute. On the upside, I was right.
I'm guessing the easiest fix would be to drop the manual memory-handling and use smart pointers instead (as, I think, there would then be no need for writing a custom copy-constructor, which is the other option).
...
Also, note that I might be completely wrong (but I think I'm right).
EDIT: Ninja-ed by one minute. On the upside, I was right.
Simple answer is because it is supposed to and you are supposed to supply a copy constructor and assignment operator to allow deep copies.
google shallow copy and deep copy.
google shallow copy and deep copy.
Quote:Original post by daniel_i_l
Thanks for the replys. How can I improve my ownership policy?
Thanks.
Either write a sane copy-constructor (and assignment operator) or don't manually handle memory when you don't have to — use a smart pointer instead (which will manually delete the memory when it's no longer being used).
In terms of actually improving the state of affairs rather than just fixing the bug, you're better doing both (such that each object owns its own copy of the tree, which is then automatically deleted when that instance is destructed without you having to do anything like provide a manual destructor or have a DeleteNode member which manually uses delete).
The Node should call it's own DeleteChildren method in its destructor. And DeleteChildren should delete child1 and 2 only if they're not null.
You can also use Reference Counting. That should also fix that problem when implementing a proper copy constructor. Though this way it is easier to write buggy code without smart pointers.
Quote:Original post by Cranky
You can also use Reference Counting. That should also fix that problem when implementing a proper copy constructor.
Whilst a smart pointer saves you having to roll-your-own, which will doubtless end up poorer in some way than those provided in, say, Boost.
EDIT: Note: this post was written before you edited your post.
Author
How do I write a copy constructor that copies the whole tree? And is that the only solution? Because I've seen many tutorials that build trees this way and they never mentioned the need for a copy constructor.
Thanks.
Thanks.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
