C++ Copy Constructor + Pointer Object

Question

I'm trying to learn "big three" in C++.. I managed to do very simple program for "big three".. but I'm not sure how to use the object pointer.. The following is my first attempt.

I have a doubt when I was writing this...

Questions

1. Is this the correct way to implement the default constructor? I'm not sure whether I need to have it or not. But what I found in another thread about copy constructor with pointer is that I need to allocate the space for that pointer before copying the address in copy constructor..
2. How to assign the pointer variable in the copy constructor? The way that I wrote in Copy Constructor might be wrong.
3. Do I need to implement the same code (except return ) for both copy constructor and operatior=?

4. Am I correct in saying that I need to delete the pointer in destructor?

   class TreeNode
   {
   public:  
      TreeNode(); 
      TreeNode(const TreeNode& node);
      TreeNode& operator= (const TreeNode& node);
      ~TreeNode();
   private:
      string data;
      TreeNode* left;
      TreeNode* right;
      friend class MyAnotherClass;
   };
   

Implementation

TreeNode::TreeNode(){

    data = "";  

}

TreeNode::TreeNode(const TreeNode& node){
     data = node.data;

     left = new TreeNode();
     right = new TreeNode();

     left = node.left; 
     right = node.right;
}

TreeNode& TreeNode::operator= (const TreeNode& node){
     data = node.data;
     left = node.left;
     right = node.right;
     return *this;
}

TreeNode::~TreeNode(){
     delete left;
     delete right;
}

Thanks in advance.

Answer 1

Am I correct in saying that I need to delete the pointer in destructor?

Whenever designing an object like this, you first need to answer the question: Does the object own the memory pointed to by that pointer? If yes, then obviously the object's destructor needs to clean up that memory, so yes it needs to call delete. And this seems to be your intent with the given code.

However in some cases, you might want to have pointers that refer to other objects whose lifetime is supposed to be managed by something else. In that case you don't want to call delete because it is the duty of some other part of the program to do so. Furthermore, that changes all the subsequent design that goes into the copy constructor and assignment operator.

I'll proceed with answering the rest of the questions under the assumption that you do want each TreeNode object to have ownership of the left and right objects.

Is this the correct way to implement the default constructor?

No. You need to initialize the left and right pointers to NULL (or 0 if you prefer). This is necessary because unintialized pointers could have any arbitrary value. If your code ever default constructs a TreeNode and then destroys it without ever assigning anything to those pointers, then delete would be called on whatever that initial value is. So in this design, if those pointers aren't pointing at anything, then you must guarantee that they are set to NULL.

How to assign the pointer variable in the copy constructor? The way that I wrote in Copy Constructor might be wrong.

The line left = new TreeNode(); creates a new TreeNode object and sets left to point to it. The line left = node.left; reassigns that pointer to point to whatever TreeNode object node.left points to. There are two problems with that.

Problem 1: Nothing now points to that new TreeNode. It is lost and becomes a memory leak because nothing can ever destroy it.

Problem 2: Now both left and node.left end up pointing to the same TreeNode. That means the object being copy constructed and the object it's taking values from will both think they own that same TreeNode and will both call delete on it in their destructors. Calling delete twice on the same object is always a bug and will cause problems (including possibly crashes or memory corruption).

Since each TreeNode owns its left and right nodes, then probably the most reasonable thing to do is make copies. So you would write something similar to:

TreeNode::TreeNode(const TreeNode& node)
    : left(NULL), right(NULL)
{
    data = node.data;

    if(node.left)
        left = new TreeNode(*node.left);
    if(node.right)
        right = new TreeNode(*node.right);
}

Do I need to implement the same code (except return ) for both copy constructor and operatior=?

Almost certainly. Or at least, the code in each should have the same end result. It would be very confusing if copy construction and assignment had different effects.

EDIT - The above paragraph should be: The code in each should have the same end result in that the data is copied from the other object. This will often involve very similar code. However, the assignment operator probably needs to check if anything has already been assigned to left and right and so clean those up. As a consequence, it may also need to watch out for self-assignment or else be written in a way that doesn't cause anything bad to happen during self-assignment.

In fact, there are ways to implement one using the other so that the actual code that manipulates the member variables is only written in one place. Other questions on SO have discussed that, such as this one.