Implementing copy assignment operator in terms of move constructor

Question

Considering the following concept/example class

class Test { public: explicit Test(std::string arg_string) : my_string( std::move(arg_string) ) { } Test(const Test& Copy) { this->my_string = Copy.my_string; } Test& operator=(Test Copy) { MoveImpl( std::move(Copy) ); return *this; } Test(Test&& Moved) { MoveImpl( std::forward<Test&&>(Moved) ); } Test& operator=(Test&& Moved) { MoveImpl( std::forward<Test&&>(Moved) ); return *this; } private: void MoveImpl(Test&& MoveObj) { this->my_string = std::move(MoveObj.my_string); } std::string my_string; };

The copy constructor takes a const& as usual.

The copy assignment operator is implemented in terms of the copy constructor (as ,if I remember correctly, Scott Meyers pointed out that exception safety and self assignment issues are resolved this way).

When implementing the move constructor and move assignment operator I figured that there was some "code duplication", which I "eliminated" by adding the MoveImpl(&&) private method.

My question is, since we know that the copy assignment operator gets a new copy of the object which will be cleaned up on scope end, whether it is correct/good practice to use the MoveImpl() function to implement the functionality of the copy assignment operator.

Does this code actually work? The move assignment looks like an ambiguous overload to me (e.g. in Test() = Test();). — Kerrek SB
– Kerrek SB, Commented Sep 8, 2016 at 8:37
Off topic: Why are you not using member initializer list in the copy constructor, or why not just = default; it? Actually, every constructor besides the first one does an assignment, and that is not good. — LogicStuff
– LogicStuff, Commented Sep 8, 2016 at 8:39
std::forward<Test&&>(Moved) can simply be std::move(Moved). — Jarod42
– Jarod42, Commented Sep 8, 2016 at 8:40
@KerrekSB, No you are correct, indeed the code complains for overload resolution. The error goes away if I declare the copy-assignment as Test& operator=(const Test& Copy). However, I think this brings back the self-assignment and exception safety issues. — george_ptr
– george_ptr, Commented Sep 8, 2016 at 8:46
@LogicStuff, if the MoveImpl performs a move how does the move constructor perform an assignment ? — george_ptr
– george_ptr, Commented Sep 8, 2016 at 8:52

Leon · Accepted Answer · 2016-09-08 09:25:28Z

The beauty of the by-value signature of the copy assignment operator is that it eliminates the need for a move assignment operator (provided that you properly define the move constructor!).

class Test { public: explicit Test(std::string arg_string) : my_string( std::move(arg_string) ) { } Test(const Test& Copy) : my_string(Copy.my_string) { } Test(Test&& Moved) : my_string( std::move(Moved.my_string) ) { } // other will be initialized using the move constructor if the actual // argument in the assignment statement is an rvalue Test& operator=(Test other) { swap(other); return *this; } void swap(Test& other) { std::swap(my_string, other.my_string); } private: std::string my_string; };

I see, by getting a copy on the assignment operator you let the compiler decide which constructor to call so you won't have to write 2 different assignment operators.

Richard Hodges · Accepted Answer · 2016-09-08 08:56:50Z

Your thinking is along the right lines, but the point of commonality is in the swap operation.

If you try to do it earlier you lose the chance to initialise members in the initialisation list in the constructors, which conceptually leads to redundant default initialisation of members and difficulties in handling exceptions neatly.

This is closer to the model that you're after:

class Test { public: explicit Test(std::string arg_string) : my_string( std::move(arg_string) ) { } Test(const Test& Copy) : my_string(Copy.my_string) { } Test& operator=(Test const& Copy) { auto tmp(Copy); swap(tmp); return *this; } Test(Test&& Moved) : my_string(std::move(Moved.my_string)) { } Test& operator=(Test&& Moved) { auto tmp = std::move(Moved); swap(tmp); return *this; } void swap(Test& other) noexcept { using std::swap; swap(my_string, other.my_string); } private: std::string my_string; };

Of course, in reality, the rule of zero should always take preference unless you absolutely need special handling in the destructor (you almost never do):

class Test { public: explicit Test(std::string arg_string) : my_string( std::move(arg_string) ) { } // copy, assignment, move and move-assign are auto-generated // as is destructor private: std::string my_string; };

Collectives™ on Stack Overflow

Implementing copy assignment operator in terms of move constructor

2 Answers 2

1 Comment

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Related