I wouldn't do it this way. The reason for the move members to exist in the first place is performance. Doing this for your move constructor is like shelling out megabucks for a super-car and then trying to save money by buying regular gas.
If you want to reduce the amount of code you write, just don't write the move members. Your class will copy just fine in a move context.
If you want your code to be high performance, then tailor your move constructor and move assignment to be as fast as possible. Good move members will be blazingly fast, and you should be estimating their speed by counting loads, stores and branches. If you can write something with 4 load/stores instead of 8, do it! If you can write something with no branches instead of 1, do it!
When you (or your client) put your class into a std::vector
, a lot of moves can get generated on your type. Even if your move is lightning fast at 8 loads/stores, if you can make it twice as fast, or even 50% faster with only 4 or 6 loads/stores, imho that is time well spent.
Personally I'm sick of seeing waiting cursors and am willing to donate an extra 5 minutes to writing my code and know that it is as fast as possible.
If you're still not convinced this is worth it, write it both ways and then examine the generated assembly at full optimization. Who knows, your compiler just might be smart enough to optimize away extra loads and stores for you. But by this time you've already invested more time than if you had just written an optimized move constructor in the first place.
Update 11 years later...
I note with some amusement that the referenced MSDN article no longer advises to write the move constructor in terms of the move assignment operator. And yet, it still gets it sort of wrong...
MemoryBlock(MemoryBlock&& other) noexcept
: _data(nullptr)
, _length(0)
{
_data = other._data;
_length = other._length;
other._data = nullptr;
other._length = 0;
}
(I've omitted logging and redundant comments)
Why not just initialize in the initialization list instead of assignments within the constructor body? Like this:
MemoryBlock(MemoryBlock&& other) noexcept
: _data(other._data)
, _length(other._length)
{
other._data = nullptr;
other._length = 0;
}
I haven't bothered to see if the optimized code is equivalent or not. I would guess that a decent optimizer could make the above two constructors have identical object code. But the second is easier to read and easier to write, with no other downsides.
Aside
The referenced MSDN article has a copy assignment operator that does not meet the basic exception safety guarantee:
MemoryBlock& operator=(const MemoryBlock& other)
{
if (this != &other)
{
// Free the existing resource.
delete[] _data;
_length = other._length;
_data = new int[_length];
std::copy(other._data, other._data + _length, _data);
}
return *this;
}
If _data = new int[_length];
throws an exception, then *this
has a _data
that points to a deleted block of memory, and a _length
with the value other._length
. Thus *this
is in a state that does not meet the class invariants:
MemoryBlock
invariants:
_data
is nullptr
or points to a new int[_length]
-allocated block of memory.
- If
_data == nullptr
then _length == 0
.
In addition to this correctness bug, there is also a performance bug:
If _length == other._length
, there is no need to go through the very expensive operation of deallocating a block of memory of length _length
just to turn around and allocate a block of memory of length _length
.
This is a more complicated copy assignment operator, but:
It carries the basic exception safety guarantee. If an exception happens, the object is left in a valid state (class invariants hold).
It avoids expensive trips to the heap at all costs. Branches are cheaper than trips to the heap, except for delete nullptr
.
MemoryBlock& operator=(const MemoryBlock& other)
{
if (this != &other)
{
if (_length != other._length)
{
// Free the existing resource.
delete[] _data;
// Set *this to a valid non-owning state
data_ = nullptr;
_length = 0;
if (other._length > 0)
{
// Set *this to own a block of length other._length
_data = new int[other._length]; // might throw
_length = other._length;
}
}
std::copy(other._data, other._data + _length, _data);
}
return *this;
}