operator = implementation in deep copy mechanism

Question

Previously I had understood a little about deep copy (basic data types), then I tried to make an exercise with std::string, for copy constructor it worked, but for operator = I'm still a bit confused,

#include <iostream>
class Person {
    //heap
    std::string *m_name;
public:
    Person(std::string_view name) : m_name {new std::string(name)} {
        std::cout << "Constructing..." << m_name << " ("<< *m_name << ") " << std::endl;
    }

    //deep copy
    Person(const Person &p){
        std::cout << "deep copy..." << std::endl;
        m_name = new std::string(*p.m_name);
    }

    Person& operator=(const Person& p) {
        if (&p == this) return *this;
        //everything I comment means it doesn't work

        // m_name = p.m_name;
        // m_name = new std::string(*p.m_name);
        // *m_name = *p.m_name;
        // m_name = new std::string(*p.m_name);
        // *m_name = *p.m_name;

        // size_t length = p.m_name->size();
        // m_name = new std::string[length];
        // *m_name = *p.m_name;

        // size_t length = m_name->size();
        // m_name = new std::string[length];
        // for(size_t i {0}; i<length; ++i){
        //  m_name[i] = p.m_name[i];
        // }
        return *this;
    }
    //deep copy

    ~Person() {
        std::cout << "Destructing..." << m_name << " (" << *m_name << ") " << std::endl;
        delete m_name;
    }
};

void caller() {
    Person rudy {"rudy"};
    
    Person rusty = rudy;
}

int main(int argc, char const **argv) {
    std::cout << "Calling caller()" << std::endl;
    caller();
    std::cout << "Back in main()" << std::endl;
    return 0;
}

it seems easy if you use a basic type like int how does the copy constructor work but operator = doesn't work?

Side note: Here is a stupidly simple way to use the copy constructor to make a nearly indestructible assignment operator: [What is the copy-and-swap idiom?](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). Now Let's take a closer look at what you have to see where things went wrong. — user4581301, Sep 22 '21 at 01:19
When you write `Person rusty = rudy;`, it actually uses the copy constructor - the language requires that. If you want the copy assignment operator used, do `Person rusty; rusty = rudy;`. The correct statement was `m_name = new std::string(*p.m_name);`, but - as Remy's just written below - you do need to `delete m_name;` first to avoid a memory leak. — Tony Delroy, Sep 22 '21 at 01:20
In a copy `operator=`, you need to EITHER 1) `delete` the old `m_name` and `new` a new `std::string` with the *value* from `*p.m_name`, like your copy constructor does (which is why you should get in the habit of implementing `operator=` using the copy-swap idiom, since copy-assignment almost always mirrors copy-construction - same with move-assignment and move-construction); or 2) simply assign the *value* of `*p.m_name` to the existing `*m_name` without destroying/creating anything at all. — Remy Lebeau, Sep 22 '21 at 01:21
Hmmm another side note first: Dynamically allocating `std::string`s is almost always an unforced error. Don't do this without a really good reason. A huge part of `std::string`'s job is to handle dynamic allocation for you, eliminating the need for copy and move infrastructure and destructors because `std::string` fully observes [RAII](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) and the [Rules of Three and Five](https://en.cppreference.com/w/cpp/language/rule_of_three) for you. — user4581301, Sep 22 '21 at 01:24
And reading further suggests you already knew the above and are simply making things hard on yourself for practice. I'll leave the above comment as a warning to those who follow and don't know `string` is better off automatically allocated. — user4581301, Sep 22 '21 at 01:26
Usually, new object should be copied first to make code exception safe. This is a good reson to use swap idiom. Simple and safe. — Phil1970, Sep 22 '21 at 01:40

score 3 · Answer 1 · answered Sep 22 '21 at 01:29

3

The correct way to implement your operator= would be either:

Person& operator=(const Person& p) {
    if (&p != this) {
        *m_name = *(p.m_name);
    }
    return *this;
}

Or:

Person& operator=(const Person& p) {
    if (&p != this) {
        std::string *tmp = m_name;
        m_name = new std::string(*(p.m_name));
        delete tmp;
    }
    return *this;
}

This can be simplified by utilizing your copy constructor to handle the actual copying of the new data, and your destructor to handle destroying the old data (this is known as the copy-swap idiom), eg:

Person& operator=(const Person& p) {
    if (&p != this) {
        Person tmp(p)
        std::swap(m_name, tmp.m_name);
    }
    return *this;
}

answered Sep 22 '21 at 01:29

Remy Lebeau

555,201
31
458
770

With the swap idiom, you don't have to check for self-assignment and it would generally be ommited. – Phil1970 Sep 22 '21 at 01:38
1

@Phil1970 I call that one a coin-toss. Personally I think that self-assignment is a almost certainly a bug that should be resolved rather than covered up, but I'll also admit it's the sort of thing that sometimes slips through in a complicated program. Remy's approach at least hides it with the minimum possible effort. – user4581301 Sep 22 '21 at 01:45
using this code can still run without having to create a temporary variable, even though I have tried this code before (see the question above) and FAILED, m_name = new std::string(*p.m_name); – Sam Smith Sep 22 '21 at 01:54
Take the parameter by value and you don't have to care about self-assignment. It is called the copy/swap idiom and not the swap idiom. I don't know that waiting to make the copy saves enough cycles to be worth implementing twice (move assignment). – sweenish Sep 22 '21 at 02:04
@sweenish if both copy and move semantics are to be implemented, it makes sense to take the parameter by value and let the compiler decide which constructor to call prior to swapping. But, for just copy-only semantics, it makes sense to avoid a copy unnecessarily. One of C++'s strengths is that you don't have to pay for what you don't need. – Remy Lebeau Sep 22 '21 at 03:49
If copy semantics are to be implemented, so should move semantics. – sweenish Sep 22 '21 at 13:17

operator = implementation in deep copy mechanism

1 Answers1