views:

933

answers:

7

I'm curious about the benefits/detriments of different constant declaration and definition options in C++. For the longest time, I've just been declaring them at the top of the header file before the class definition:

//.h
const int MyConst = 10;
const string MyStrConst = "String";
class MyClass {
...
};

While this pollutes the global namespace (which I know is a bad thing, but have never found a laundry list of reasons why it is bad), the constants will still be scoped to individual translation units, so files that don't include this header won't have access to these constants. But you can get name collisions if other classes define a constant of the same name, which is arguably not a bad thing as it may be a good indication of an area that could be refactored.

Recently, I decided that it would be better to declare class specific constants inside of the class definition itself:

//.h
class MyClass {
    public:
         static const int MyConst = 10;
...
    private:
         static const string MyStrConst;
...
};
//.cpp
const string MyClass::MyStrConst = "String";

The visibility of the constant would be adjusted depending on whether the constant is used only internally to the class or is needed for other objects that use the class. This is what I'm thinking is the best option right now, mainly because you can keep internal class constants private to the class and any other classes using the public constants would have a more detailed reference to the source of the constant (e.g. MyClass::MyConst). It also won't pollute the global namespace. Though it does have the detriment of requiring non-integral initialization in the cpp file.

I've also considered moving the constants into their own header file and wrapping them in a namespace in case some other class needs the constants, but not the whole class definition.

Just looking for opinions and possibly other options I hadn't considered yet.

+1  A: 

You can declare them as globals in the c++ file, as long as they are not referenced in the header. Then they are private to that class and won't pollute the global namespace.

Alex Brown
Yes, but you have to use `static` or an anonymous namespace in this case - if you don't, in some implementations you pollute the global namespace used by the linker and get name clashes on linking...
hjhill
+2  A: 

Pollution of the global namespace is bad because someone (e.g. the writer of a library you use) might want to use the name MyConst for another purpose. This can lead to severe problems (libraries that can't be used together etc.)

Your second solution is clearly the best if the constants are linked to a single class. If that isn't so easy (think of physical or math constants without ties to a class in your program), the namespace solution is better than that. BTW: if you must be compatible to older C++ compilers, remember some of them can't use integral initialization in a header file - you must initialize in the C++ file or use the old enum trick in this case.

I think there are no better options for constants - at least can't think of one at the moment...

hjhill
+2  A: 

Personally I use your second approach; I've used it for years, and it works well for me.

From a visibility point I would tend to make the private constants file level statics as nobody outside the implementation file needs to know they exist; this helps prevent chain reaction recompiles if you need to change their names or add new ones as their name scope is the same as their usage scope...

Len Holgate
+6  A: 

Your claim that declaring a non-integral constant as a static class member "have the detriment of requiring non-integral initialization in the cpp file" is not exactly solid, so to say. It does require a definition in cpp file, but it is not a "detriment", it is a matter of your intent. Namespace-level const object in C++ has internal linkage by default, meaning that in your original variant the declaration

const string MyStrConst = "String"; 

is equivalent to

static const string MyStrConst = "String"; 

i.e. it will define an independent MyStrConst object in every translation unit into which this header file is included. Are you aware of this? Was this your intent or not?

In any case, if you don't specifically need a separate object in every translation unit, the declaration of MyStrConst constant in your original example is not a good practice. Normally, you'd only put a non-defining declaration in the header file

extern const string MyStrConst; 

and provide a definition in the cpp file

const string MyStrConst = "String";

thus making sure that the entire program uses the same constant object. In other words, when it comes to non-integral constants, a normal practice is to define them in cpp file. So, regardless of how you declare it (in the class or out) you will normally always have to deal with the "detriment" of having to define it in cpp file. Of course, as I said above, with namespace constants you can get away with what you have in your first variant, but that would be just an example of "lazy coding".

Anyway, I don't think there is a reason to over-complicate the issue: if the constant has an obvious "attachment" to the class, it should be declared as a class member.

P.S. Access specifiers (public, protected, private) don't control visibility of the name. They only control its accessibility. The name remains visible in any case.

AndreyT
+1  A: 

Polluting the global namespace should be self-evidently bad. If I include a header file, I don't want to encounter or debug name collisions with constants declared in that header. These types of errors are really frustrating and sometimes hard to diagnose. For example, I once had to link against a project that had this defined in a header:

#define read _read

If your constants are namespace pollution, this is namespace nuclear waste. The manifestation of this was a a series of very odd compiler errors complaining about missing the _read function, but only when linking against that library. We eventually renamed the read functions to something else, which isn't difficult but should be unnecessary.

Your second solution is very reasonable as it puts the variable into scope. There's no reason that this has to be associated with a class, and if I need to share constants among classes I'll declare constants in their own namespace and header file. This isn't great for compile-time, but sometimes it's necessary.

I've also seen people put constants into their own class, which can be implemented as a singleton. This to me seems work without reward, the language provides you some facilities for declaring constants.

James Thompson
A: 

If only one class is going to use these constants, declare them as static const inside the class body. If a bunch of related classes are going to use the constants, declare them either inside a class/struct that only holds the constants and utility methods or inside a dedicated namespace. For example,

namespace MyAppAudioConstants
{
     //declare constants here
}

If they are constants used by the whole application (or substantial chunks of it), declare them inside a namespace in a header that is (either implicitly or explicitly) included everywhere.

namespace MyAppGlobalConstants
{
    //declare constants here
}
Chinmay Kanchi
A: 

don't pollute global namespace, pollute local.

namespace Space
  {
  const int Pint;
  class Class {};
  };

But practically...

class Class
  {
  static int Bar() {return 357;}
  };
ima