Proper way to implement copy-assignment operator

Question

I am trying to understand the correct way to implement the copy-assignment operator =. So I've implemented this example:

class Window {
public:
    Window(const std::string& = std::string());

    Window(const Window&);
    Window& operator=(const Window&);
    ~Window();

    std::string win_name()const { return *psWinName; }
    std::size_t win_id()const { return winID; }

private:
    std::string* psWinName =  nullptr;
    std::size_t winID;
};

Window::Window(const std::string& win_name) : 
    psWinName(new std::string(win_name)),
        winID(0) {
}

Window::Window(const Window& rhs) :
    psWinName(new std::string(*rhs.psWinName)),
    winID(rhs.winID) {
}


/* Erroneous copy-assignment operator
Window& Window::operator=(const Window& rhs)
{
    delete psWinName;
    psWinName = new std::string(*rhs.psWinName);;
    winID = rhs.winID;

    return *this;
}
*/

Window& Window::operator=(const Window& rhs)
{
    std::string* tmp = new std::string(*rhs.psWinName);
    delete psWinName;
    psWinName = tmp;
    winID = rhs.winID;

    return *this;
}

Window::~Window() 
{
    delete psWinName;
}

int main() 
{

    /*Window win_explorer("Explorer");
    auto win_exp2(win_explorer);
    Window win_desktop("Desktop Window");

    Window win;
    win = win_desktop;

    win = win;

    cout << win_explorer.win_name() << " : " << win_explorer.win_id() << endl;
    cout << win_exp2.win_name() << " : " << win_exp2.win_id() << endl;
    cout << win_desktop.win_name() << " : " << win_desktop.win_id() << endl;
    cout << win.win_name() << " : " << win.win_id() << endl;*/

    int* p = new int(9);
    cout << p << " " << *p << endl;

    int* tmp = p;
    cout << tmp << " " << *tmp << endl;
    delete p;
    p = new int(*tmp);

    cout << p << " " << *p << endl;
    cout << tmp << " " << *tmp << endl;


    cout << "\ndone\n";
}

So as far I've showed the incorrect form of = operator.

Is it wrong because:

delete psName;
psName = new std::string(*rhs.psName); // UB here?

So I don't know how the Order of evaluation occurs here: psName = new std::string(*rhs.psName);. So is this a UB?

Also what about:

delete psName;
psName = new std::string();
*psName = *rhs.psName;

But yields the same result which is undefined value.

I talk bout assigning some object to itself.

** Also people argue about: delete psName; psName = new std::string(); they say new may throw. But I think it shouldn't as long as I've released a resource the same as it has.

** N.B: In fact I am not about a std::string but about any resource to acquire.

Change `std::string* psWinName` to `std::string psWinName` and then use the Rule of Zero (don't define any of the special members functions. Let RAII work for you) — NathanOliver, Dec 12 '19 at 20:43
Why `std::string* psWinName` and not `std::string psWinName`? — Evg, Dec 12 '19 at 20:43
"_So I don't know how the Order of evaluation occurs here: `psName = new std::string(*rhs.psName);`. So is this a UB?_" About, what, specific, order of evaluation you are confused about? Why do you think, that this would be UB? — Algirdas Preidžius, Dec 12 '19 at 20:45
@NathanOliver-ReinstateMonica: This is not what I am about to achieve. But about resource management. — Itachi Uchiwa, Dec 12 '19 at 20:45
Use the [copy and swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). Or even better, get rid of the pointer. Then you won't need the destructor/copy ctor/copy assignment. — HolyBlackCat, Dec 12 '19 at 20:45
@HolyBlackCat: Thank you. But I only want to know what's wrong here, and what should I do. — Itachi Uchiwa, Dec 12 '19 at 20:46
There is nothing wrong here. Why do you think you have UB? Is it because of `delete`? `delete` just kills what the pointer pointed to, it doesn't mean you can't assign a new address to the pointer afterwards. Otherwise this is just a dupe of: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — NathanOliver, Dec 12 '19 at 20:47
@NathanOliver-ReinstateMonica: What about assigning an object to itself? then `delete` deletes the object which pointed to by `*this` and `rhs` then the initialization `psName = new std::string(*rhs.psName);` I think is `UB` — Itachi Uchiwa, Dec 12 '19 at 20:50
Sure, you need to prevent self assignment. I'm going to close this as a dupe of the rule of 3 canonical as it goes through all of this. — NathanOliver, Dec 12 '19 at 20:52
@ItachiUchiwa *they say new may throw. But I think it shouldn't* -- You should be writing safe code, not guess at what something should or should not do. You shouldn't be adjusting member variables before calling functions that may `throw`. — PaulMcKenzie, Dec 12 '19 at 20:56

Proper way to implement copy-assignment operator

0 Answers0