C++ swap problem in inheritance scenario

Question

I want to add swap functionality to two existing C++ classes. One class inherits from the other. I want each classes' instances to only be swappable with instances of the same class. To make it semi-concrete, say I have classes Foo and Bar. Bar inherits from Foo. I define Foo::swap(Foo&) and Bar::swap(Bar&). Bar::swap delegates to Foo::swap. I want Foo::swap to only work on Foo instances and Bar::swap to only work on Bar instances: I can't figure out how to enforce this requirement.

Here's a sample of what's giving me trouble:

#include <algorithm>
#include <iostream>

struct Foo {
    int x;
    Foo(int x) : x(x) {};

    virtual void swap(Foo &other) {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
        std::swap(this->x, other.x);
    };
};

struct Bar : public Foo {
    int y;
    Bar(int x, int y) : Foo(x), y(y) {};

    virtual void swap(Bar &other) {
        std::cout << __PRETTY_FUNCTION__ << " ";
        Foo::swap(other);
        std::swap(this->y, other.y);
    };
};

void display(Foo &f1, Foo &f2, Bar &b34, Bar &b56)
{
    using namespace std;

    cout << "f1:  " << f1.x                  << endl;
    cout << "f2:  " << f2.x                  << endl;
    cout << "b34: " << b34.x << " " << b34.y << endl;
    cout << "b56: " << b56.x << " " << b56.y << endl;
}

int main(int argc, char **argv)
{
    {
        Foo f1(1), f2(2);
        Bar b34(3,4), b56(5,6);
        std::cout << std::endl << "Initial values: " << std::endl;
        display(f1,f2,b34,b56);
    }

    {
        Foo f1(1), f2(2);
        Bar b34(3,4), b56(5,6);
        std::cout << std::endl << "After Homogeneous Swap: " << std::endl;
        f1.swap(f2);             // Desired
        b34.swap(b56);           // Desired
        display(f1,f2,b34,b56);
    }

    {
        Foo f1(1), f2(2);
        Bar b34(3,4), b56(5,6);
        std::cout << std::endl << "After Heterogeneous Member Swap: " << std::endl;
        // b56.swap(f2);         // Doesn't compile, excellent
        f1.swap(b34);            // Want this to not compile, but unsure how
        display(f1,f2,b34,b56);
    }

    return 0;
}

Here's the output:

Initial values: 
f1:  1
f2:  2
b34: 3 4
b56: 5 6

After Homogeneous Swap: 
virtual void Foo::swap(Foo&)
virtual void Bar::swap(Bar&) virtual void Foo::swap(Foo&)
f1:  2
f2:  1
b34: 5 6
b56: 3 4

After Heterogeneous Member Swap: 
virtual void Foo::swap(Foo&)
f1:  3
f2:  2
b34: 1 4
b56: 5 6

You can see in the final output group where f1.swap(b34) "sliced" b34 in a potentially nasty way. I'd like the guilty line to either not compile or blow up at runtime. Because of the inheritance involved, I think I run into the same problem if I use a nonmember or friend swap implementation.

The code is available at codepad if that helps.

This use case arises because I want to add swap to boost::multi_array and boost::multi_array_ref. multi_array inherits from multi_array_ref. It only makes sense to swap multi_arrays with multi_arrays and multi_array_refs with multi_array_refs.

Answer 1

(Somewhat hacky solution)

Add a protected virtual method, isBaseFoo(), make it return true in Foo, and false in Bar, the the swap method for Foo could check it's argument has isBaseFoo()==true.

Evil, and detects the problem only at run-time, but I can't think of anything better, although Charles Bailey's answer might be better, if you allow dynamic_cast<>.

Answer 2

Swap, like assignment and comparison work well with value types and don't work well with bases of class hierarchies.

I've always found it easiest to follow the Scott Meyer's Effective C++ recommendation of not deriving from concrete classes and making only leaf classes concrete. You can then safely implement swap, operator==, etc. as non-virtual functions for leaf nodes only.

While it's possible to have a virtual swap functions the whole point of having virtual base classes is to have dynamic behaviour at runtime so I think you're on to a loser trying to get all incorrect possibilities to fail at compile time.

If you want to go the virtual swap route, then one possible approach is to do something like this.

class Base
{
public:
    virtual void Swap(Base& other) = 0;
};

class ConcreteDerived
{
    virtual void Swap(Base& other)
    {
        // might throw bad_cast, in this case desirable
        ConcreteDerived& cother = dynamic_cast<ConcreteDerived&>(other);bad_cast
        PrivateSwap(cother);
    }

    void PrivateSwap(ConcreteDerived& other)
    {
        // swap implementation
    }
};

Answer 3

You cannot really do this, but I don't see the point, anyway. It's no worse than slicing on operator= or copy constructors, and you cannot avoid the latter, either. Why should swap be any different?

For the same reason, it's likely not worth it making swap virtual, for the same reason why you don't make operator= virtual.

Answer 4

I think that this case scenario is now addressed by the presence of move semantics in C++11. I usually use the swap function only to avoid copy duplication in the assignments so it's only used statically, by the derived class that needs to extend the swap function and that knows the static type of its base so there is no need for virtuality (which as it was stated can lead to subtle problems with slicing). In fact I declare the swap function as a protected method in my base to make sure it's not use directly anywhere else. The most concrete class can then have the final version public if necessary.

Answer 5

What you are actually trying to do is swap instances of classes from a third party inheritance hierarchy. Given that, I'd forget about using swap on the actual classes and add a level of indirection. Using boost::shared_ptr is a good approach; use shared_ptr instances containing whatever class you want, and swap to your heart's content.

In general, solving the problem as you asked it at compile time is hard for all the reasons described by the other answerers.