Aberrant Aberrant - 1 month ago 12
C++ Question

Should the Copy-and-Swap Idiom become the Copy-and-Move Idiom in C++11?

As explained in this answer, the copy-and-swap idiom is implemented as follows:

class MyClass
{
private:
BigClass data;
UnmovableClass *dataPtr;

public:
MyClass()
: data(), dataPtr(new UnmovableClass) { }
MyClass(const MyClass& other)
: data(other.data), dataPtr(new UnmovableClass(*other.dataPtr)) { }
MyClass(MyClass&& other)
: data(std::move(other.data)), dataPtr(other.dataPtr)
{ other.dataPtr= nullptr; }

~MyClass() { delete dataPtr; }

friend void swap(MyClass& first, MyClass& second)
{
using std::swap;
swap(first.data, other.data);
swap(first.dataPtr, other.dataPtr);
}

MyClass& operator=(MyClass other)
{
swap(*this, other);
return *this;
}
};


By having a value of MyClass as parameter for operator=, the parameter can be constructed by either the copy constructor or the move constructor. You can then safely extract the data from the parameter. This prevents code duplication and assists in exception safety.

The answer mentions you can either swap or move the variables in the temporary. It primarily discusses swapping. However, a swap, if not optimised by the compiler, involves three move operations, and in more complex cases does additional extra work. When all you want, is to move the temporary into the assigned-to object.

Consider this more complex example, involving the observer pattern. In this example, I've written the assignment operator code manually. Emphasis is on the move constructor, assignment operator and swap method:

class MyClass : Observable::IObserver
{
private:
std::shared_ptr<Observable> observable;

public:
MyClass(std::shared_ptr<Observable> observable) : observable(observable){ observable->registerObserver(*this); }
MyClass(const MyClass& other) : observable(other.observable) { observable.registerObserver(*this); }
~MyClass() { if(observable != nullptr) { observable->unregisterObserver(*this); }}

MyClass(MyClass&& other) : observable(std::move(other.observable))
{
observable->unregisterObserver(other);
other.observable.reset(nullptr);
observable->registerObserver(*this);
}

friend void swap(MyClass& first, MyClass& second)
{
//Checks for nullptr and same observable omitted
using std::swap;
swap(first.observable, second.observable);

second.observable->unregisterObserver(first);
first.observable->registerObserver(first);
first.observable->unregisterObserver(second);
second.observable->registerObserver(second);
}

MyClass& operator=(MyClass other)
{
observable->unregisterObserver(*this);
observable = std::move(other.observable);

observable->unregisterObserver(other);
other.observable.reset(nullptr);
observable->registerObserver(*this);
}
}


Clearly, the duplicated part of the code in this manually written assignment operator is identical to that of the move constructor. You could perform a swap in the assignment operator and the behaviour would be right, but it would potentially perform more moves and perform an extra registration (in the swap) and unregistration (in the destructor).

Wouldn't it make much more sense to reuse the move constructor's code in stead?

private:
void performMoveActions(MyClass&& other)
{
observable->unregisterObserver(other);
other.observable.reset(nullptr);
observable->registerObserver(*this);
}

public:
MyClass(MyClass&& other) : observable(std::move(other.observable))
{
performMoveActions(other);
}

MyClass& operator=(MyClass other)
{
observable->unregisterObserver(*this);
observable = std::move(other.observable);

performMoveActions(other);
}


It looks to me like this approach is never inferior to the swap approach. Am I right in thinking that the copy-and-swap idiom would be better off as the copy-and-move idiom in C++11, or did I miss something important?

Answer

It's been a long time since I asked this question, and I've known the answer for a while now, but I've put off writing the answer for it. Here it is.

The answer is no. The Copy-and-swap idiom should not become the Copy-and-move idiom.

An important part of Copy-and-swap (which is also Move-construct-and-swap) is a way to implement assignment operators with safe cleanup. The old data is swapped into a copy-constructed or move-constructed temporary. When the operation is done, the temporary is deleted, and its destructor is called.

The swap behaviour is there to be able to reuse the destructor, so you don't have to write any cleanup code in your assignment operators.

If there's no cleanup behaviour to be done and only assignment, then you should be able to declare the assignment operators as default and copy-and-swap isn't needed.

The move constructor itself usually doesn't require any clean-up behaviour, since it's a new object. However, in this question's observer pattern example, that's actually an exception where you have to do extra cleanup work because references to the old object need to be changed. In general, I would recommend making your observers and observables, and other design constructs based around references, unmovable whenever possible.

Comments