CODEWITHSUNDEEP
c++visual-c++
Kobojunkie
Kobojunkie

Reputation: 6545

Struggling with Copy Constructors in C++

I am trying to write me a copy constructor for my struct but I don't seem to be getting this right and would appreciate any help possible please. I want to do a deep copy, recursively, but I keeping getting suggestions to initialize in the initialization list and that does not seem to be working out well either. 

struct Node
{
    Node* left; // will be our previous
    Node* right;// will be our next
    Node* previous;// get a handle to the previous node
    string value;

    Node(string nval): left(NULL), right(NULL), previous(NULL), value(nval)
    {

    }

    Node(Node const& node)
     : previous(new Node(node.previous)), 
       left(new Node(node.left)), 
       right(new Node(node.right)), 
       value(node.value)
    {
    }

    Node& operator=(const Node&)
    {
       // ...
    }
};

Thanks in advance.

Upvotes: 0

Views: 154

Answers (1)

je4d
je4d

Reputation: 7838

To avoid this recursing infinitely, you need to test for null. You also need to dereference the objects in the RHS node.

 Node(Node const& node) :
     previous(node.previous ? new Node(*node.previous) : NULL),
     left(node.left ? new Node(*node.left) : NULL),
     right(node.right ? new Node(*node.right) : NULL), 
     value(node.value)
 {
 }

But this will only work if previous != next != previous always holds, and your nodes don't point to each other. If this is a linked list structure, this won't work.

I'm guessing that your nodes do point together, and what you're trying to do here is to clone a whole data structure whenever one node is copied. In that case I can't see a way you can sensibly do this in a copy constructor. I'd suggest making the class noncopyable and writing a separate function to clone the whole data structure. If you provide more context of how your structure of Nodes is constructed, I may be able to provide a more concrete suggestion.

Edit

You indicated in the comments that this is a tree, and that previous is the parent node pointer. This copy constructor should do a subtree clone:

Node(Node const& node) :
    previous(NULL),
    left(node.left ? new Node(*node.left) : NULL),
    right(node.right ? new Node(*node.right) : NULL), 
    value(node.value)
{
    if (left)
        left->previous = this;
    if (right)
        right->previous = this;
}

Note that instead of allocating a new Node for previous, it leaves it blank and lets its parent set it.

Upvotes: 6

Related Questions