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

Question

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 1

Give each special member the tender loving care it deserves, and try to default them as much as possible:

class MyClass
{
private:
    BigClass data;
    std::unique_ptr<UnmovableClass> dataPtr;

public:
    MyClass() = default;
    ~MyClass() = default;
    MyClass(const MyClass& other)
        : data(other.data)
        , dataPtr(other.dataPtr ? new UnmovableClass(*other.dataPtr)
                                : nullptr)
        { }
    MyClass& operator=(const MyClass& other)
    {
        if (this != &other)
        {
            data = other.data;
            dataPtr.reset(other.dataPtr ? new UnmovableClass(*other.dataPtr)
                                        : nullptr);
        }
        return *this;
    }
    MyClass(MyClass&&) = default;
    MyClass& operator=(MyClass&&) = default;

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

The destructor could be implicitly defaulted above if desired. Everything else needs to be explicitly defined or defaulted for this example.

Reference: http://accu.org/content/conf2014/Howard_Hinnant_Accu_2014.pdf

The copy/swap idiom will likely cost you performance (see the slides). For example ever wonder why high performance / often used std::types like std::vector and std::string don't use copy/swap? Poor performance is the reason. If BigClass contains any std::vectors or std::strings (which seems likely), your best bet is to call their special members from your special members. The above is how to do that.

If you need strong exception safety on the assignment, see the slides for how to offer that in addition to performance (search for "strong_assign").