127

Probably a duplicate, but not an easy one to search for...

Given a header like:

namespace ns1
{
 class MyClass
 {
  void method();
 };
}

I've see method() defined in several ways in the .cpp file:

Version 1:

namespace ns1
{
 void MyClass::method()
 {
  ...
 }
}

Version 2:

using namespace ns1;

void MyClass::method()
{
 ...
}

Version 3:

void ns1::MyClass::method()
{
 ...
}

Is there a 'right' way to do it? Are any of these 'wrong' in that they don't all mean the same thing?

Mr. Boy
  • 60,845
  • 93
  • 320
  • 589
  • In most of the codes I usually see third version (but I don't why :D) , second version is simply opposite to why namespaces are introduced I guess. – Mr.Anubis Dec 30 '11 at 16:44
  • 1
    As in interesting fact, Visual Assist refactoring tools are happy to generate code using either #1 or #3, depending which style is already in use in the target file. – Mr. Boy Dec 31 '11 at 15:48

8 Answers8

65

Version 2 is unclear and not easy to understand because you don't know which namespace MyClass belongs to and it's just illogical (class function not in the same namespace?)

Version 1 is right because it shows that in the namespace, you are defining the function.

Version 3 is right also because you used the :: scope resolution operator to refer to the MyClass::method () in the namespace ns1. I prefer version 3.

See Namespaces (C++). This is the best way to do this.

Paul Roub
  • 36,322
  • 27
  • 84
  • 93
GILGAMESH
  • 1,816
  • 3
  • 23
  • 33
  • 24
    Calling #2 "wrong" is a huge exaggeration. By this logic, all symbol names are "wrong" because they can potentially hide other symbol names in other scopes. – tenfour Dec 30 '11 at 16:59
  • 1
    It's illogical. It will compile fine (sorry, misexplained that in the answer), but why would you define a function outside it's namespace? It confuses the reader. Also, when many namespaces are used, it does not show which namespace MyClass belongs to. Versions 1&3 fix this problem. In conclusion, it's not wrong, but just unclear and confusing. – GILGAMESH Dec 30 '11 at 16:59
  • A good habit is to avoid it. When working on a team, it will be confusing for others. It's just not a lot of people use it that way and not a lot of people understand it that way. – GILGAMESH Dec 30 '11 at 17:08
  • 3
    I agree with @PhoenicaMacia, the *using* trick is awful and can lead to confusion. Consider a class that implements an operator as a free function, in the header you would have `namespace N {struct X { void f(); }; X operator==( X const &, X const & ); }`, now in the cpp file with the *using* statement you can define the member function as `void X::f() {}`, but if you define `X operator==(X const&, X const&)` you will be defining a different operator from the one defined in the header (you will have to use either 1 or 3 for the free function there). – David Rodríguez - dribeas Dec 30 '11 at 19:32
  • 1
    In particular I prefer 1, and the example in the linked article does not really resolve anything the first example uses 1, the second uses a mix of 1 and 3 (the functions are defined with qualification, **but** they are defined inside the outer namespace) – David Rodríguez - dribeas Dec 30 '11 at 19:35
  • 1
    Of the 3 I would say 1) is the best however most IDEs have the rather annoying habit of indenting everything inside that namespace declaration and it does add some confusion. – locka Aug 18 '14 at 10:57
  • Do exist real problems involving the use of method #2 (though I prefer #1 and #3) ? – Moia Apr 27 '18 at 07:07
  • @Moia Method #2 only works for defining class member function (no friend or regular functions declared in the namespace can be defined this way), and it only sort of works. As soon the class name becomes ambiguous — for example, because you included a header that declares an identically named class in the global namespace (or a namespace you are `using`), your code will stop compiling. – agurtovoy Feb 02 '19 at 01:39
36

5 years later and i thought I'd mention this, which both looks nice and is not evil

using ns1::MyClass;

void MyClass::method()
{
  // ...
}
Puzomor Croatia
  • 781
  • 7
  • 16
  • 4
    This is the best answer. It looks the cleanest, and avoids the issues with OP's Versions 1, which may bring things into the namespace unintentionally, and 2, which may bring things into the global space unintentionally. – ayane_m Nov 26 '16 at 07:57
  • Yes, this is a great combination of less typing than 3, whilst still explicitly declaring intent. – j b Jan 31 '20 at 08:33
  • What if you don't have class methods and just non-class methods. – Ruolin Liu May 11 '21 at 01:20
  • Personally, I still mostly use version 1, which also works for non-class methods. But being completely honest, I no longer use namespaces or classes split across header/source files anymore, so there's that. – Puzomor Croatia Feb 03 '22 at 23:08
18

I'm using version 4 (below) because it combines most of the advantages of version 1 (terseness of the resoective definition) and version 3 (be maximally explicit). The main disadvantage is that people aren't used to it but since I consider it technically superior to the alternatives I don't mind.

Version 4: use full qualification using namespace aliases:

#include "my-header.hpp"
namespace OI = outer::inner;
void OI::Obj::method() {
    ...
}

In my world I'm frequently using namespace aliases as everything is explicitly qualified - unless it can't (e.g. variable names) or it is a known customization point (e.g. swap() in a function template).

Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
  • 1
    While I think the "it's better so I don't care if it confuses people" logic is flawed, I have to agree this _is_ a good approach for nested namespaces. – Mr. Boy Dec 31 '11 at 15:51
  • 1
    +1 for the excellent "why-didn't-I-think-of-that" idea! (As for "people aren't used to [new techically superior things]", they'll get used to it if more people do it.) – wjl Feb 17 '14 at 19:25
  • Just to make sure I understand, are both `outer` and `inner` defined as namespaces in other header files already? – dekuShrub Mar 11 '19 at 06:15
9

Googles C++ Style Guide dictates your version 1, without indentation though.

Googles C++ Style Guide for namespaces

Bjarke Freund-Hansen
  • 28,728
  • 25
  • 92
  • 135
4

Version 3 makes the association between the class and the namespace very explicit at the expense of more typing. Version 1 avoids this but captures the association with a block. Version 2 tends to hide this so I'd avoid that one.

Paul Joireman
  • 2,689
  • 5
  • 25
  • 33
4

I choose Num.3 (a.k.a. the verbose version). It's more typing, but the intent is exact to you and to the compiler. The problem you posted as-is is actually simpler than the real world. In the real world, there are other scopes for definitions, not just class members. Your definitions aren't very complicated with classes only - because their scope is never reopened (unlike namespaces, global scope, etc.).

Num.1 this can fail with scopes other than classes - anything that can be reopened. So, you may declare a new function in a namespace using this approach, or your inlines could wind up being substituted via ODR. You will need this for some definitions (notably, template specializations).

Num.2 This is very fragile, particularly in large codebases - as headers and dependencies shift, your program will fail to compile.

Num.3 This is ideal, but a lot to type - what your intent is to define something. This does exactly that, and the compiler kicks in to make sure you've not made a mistake, a definition is not out of synch with its declaration, etc..

justin
  • 104,054
  • 14
  • 179
  • 226
3

It turns out it's not only "coding-style matter". Num. 2 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
1

All the ways are right, and each one has its advantages and disadvantages.

In the version 1, you have the advantage of not having to write the namespace in front of each function. The disadvantage is that you'll get a boring identation, specially if you have more than one level of namespaces.

In version 2, you make your code cleaner, but if you have more than one namespace being implemented in the CPP, one may access the other one's functions and variables directly, making your namespace useless (for that cpp file).

In version 3, you'll have to type more and your function lines may be bigger than the screen, which is bad for design effects.

There is also another way some people use it. It is similar to the first version, but without the identation problems.

It's like this:

#define OPEN_NS1 namespace ns1 { 
#define CLOSE_NS1 }

OPEN_NS1

void MyClass::method()
{
...
}

CLOSE_NS1

It's up to you to chose which one is better for each situation =]

Renan Greinert
  • 3,376
  • 3
  • 19
  • 29
  • 17
    I don't see any reason to use a macro here. If you don't want to indent, just don't indent. Using a macro just makes the code less obvious. – tenfour Dec 30 '11 at 16:57
  • 1
    I think the last version you mention is useful whenever you want to compile you code with old compilers that do not support namespaces (Yes, some dinosaurs are still around). In that case you can put the macro inside an `#ifdef` clause. – Luca Martini Dec 30 '11 at 17:08
  • You don't have to ident if you don't want, but if you don't use macros, some IDEs will try to do that for you. For example, in Visual Studio you can select the whole code and press ALT+F8 to auto-ident. If you don't use the defines, you'll lose that functionality. Also, I don't think that OPEN_(namespace) and CLOSE_(namespace) is less obvious, if you have it in your coding standard. The reason @LucaMartini gave is also interesting. – Renan Greinert Dec 30 '11 at 18:00
  • If this was made generic i.e. `#define OPEN_NS(X)` I think it's _slightly_ useful, but not really... I don't object to macros but this seems a bit OTT. I think Dietmar Kühl's approach is better for nested namespaces. – Mr. Boy Dec 31 '11 at 15:50