What is the proper structure of a copy assignment in c++?

Question

I am handling a binary search tree data structure and I am arrived at the implementation of the copy semantics. Since I want to do a deep copy, I decided to create a new object inside the copy assignment and then to pass it outside the function. However when I invoke a copy assignment nothing is done on the variable of the object that I want to change. What am I doing wrong? I assume that there is an error in the definition of the keywords in the operator overload.

// copy assignment -- deep copy
bst operator=(const bst& bintree){
    std::cout<<"copy assignment invoked"<<std::endl;
    bst newbst{};//here I create a new object, so I am sure it is a deep copy
    newbst.insert(bintree.root->value);//I add the root first
    copy_part(bintree.root, newbst);//then I add the remaining nodes    
    return newbst;
}
void copy_part(Node<treepair>* x, bst& bintree){
        if(x->left!=nullptr){
            bintree.insert(x->left->value);     
                copy_part(x->left,bintree);
        }
        if(x->right!=nullptr){
            bintree.insert(x->right->value);        
                copy_part(x->right,bintree);
        }
}
...

int main(){

bt_INT bintree{};
/*
here i fill bintree
*/

bt_INT bintree_copy{};//made an empty tree

    bintree_copy = bintree;//copy assignment

   //however if i print bintree_copy it gives me an empty array.
   // what am I missing here?



}

Notice that in the definition of the operator= I did not write bst& because I had to return a local variable.

EDIT: I am reading your comments and I made a few changes

class bst{
...

bst& operator=(const bst& bintree){

    (*this).clear();//I clear the nodes present in the LHS of the assignment    
    (*this).insert(bintree.root->value);//I insert the root first
    copy_part(bintree.root, *this);
    return *this;
}
void copy_part(Node<treepair>* x, bst& bintree){//I left this part since I need to perform a deep copy
        if(x->left!=nullptr){
            bintree.insert(x->left->value);     
                copy_part(x->left,bintree);
        }
        if(x->right!=nullptr){
            bintree.insert(x->right->value);        
                copy_part(x->right,bintree);
        }
}
 ...
};

Now it works fine. Now I just have to cope with the copy constructor.

An assignment operator should implement to assign a copy from RHS to `*this`. Whether the _contents_ is a flat or deep copy is on your decision BUT an assignment operator does not provide a copy of `*this` itself. That's wrong be design - completely. There is no creation of new instance necessary. Once, you call assignment you already have two instances - the one on the left side and the one on the right side. (Strictly speaking, they could be the same object... but that's a special case which is often handled by `if (this != &rhs)`.) — Scheff's Cat, Feb 12 '20 at 09:45
... and hence it doesn't make any sense to not return a reference. Return type should be `bst&` and you should return `*this`. And: an assignment operator MUST be a member function. (It's one of the few operators which cannot be a plain (i.e. non-member) function.) — Scheff's Cat, Feb 12 '20 at 09:48
I think I understand now. Using *this I can act directly on the LHS of the = operator. So I can delete its content and fill it again — NicDom23, Feb 12 '20 at 10:12
_Using *this I can act directly on the LHS of the = operator._ Correct. ;-) — Scheff's Cat, Feb 12 '20 at 10:13
Thinking twice: To keep `*this` always in a consistent state, you could fill a local instance (as you did), then swap `*this` with the local, and then `return *this;`. The local instance with the now old contents of `*this` will go out of scope and cause that the old contents is deleted (if the destructor does it properly). The swapping in containers is usually very cheap because it does exchange only the pointers to actual contents. You could use the same trick in your `class bst`. — Scheff's Cat, Feb 12 '20 at 10:17
FYI: [SO: What is the copy-and-swap idiom?](https://stackoverflow.com/a/3279550/7478597) — Scheff's Cat, Feb 12 '20 at 10:19

Build Succeeded · Answer 1 · 2020-02-12T10:37:57.073

You don't need to create the temporary object in copy assignment. Due to it you loose changes made locally to that object (temporary bst object).

Mostly self-assignment, memory allocation and copying of elements are steps copy assignment have. Example:

struct C
{
    std::unique_ptr<int[]> data;
    std::size_t size;
    // non-copy-and-swap assignment
    C& operator=(const C& other)
    {
        // check for self-assignment
        if(&other == this)
            return *this;
        // reuse storage when possible
        if(size != other.size)
        {
            data.reset(new int[other.size]);
            size = other.size;
        }
        std::copy(&other.data[0], &other.data[0] + size, &data[0]);
        return *this;
    }
    // note: copy-and-swap would always cause a reallocation
};

The ideal way of copy assignment example link: https://en.cppreference.com/w/cpp/language/copy_assignment

While this link may answer the question, it is better to include the essential parts of the answer here and provide the link for reference. Link-only answers can become invalid if the linked page changes. — Johan, Feb 12 '20 at 10:16

What is the proper structure of a copy assignment in c++?

1 Answers1