Set struct variable nested inside a class in a copy constructor

465 views Asked by At

I'm having a terrible time figuring out how to write a copy constructor. As you can see below, we have a class with a struct nested inside of it for linked nodes to contain data. I can't use member-wise assignment to copy DynIntStack, because the pointers in the StackNode struct will just end up pointing to the same object.

Unfortunately, I can't figure out how to write the constructor such that it sets the value inside the new StackNode to be equal to the value of the StackNode inside the DynIntStack I'm passing the constructor.

I understand what I'm trying to do here, I just can't figure out how to write it out correctly.

class DynIntStack
{
private:
    // Structure for stack nodes
    struct StackNode
    {
        int value;        // Value in the node
        StackNode *next;  // Pointer to the next node
    };

    StackNode *top;      // Pointer to the stack top

public:
    // Constructor
    DynIntStack()
    {
        top = NULL;
    }
    // copy constructor
    DynIntStack(DynIntStack &obj)
    {
        DynIntStack::StackNode value = obj::StackNode.value;
        DynIntStack::StackNode next = new StackNode;
    }
3

There are 3 answers

4
Vlad from Moscow On BEST ANSWER

Try the following

DynIntStack( DynIntStack &obj ) : top( nullptr )
{
    StackNode **last = ⊤

    for ( StackNode *current = obj.top; current; current = current->next )
    {
        *last = new StackNode { current->value, nullptr };
        last = &( *last )->next; 
    } 
}

If your compiler does not support this form of the new operator

*last = new StackNode { current->value, nullptr };

when you can replace it with statements

*last = new StackNode;
( *last )->value = current->value;
( *last )->next = nullptr; // or NULL
0
NathanOliver On

You are doing this wrong. You need to fully implement your linked list class and then in you copy constructor for DynIntStack it would become:

DynIntStack(DynIntStack &obj) : top(obj.top) {}

What would be better yet is to just use a std::list and not worry about it.

0
donjuedo On

The problem I see here is value and next in the copy constructor. You have specified a type for each, so the compiler allocates local variables for each, separate entities from the member data of the same names. The wrong things are being set, unused, and deallocated when going out of scope.