2

I've got an argument with my colleague.

We have a class that is a member of a namespace (which is a member of the other namespace but that is not important here I think). In the header file, we nest the class block into the namespace block like this:

namespace NS {
    class A {
        void method();
        // ...
    };
}

And here is the .cpp file and the subject of our argument. I wrote:

using namespace NS;

void A::method() {
    // ...
}

The colleague told me that I use the 'using' directive improperly, and I should have used here the same NS { ... } as in the header. (He's modified the code and got some compiler errors that he only managed to get rid of by removing the using directive and surrounding the code with NS { ... }.)

My point is that 'using' just affects the name lookup so A is looked for in the NS namespace, so my approach is correct, and his compiler problems were caused by something else but not by that 'using' directive.

Who is right and why?

Added: guys, please don't answer like 'I did this (or that) way many times', that's not much useful. We need the theory here: why this or that approach is right or wrong.

Alexander Dunaev
  • 980
  • 1
  • 15
  • 40

6 Answers6

4

Your colleague is correct, you should wrap cpp with namesapce which means you define your funcitons inside namesapce NS.

namespace NS
{    
  void A::method() 
  {
    // ...
  }
}

You may get conflict name lookup if you have multiple A available.

The dangerous thing happening here is that the using declaration takes a snapshot of whatever entities named A::method in namespace NS have been seen by the time the using declaration is encountered.

Have a read of Google cpp style guilde regarding namespace, also 101 c++ coding standards

billz
  • 44,644
  • 9
  • 83
  • 100
3

Both of you are right in different ways. You are right in that the using directive will let the compiler resolve A in void A::method() to be NS::A, but he is also right in that in most cases you are better off opening the namespace.

The reason is that not everything that is declared in the namespace in the header will be looked up, and for those elements you will need to open the namespace (or provide the qualification manually), so it makes sense to provide a uniform approach.

A common example is the definition of operators that are applied to the type:

// header
namespace NS {
   class A { ... };
   std::ostream& operator<<(std::ostream&,A const&);
}
// implementation file (incorrect):
using namespace NS;
std::ostream& operator<<(std::ostream& o, A const & a) {
   // do something
}

The problem here is that function definitions (when the function name is not a qualified name) are self-declaring. In the header the operator is declared as ::NS::operator<<, but in the implementation file it is defined as std::ostream& ::operator<<(std::ostream&, NS::A const&). The using directive will allow the resolution of the A identifier to be NS::A, but it won't make that function definition reside in the NS namespace.

This leads to a subtle problem, where any use of that operator will find ::NS::operator<< due to ADL, but that operator is not defined anywhere (although there is a similar operator in a different namespace)

Having said all that, there are some coding conventions (where I work now has such a guideline) that require namespaces for all types and that free function definitions should be performed outside of the namespace and always qualified to avoid self declarations. In the example above, the operator would be defined in the implementation file as:

std::ostream& ::NS::operator<<(std::ostream& o, A const& a) { ... }

But unless you really get used to always qualifying, this is prone to errors if you forget the qualification of the function.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
1
namespace NS {
    class A {
        void method();
        // ...
    };
}

Always done it that way...

Justin Donohoo
  • 380
  • 2
  • 14
1

It turns out it's not only "coding-style matter". Using namespace ... leads to linking error when defining and initializing a variable declared extern in header file. Take a look at example in my question. Definition of constant within namespace in cpp file

Community
  • 1
  • 1
jakumate
  • 115
  • 8
0

You are correct. I've been bitten by this a number of times (that is, bitten by thinking that using declarations don't do this). There are three ways you could write this, and all will work:

namespace NS
{
    void A::method()
    { }
}

Or:

NS::A::method() { }

Or:

using namespace NS;

A::method() { }

However, in terms of style, I would suggest either the first or the second - using declarations can get hairy when you have multiple classes with the same name and methods around.

Yuushi
  • 25,132
  • 7
  • 63
  • 81
0

It is considered good practice not to use 'using namespace' in code declarations or libraries but its OK to use them for the actual program's code.

Ie: dont use it for the Cpp definition but you can use it when your calling the function/class outside.

This is all just good coding practice and unless other people are using your code it matters more what you want.

The best is to put the cpp code into Namespace { ... } for readability

Sellorio
  • 1,806
  • 1
  • 16
  • 32