Swap-Idiom of abstract class with private data members

Question

Assume we have this two classes, with implemented swap-idioms. The copy constructor and assignment operator of the base class are deleted, as it makes no sense. However the swap-method is implemented, as it holds a member.

namespace std
{
    template<>
    void swap<Base>(Base& a, Base& b)
    {
        a.swap(b);
    }

    template<>
    void swap<Derived>(Derived& a, Derived& b)
    {
        a.swap(b);
    }
}

class Base
{
public:
    Base(int ID) : ID_(ID) {}
    virtual std::string getString()=0;

    Base(const Base&)=delete;
    operator=(const Base&)=delete;

    void swap(Base& rhs)
    {
        std:swap(ID_, rhs.ID_);
    }
private:
    int ID_;
}

class Derived : public Base
{
public:
    Derived(int ID, bool Value) : Base(ID), Value_(Value) {}
    virtual ~Derived() {}
    Derived(Derived& rhs)
    {
        std::swap(*this, rhs);
    }

    virtual std::string getString() {return Value_ ? "True" : "False"}
    void swap(Derived& lhs, Derived& rhs)
    {
        std::swap(static_cast<Base&>(lhs), static_cast<Base&>(rhs);
        std::swap(lhs.Value_, rhs.Value_);
    }
private:
    bool Value_;
}

As seen in many examples, this would be the standard way to do it I suppose. However, I see a problem with the public Base::swap, as it should not be possible to swap only the abstract base-class!

Wouldn't it be better to remove the template for the Base class and make the Base::swap Method protected:

class Base
{
    ...
protected:
    void swap(Base& rhs, Base &lhs);
}

class Derived : public Base
{
    ...
public:
    void swap(Derived& lhs, Derived& rhs)
    {
        Base::swap(static_cast<Base&>(lhs), static_cast<Base&>(rhs);
        std::swap(lhs.Value_, rhs.Value_);
    }
}

Assuming, there is another class derived from base, with the first implementation it would be possible to swap the ID, however the data members of the derived objects would stay the same.

So, am I right thinking that swapping of a abstract class should not be possible from outside?

Note: If you define a swap function for your class, you should declare it as an overload in the namespace in which your class is declared, not a specialization of `std::swap`. — Brian Bi, Sep 02 '15 at 20:47
See http://stackoverflow.com/questions/11562/how-to-overload-stdswap — Brian Bi, Sep 02 '15 at 20:53
You are right in that private datamembers of a class should only be accessed by a swap that is allowed to access those data members... — , Sep 02 '15 at 21:11

score 2 · Answer 1 · edited May 23 '17 at 11:50

The copy constructor and assignment operator of the base class are deleted, as it makes no sense.

Actually, this is terrible. Now you made Derived uncopyable! Why? There is no reason to add this restriction. The default copy constructor and assignment operator of Base are perfectly reasonable in the context of copying the most-base class of the hierarchy.

Once you undo that, there's no need to do anything else as std::swap(derived1, derived2) would already do the right thing. The default move construction/operation is correct. It's always good to let the compiler do things for you.

But if you want to override swap anyway, the correct way to do that would be as a non-member friend:

class Base { 
   ...
   friend void swap(Base& lhs, Base& rhs) {
       using std::swap;
       swap(lhs.ID_, rhs.ID_);
   }
};

class Derived : public Base {
   ...
   friend void swap(Derived& lhs, Derived& rhs) {
       using std::swap;
       swap(static_cast<Base&>(lhs), static_cast<Base&>(rhs));
       swap(lhs.Value_, rhs.Value_);
   }
};

Also, your Derived copy constructor makes no sense. Remove it as per the first paragraph of my answer.

Obviously you are right about the non-member friend, thanks. However, think about another derived class, holding a int instead of a bool, and the ID somehow mapped to a database table holding generic values, it would certainly _not_ be reasonable to be able to modify the Base information without information about the derived class. — , Sep 02 '15 at 21:56
Useful to note that `static_cast` only works in the free standing swap function and not in `Derived::swap(Derived& rhs)` — Fred Schoen, Apr 12 '16 at 08:21

Swap-Idiom of abstract class with private data members

1 Answers1