Cevik
Cevik

Reputation: 333

Best practice with argument dependent name lookup

today i face a problem that hurts my peace of mind. I have resumed my problem in a very smart and meaningfull example where the expected result is not met although no error is generated.

#include <iostream>


namespace MATH {


template <std::size_t M, std::size_t N, class T>
class Matrix
{

};

void invert(Matrix<2, 2, double>& m)
{
    std::cout << "DIM 2 : MATH version (use of the determinant)" << std::endl;
}

void invert(Matrix<3, 3, double>& m)
{
    std::cout << "DIM 3 : MATH version (use of the determinant)" << std::endl;
}


}


namespace GEOM {


template <std::size_t N>
using Matrix = MATH::Matrix<N, N, double>;// orthonormal set of vectors

template <std::size_t N>
void invert(Matrix<N>& m)
{
    std::cout << "DIM " << N << " : GEOM version (use of the transpose)" << std::endl;
}

void geom_foo_purpose(Matrix<3>& m)
{
    invert(m);
}


}


int main(int argc, char **argv)
{
    GEOM::Matrix<3> m;
    GEOM::geom_foo_purpose(m);

    return 0;
}

output : std::cout << "DIM 3 : MATH version (use of the determinant)" << std::endl;

In geom_foo_purpose definition, the call to invert results in an unqualified id because a template deduction is requested. So, the ADL is able to say : ok, let's look at the MATH namespace. The fact that allows MATH::invert to be prefered to the GEOM::invert version because the non template version has priority is inadmissible in this case i think.

For example, i want to develop the GEOM content first, by defining the Matrix class as a GEOM type. GEOM::invert is called. No problem. One day, i want to generalize my Matrix class in another namespace MATH and i could think : ok, i keep the same methods and i don't break the code. Just put a using MATH::Matrix... And i become unable to understand where is the performance overhead nor why some sensitive measures change.

So i actually think about three solutions :

Is there a decent way to overcome this ?

Upvotes: 1

Views: 256

Answers (2)

Ralph Tandetzky
Ralph Tandetzky

Reputation: 23600

General advice

It is best practice to keep classes and functions that are designed to work together in one namespace to enable ADL. (See Item #57 in C++ Coding Standards: Keep a type and its nonmember function interface in the same namespace.) Sometimes it is recommended to keep unrelated classes and functions in separate namespaces. (See Item #58 in the same book.) I find this too restrictive, because it creates a huge namespace jungle very quickly.

Common practice

It is more common practice to put all stuff that belongs logically to one library into one namespace. It's the libraries responsibility to avoid internal name clashes. Users of the library should usually not add new stuff to a libraries namespace. That's pretty much how the standard library, boost, OpenCV and a host of other libraries work. That's what I recommend. Don't try to overseparate. It just hurts your head and is not necessary.

Namespace usage

These common libraries use nested namespaces for some purposes, like boost::detail for implementation details, that should not be of interest to the user of the library. Other examples where more namespaces are necessary are

  • inline namespaces for versioning and
  • namespaces to select a set of user-defined literals and
  • possibly some others.

EDIT:

Advice for your situation

If I understand correctly, your GEOM::Matrix is supposed to be an orthogonal matrix. (I just noticed that very late and would suggest a clearer name for it.) That should be a different type, this type has to ensure the internal invariant of being orthogonal. Using using or typedef just does aliasing and there's no way for you to guarantee, that the invariant if broken through some MATH::Matrix functions. Therefore, I would suggest composition as the means to solve the problem:

namespace GEOM
{

template <std::size_t N, typename T>
class OrthogonalMatrix
{
private:
    MATH::Matrix<N,N,T> mat;

public:
    // Enable use of const interface of MATH::Matrix<N,N,T>. 
    operator const MATH::Matrix<N,N,T> &() const { return mat; }

    // ... here goes the implementation ensuring 
    // orthogonality at all times.
};

} // end of namespace GEOM

An alternative would be private inheritance, which enables you to make MATH::Matrix methods public selectively with the keyword using like this:

namespace GEOM
{

template <std::size_t N, typename T>
class OrthogonalMatrix : private MATH::Matrix<N,N,T>;
{
public:
    // Make `MATH::Matrix<N,N,T>::transposeInPlace` publicly accessible
    using MATH::Matrix<N,N,T>::transposeInPlace;

    // Enable use of const interface of MATH::Matrix<N,N,T>. 
    operator const MATH::Matrix<N,N,T> &() const { return *this; }

    // ... here goes the implementation ensuring 
    // orthogonality at all times.
};

} // end of namespace GEOM

You should not do that with the constructor or entry access functions, since they would enable the user of the class to break invariants which can easily lead to a lot of confusion. Also public inheritance plus using in the private section would be wrong, because the user of the class would still be able to break invariants by converting to MATH::Matrix<N,N,T> & and messing up the matrix then.

Upvotes: 0

Christophe
Christophe

Reputation: 73366

The problem is that your GEOM::Matrix is only a synonym of MATH::Matrix, and not a new type in GEOM. This is the intended effect of a using statement, and therefore the argument dependent name lookup finds the MATH::invert() as the best match.

If you want to fix this, define a real GEOM::Matrix this way:

    template <std::size_t N>
    class Matrix : public MATH::Matrix<N, N, double> {};// orthonormal set of vectors

Here an online demo.

Edit:

There's a fundamental design issue that you have to decide on:

  • Either you want to have distinct (yet somewhat interchangeable) Matrix types and benefit from ADL. In this case you manage two types and can have of course to manage some conversions (from GEOM to MATH).
  • Or you want to have one single Matrix type (defined in one namespace) with specialisation for some parameters (in the same namespace, i.e. the transposition inversion would migrate to MATH).

Boh have their advantages and inconvenience. The choice is yours. Personally I'd prefer the second option: all the matrix specific operations would then better isolated. And as you say, a matrix is a matrix. But apparently, you've choosen the second option, and my answer tries to provide you some solutions for that.

Upvotes: 1

Related Questions