views:

353

answers:

7

The constructors of globally declared classes are invoked before main is entered. While this can be confusing to a new reader of the code because it is done so infrequently, is it necessarily a bad idea?

+18  A: 

It's not necessarily a bad idea, but usually, yes.

First, it's global data, and globals are usually a bad thing. The more global state you have, the harder it becomes to reason about your program.

Second, C++ doesn't guarantee intialization order of static objects defined in different translation units (.cpp files) -- so if they depend on one another, you might be in trouble.

jalf
If the code affects variables declared in other files, you might have bigger problems than initialization order. If it so happens that none of the functions or variables defined in a file are directly used elsewhere in the program, the linker can discard the whole compilation unit including initialization code. (This is a problem if the file is used via hooking or a registry which is installed by global construction.)
Ben Voigt
In addition, the threading of globals is unsafe to say the least and accessing them is slow (even accessing heap is faster). Globals are a bad idea in the vast, vast, vast majority of cases. There are, of course, exceptions.
DeadMG
The constructors are not order-dependent and do some benign initializations. The threading is carefully controlled.
Bruce
@Bruce: That's part of the problem solved then. But it is still global state. Because it is globally visible, it potentially affect *any* code in your application. Reuse gets harder, because the code you're trying to reuse might depend on a global somewhere. It gets harder to reason about the effects of your code, because that code might depend on the state of a global. Dependencies get harder to track because they're not clearly visible (as parameters being passed to functions). As I said, it is not **necessarily** bad in your specific case. But often, it is a sign of bad design.
jalf
+1  A: 

In addition to being of questionable form - It will not be portable to some platforms.

Ofir
+1  A: 

As you point out, it is allowed. In the last company I worked at, when such a situation would arise, we made it policy to add an appropriate comment to main() to indicate for which variables this applied to. If you have a bad situation, try to make the best of it.

Sparky
+7  A: 

Yes, this is bad. Since you will have no way to catch exceptions and deal with them, the default handler will be used. In C++ that means calling terminate...

Example: contents a.cpp

#include <stdexcept>

int f() { throw std::runtime_error("boom"); return 42; }
int i = f();

int main(int argc, char* argv[])
{
  return 0;
}

Output of : g++ a.cpp && ./a.out

terminate called after throwing an instance of 'std::runtime_error'
  what():  boom
Aborted (core dumped)

You can try add a try ... catch in your main, that won't help.

Edit: Jalf's points are valid too. Listen to his advice.

bltxd
what about main() try { ... } catch(...) { ... } ?
Alexandre C.
I don't think this example is legal because of the `int i = f();`. You can't call a function while declaring a global variable as far as I know.
Evan Teran
@Alexandre - we're talking about code called BEFORE main starts. So a try/catch in main can't get to it because the try hasn't been entered when this code is executed.
Michael Kohne
I'm talking about a "function try block", not a try/catch block inside of main. Do those catch exceptions thrown by static variable constructors ?
Alexandre C.
@Alex: A function try block is just a fancy way of wrapping the contents of the function within a try block. `main` *hasn't been entered yet*, you could do anything you want to `main`, it won't matter because *we aren't yet there.*
GMan
A: 

Using global/static objects with non-trivial constructors and destructors is awful. I saw enough huge software projects that found themselves in a catastrophe because of an uncontrolled use of global/static objects and singletones.

The problem is not the fact that it's the code that is run outside the main. It's that those objects are constructed and destroyed in an uncontrollable order.

In addition I believe it's a generally bad practice to use global variables anyway (even ordinary variables), with some exceptions. Every piece of code should be run within a well-defined context, and all the variables should belong to it.

valdo
std::cout is global.
ChrisW
@ChrisW: you dont want to destroy std::cout ...do you ??
smerlin
@smerlin - No, but it's an example of a global variable; which is constructed. Meyers gave an example (which he said is used in the std::cout implementation) of how to control the sequence in which globals are constructed; or rather, how to guarantee that a global is constructed before you use it, even if you're a global and if the thing you're using (e.g. std::cout) is in a different translation unit.
ChrisW
A: 

It's not bad form at all, and it's not confusing. Static initialization is a deliberate feature of the language. Use it when you need to. Same with globals. Use them when you need to. Like any feature, knowing when it is appropriate and knowing its limitations is part of being a strong programmer.

Bad form is restructuring an otherwise perfectly fine program just to avoid globals or static initialization.

John
That kind of assumes you're working with an already-written project; the original question is really prescriptive, not evaluatory.
Marc Bollinger
A: 

I will have to go so far as to say it is bad form for non-PODs, especially if you are working in a team primarily because of initialization order problems that can easily arise from this.

// the following invokes undefined behavior.
static bool success=cout<<"hello world\n";

People generally don't write code like above, but consider if success was initialized to the return value of another function. Now anyone tempted to use cout or cerr or any other global object in that function has invoked the same undefined behavior.

For user-defined types with constructors and destructors, consider the alternative method where access is initialization:

static Foo& safe_static()
{
    static Foo f;
    return f;
}

Unfortunately this also has problems with thread safety so a lock mechanism of some sort is required for the construction of 'f' if you are accessing safe_static concurrently.

That said, I do think one should try to keep things simple. Unfortunately when it comes to user-defined objects defined at file scope, it's far too easy to run into undefined behavior. The little extra effort required to write something like safe_static above can prevent a lot of headaches.

Exceptions are another point. If your static objects throw out of their constructor, then you have no way to catch the exception. If you want your application to be really robust and even handle startup errors, you'll have to structure your code carefully (ex: have try/catch blocks in the constructors for the objects you create at file scope so that the exception is not thrown outside of the ctor and also avoid initializer lists that throw).

If you're working in a team, you might think to yourself, "Oh, I'm not accessing any other globals in my class, I might as well make it a simple global with internal linkage at file scope." That might true then, but before you know it, your co-worker adds another global variable and tries to access it from the constructor of your class. Suddenly you have undefined behavior which might not even show up as a problem on the main platform you are targeting only for your code to crash and do other odd things when you try to port your code elsewhere.

It's really not worth the potential headaches IMO, and it's a problem that's much easier to avoid than to fix.