6

I recently read about copy & swap and am now trying to implement the ctors in a base and derived class. I have the four constructors in both my base and derived class, however I am unsure how to implement the assignment operator of the derived class.

explicit Base(int i) : m_i{i} {}
Base(const Base & other) : m_i{other.m_i}
Base(Base && other) : Base(0) { swap(*this, other); }
Base & operator=(Base other) { swap(*this, other); return *this; }
friend void swap(Base & a, Base & b) noexcept {
    using std::swap;
    swap(a.m_i, b.m_i);
}

explicit Derived(int j) : Base(42), m_j(j) {}
Derived(const Derived & other) : Derived(other.m_j) {}
Derived(Derived && other) : Derived(other.m_j) { swap(*this, other); }
Derived & operator=(Derived other) { /*???*/ }
friend void swap(Derived & a, Derived & b) noexcept {
    using std::swap;
    swap(a.m_j, b.m_j);
}
Deduplicator
  • 44,692
  • 7
  • 66
  • 118
gartenriese
  • 4,131
  • 6
  • 36
  • 60
  • Rule-of-thump: Single-argument non-copy/move ctors should be `explicit`: You really don't want to have an implicit conversion from `int` to `Base`... – Deduplicator Sep 22 '14 at 11:37
  • Also, you need to define `swap` in `Derived` as well, unless the one for `Base` is good enough – Deduplicator Sep 22 '14 at 11:40

2 Answers2

9

Consider using = default as much as possible. And if we are talking about public inheritance, you really need a virtual destructor as well.

Here is how your Base would look using the copy/swap style:

class Base
{
    int m_i;
public:
    virtual ~Base() = default;
    Base(const Base& other) = default;
    Base& operator=(Base other) noexcept
    {
        swap(*this, other);
        return *this;
    }
    Base(Base&& other) noexcept
        : Base(0)
    {
        swap(*this, other);
    }

    explicit Base(int i) noexcept
        : m_i{i}
        {}

    friend void swap(Base& a, Base& b) noexcept
    {
        using std::swap;
        swap(a.m_i, b.m_i);
    }
};

The only difference from what you have is that I've added the virtual destructor, and used = default for the copy constructor.

Now for Derived:

class Derived
    : public Base
{
    int m_j;
public:
    Derived(const Derived& other) = default;
    Derived& operator=(Derived other) noexcept
    {
        swap(*this, other);
        return *this;
    }
    Derived(Derived&& other) noexcept
        : Derived(0)
    {
        swap(*this, other);
    }

    explicit Derived(int j) noexcept
        : Base(42)
        , m_j{j}
        {}

    friend void swap(Derived& a, Derived& b) noexcept
    {
        using std::swap;
        swap(static_cast<Base&>(a), static_cast<Base&>(b));
        swap(a.m_j, b.m_j);
    }
};

I've let the compiler implicitly take care of the destructor since the compiler will implicitly give me a virtual one that does the right thing in this case.

Again I've explicitly defaulted the copy constructor. This corrects a bug in your version which neglects to copy Base.

The operator= looks just like the Base version.

The Derived move constructor does not need to move or copy anything from other since it is going to swap with other.

The Derived swap function must swap the Base part as well as the Derived part.


Now consider not using the copy/swap idiom. This can be surprisingly easier, and in some cases, higher performing.

For Base you can use = default for all 5 of your special members:

class Base
{
    int m_i;
public:
    virtual ~Base() = default;
    Base(const Base&) = default;
    Base& operator=(const Base&) = default;
    Base(Base&&) = default;
    Base& operator=(Base&&) = default;

    explicit Base(int i) noexcept
        : m_i{i}
        {}

    friend void swap(Base& a, Base& b) noexcept
    {
        using std::swap;
        swap(a.m_i, b.m_i);
    }
};

The only work that is really required here is your custom constructor and swap function.

Derived is even easier:

class Derived
    : public Base
{
    int m_j;
public:
    explicit Derived(int j) noexcept
        : Base(42)
        , m_j{j}
        {}

    friend void swap(Derived& a, Derived& b) noexcept
    {
        using std::swap;
        swap(static_cast<Base&>(a), static_cast<Base&>(b));
        swap(a.m_j, b.m_j);
    }
};

All 5 of the special members can be implicitly defaulted!

We couldn't default them in the Base because we needed to specify the virtual destructor, which inhibits the generation of the move members, and the generation of the copy members is deprecated with a user-declared destructor. But since we do not need to declare the destructor in Derived, we can just let the compiler handle everything.

As one of the big selling points of copy/swap is reduced coding, it can be ironic that using it can actually require more coding than letting the compiler default the special members.

Of course if the defaults do not do the right thing, then don't use them. I'm simply saying that the defaults should be your first choice, ahead of copy/swap.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
  • "Again I've explicitly defaulted the copy constructor. This corrects a bug in your version which neglects to copy Base." - Does that mean the default copy ctor calls the copy ctor of its base class? – gartenriese Sep 22 '14 at 15:46
  • 1
    @gartenriese: That is correct. All defaulted special members will iterate first over the bases, and then over the non-static data members, performing the indicated operation on each in turn. Oh, except the destructor performs this iteration in the reverse order of the constructors. – Howard Hinnant Sep 22 '14 at 15:51
  • Alright, thanks! BTW, I can't use the defaults because I have to copy some OpenGL stuff. – gartenriese Sep 22 '14 at 15:56
  • "The Derived move constructor does not need to move or copy anything from other since it is going to swap with other", - doesn't swap actually move things? Or did you mean "through a direct call"? – Oleksa Jul 09 '20 at 12:41
  • A why do you need to implement swap if you use default versions of copy and move constructors? – Oleksa Jul 09 '20 at 12:47
  • I meant through a direct call. – Howard Hinnant Jul 09 '20 at 14:02
  • I wanted to include the complete public API of the OP's example which includes a swap. – Howard Hinnant Jul 09 '20 at 14:03
4

You implement op= exactly the same way for Derived as for Base:

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

I hope you are aware of the up- and down-sides of passing the argument by value there, though:

  • Up-side: Only one function needed for all value categories.
  • Down-Side: Second move for xvalues, move in addition to the needed copy for prvalues.

Other points to consider:

  • Rule-of-thumb: Single-argument non-copy/move ctors should be explicit: You really don't want to have an implicit conversion from int to Base...
  • You forgot to re-implement swap for Derived (swap all sub-objects, both base and member). You might forego it if Derived does not add any members though.
Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
Deduplicator
  • 44,692
  • 7
  • 66
  • 118
  • 1
    @Jon: But I like to thump with my thumb ;-) (Thanks.) – Deduplicator Sep 22 '14 at 11:50
  • haha! C++ does have several rules of thumb that one should get thumped for not following! :-) – Jonathan Wakely Sep 22 '14 at 11:54
  • So you don't call any ctor of Base at all and do all the necessary things in Derived? – gartenriese Sep 22 '14 at 11:56
  • @gartenriese: Yes, at least not directly. (All Derived-ctors call a Base-ctor, and Derived::swap needs to call Base::swap) – Deduplicator Sep 22 '14 at 14:26
  • But if I call Base::swap from Derived::swap, don't I swap Base twice when calling the move ctor of Derived? The first time in the move ctor of Base and the second time in the move ctor of Derived. – gartenriese Sep 22 '14 at 14:33
  • You need to swap each sub-object of the `Derived`, and at the moment you don't, you are only swapping the member- and not the base-subobject. `swap` is a normal function not a ctor or dtor, which magically will call the base-versions too. – Deduplicator Sep 22 '14 at 14:36
  • But you said I'll have to call Base::swap inside Derived::swap. So when I call the move ctor of Derived it will first call the move ctor of Base which will call Base::swap. When the move ctor of Base is done, it goes inside the move ctor of Derived which will call Derived::swap. Which, also, will call Base::swap. Thus Base::swap will be called twice? Am I wrong? – gartenriese Sep 22 '14 at 14:42
  • 1
    Ah, your derived move-ctor is simply wrong: It needs to cheaply default-init the object and then swap everything. (If the default-ctor or swap is not cheap, it needs to be custom-coded). Which leads me to a question: Why don't you have a default-ctor? (Maybe add a default to ctor(int)) – Deduplicator Sep 22 '14 at 14:45
  • Ah, thanks, fixed my code in the question. EDIT: I did not think I would need a default ctor. Do I? – gartenriese Sep 22 '14 at 14:50
  • That's for you to decide. For copy&swap idiom applied to move-ctor, so cheap-init and swap, you just need a way to cheaply init a zombie-object. – Deduplicator Sep 22 '14 at 15:02