Is it OK to use a macro to specialize std::swap?

Question

So, I have a macro.

// swap_specialize.hpp
#include <algorithm>

#ifndef STD_SWAP_SPECIALIZE
#define STD_SWAP_SPECIALIZE( CLASSNAME )      \
    namespace std {         \
    template<> inline                 \
    void swap( CLASSNAME & lhs, CLASSNAME & rhs ) \
    { lhs.swap(rhs); } }
#endif

So then I have a class

// c.hpp
#include <vector>
#include "swap_specialize.hpp"
class C
{
    public:
        C();

        void swap(C& rhs)
        {
            data_.swap(rhs.data_);
        }
        C& operator=(C rhs)
        {
            rhs.swap(*this);
            return *this;
        }
    private:
        std::vector<int> data_;
}

STD_SWAP_SPECIALIZE(C)

Is it bad, stylistically, to do this? Is it a code smell? Or is it an OK practice?

Answer 1

I don't see it buying you much. For non-template classes, it saves you very few lines. And for template classes, when you want to specialize generically (i.e. for all T), it just won't work.

Answer 2

Assuming you are doing to use STD_SWAP_SPECIALIZE() for a number of other classes, I this this is perfectly reasonable. After all it's much more readable to have a series of

STD_SWAP_SPECIALIZE(Foo)
STD_SWAP_SPECIALIZE(Bar)
STD_SWAP_SPECIALIZE(Baz)

Than the equivilent expanded code. Also, if the expansion of STD_SWAP_SPECIALIZE was a little larger, then the macro definition gives you the code in a single place if it needed changing. (As it is the template definition is pretty small in your example then it's probably a moot point).

Answer 3

I would say it's OK if it increases readability. Judge yourself. Just my two cents: Specializing std::swap isn't really the right way to do this. Consider this situation:

my_stuff::C c, b;
// ...
swap(c, b);
// ...

This won't find std::swap if you haven't done using std::swap or something similar. You should rather declare your own swap in C's namespace:

void swap(C &a, C &b) {
  a.swap(b);
}

Now, this will work also in the above case, because argument dependent lookup searches in the namespace of the class. Code swapping generic things where the type isn't known should do it like this:

using std::swap;
swap(a, b);

Regardless of the type, this will use the best matching swap, and fall-back to std::swap if there wasn't a better matching one in the namespaces of a. Hard-coding the call to std::swap will cut too short on types that don't specialize std::swap but rather decide to provide their own swap in their namespace.

This is superious in another way: Imagine C is a template. You cannot specialize std::swap in this case. But just defining your own swap, that's perfectly fine.

template<typename T>
void swap(C<T> &a, C<T> &b) {
  a.swap(b);
}

This is the way how the swap for std::string and other classes is implemented too.