// Inside a btree method, if we just call swap(), it will choose the
// btree::swap method, which we don't want. And we can't say ::swap
// because then MSVC won't pickup any std::swap() implementations. We
// can't just use std::swap() directly because then we don't get the
// specialization for types outside the std namespace. So the solution
// is to have a special swap helper function whose name doesn't
// collide with other swap functions defined by the btree classes.
I don't get whats wrong. They have an implementation of swap in their namespace already they don't want to use, the standard one isn't templated to work with their btree, so you use a different swap in methods.
Sounds more like this should be btree::swap and whatever they have as swap() should be something else.
I think the syntax gymnastics needed to achieve the result are quite complex for what they aim for. There's nothing technically wrong with it. It's just that choosing a function from a namespace shouldn't need a 7 line comment about how and why in my opinion. It's a single statement function.
Additionally `using std::swap; swap();` not being exactly equivalent to `std::swap();` is rather strange (even if it can be explained). A candidate for code/design smell in my opinion.
> I think the syntax gymnastics needed to achieve the result are quite complex for what they aim for. There's nothing technically wrong with it. It's just that choosing a function from a namespace shouldn't need a 7 line comment...
It's probably important to point out that your last sentence above is not correct. That's not what this code is doing. If all it needed to do was call swap from a particular namespace, it would be as simple as you imply. It's trying to maximize the caller's ability to specialize swap for the type he's using in the template, while still allowing for std::swap to be used as a fallback.
> Additionally `using std::swap; swap();` not being exactly equivalent to `std::swap();` is rather strange (even if it can be explained).
One says "use standard swap". One says "bring std::swap into this namespace and choose the best swap". I agree with you the difference is subtle and non-obvious. But it is important when doing sufficiently generic programming.
There is nothing wrong with that code. The comment is strange, but the code is fine. 'swap' function call has to be resolved via ADL and this is how it's done correctly. It's unfortunate that the C++ Standard essentially requires swap to be called this way (i.e.: using std::swap; swap(a, b);). In your code it's best factored out into a separate 'swap_helper' function and this is what's done there.
C++ is convoluted. Sometimes code that seems quite wrong has a good reason to exist.
