C++

Forum

 
When to use global variables...

iCurzU (7)
Hello everyone.

I recently got some code from a guy that uses an external API.
The code has a class with a reference counter which is used for initialization purposes.
Now I really do NOT advocate global variable usage but in this case I'm a little unsure if it's actually not better to do so.
I'm going to give you the pseudo code of the existing code and the pseudo code of how I think it should be solved instead.

Without global variable:
pseudo_class.h
1
2
3
4
5
6
7
8
9
10
11
12
13
 
class pseudo_class
{
public:
  pseudo_class();
  virtual ~pseudo_class();

  // some other declarations
  int example_var_;
private:
  static void initialize();
  static void finish();
  static int init_var_;
};



pseudo_class.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
 
// misc include directives here

int pseudo_class::init_var_ = 0;

pseudo_class::pseudo_class() : example_var_(0)
{
  initialize();
}

pseudo_class::~pseudo_class()
{
  finish();
}

void pseudo_class::initilize()
{
  if ( init_var_ == 0 )
  {
    if ( initialize_external_api() == ANY_ERROR )
      throw "Unable to initialize external API!";
  }
  init_var_++;
}

void pseudo_class::finish()
{
  init_var_--;
  if ( init_var_ == 0 )
  {
    finish_external_api();
  }
}


I don't think there is a general problem with this code except for :
* The external API that is used is a common library that I use in my current code.
* The constructor contains an exception which in my opinion is not great.

Anyway adding this class to my code ends up with conflicting initializations since as soon as I use the constructor of pseudo_class it runs initilize_external_api() which only should be run once for each process.

I believe that adding one global variable will solve my problem.

Pseudo code using one global variable.
my_globals.h
1
2
3
4
5
6
7
8
9
10
 
typedef enum
{
  NotInitialized,
  InitializationSucceeded,
  InitializationFailed,
} InitializationState;
external InitializationState global_api_init;

void global_init_code();
void global_finish_code();


my_globals.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
// misc include directives here

InitializationState global_api_init = NotInitialized;

void global_init_code()
{
  if ( global_api_init == NotInitialized )
  {
    if ( initialize_external_api() != ANY_ERROR )
      global_api_init = InitializationSucceeded;
    else
      global_api_init = InitializationFailed;
  }
}

void global_finish_code()
{
  if ( global_api_init == InitializationSucceeded )
  {
    finish_external_api();
    global_api_init = NotInitialized;
  }
}


pseudo_class.h
1
2
3
4
5
6
7
8
9
10
11
12
13
 
class pseudo_class
{
public:
  static pseudo_class *new_pseudo_class();

  virtual ~pseudo_class();

  // some other declarations
  int example_var_;
protected:
  pseudo_class();

};



pseudo_class.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
// misc include directives here
#include "my_globals.h"

pseudo_class *pseudo_class::new_pseudo_class()
{
  if ( global_api_init != InitializationSucceeded )
  {
    std::cerr << "pseudo_class requires external API to be successfully initialized!";
    return nullptr;
  }
  return new pseudo_class();
}

pseudo_class::pseudo_class() : example_var_(0)
{
}

pseudo_class::~pseudo_class()
{
}


main.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
// misc include directives here
#include "my_globals.h"

int main(int argc, char **argv)
{

  global_init_code();

  run_code_as_usual();

  global_finish_code();

  ...
}


Introducing a global variable to the code is not excellent.

Have you encountered and solved these kinds of initilization schemes more elegantly?

Best Regards,
/iCurzU
Last edited on
andywestken (4094)
Well, loads of globals are a bad thing, but the odd one can simplify things.

A habit I have picked up from MFC and ATL programming is the use of a class to represent the app or module. Usually this will be the only global, though sometimes I have a utility object or two (e.g. a logger object).

I would also use 2-phase construction in this situation. Esp. if the 3rd party code you're calling is a C-API.

So my main function is generally

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
// misc include directives here
#include "app.h"

App g_app;

int main(int argc, char **argv)
{
  int ret = g_add.init_code();

  if(success != ret)
  {
    cerr << "App init failed : error #" << ret << endl;
  }
  else
  {
    ret = app.run_code();

    app.finish_code();
  }

  return ret;
}

iCurzU (7)
Thanks for the answer Andy!

I'm not sure if you will read this but I'm going to ask a question anyway...

Regarding the two phase construction I prefer to stick to a singleton pattern here.
Meaning I removed all public means of constructing the object and instead require the usage of a static create method.
I even removed all means of copying the object since its contents contains various references to system resources that simply aren't trivial to just share with another object.

Now I'm essentially trying to evaluate two methods of storing globals.

The one you describe using a single application/module object.
The other is to use a namespace with POD (Plain Old Data).

my_globals.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
// misc include directives here
namespace my_globals
{
  typedef enum
  {
    NotInitialized,
    InitializationSucceeded,
    InitializationFailed,
  } InitializationState;
  external InitializationState api_init;

  void init();
  void finish();
} // namepace


my_globals.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
// misc include directives here
namespace my_globals
{
  InitializationState api_init = NotInitialized;

  void init()
  {
    if ( api_init == NotInitialized )
    {
      if ( initialize_external_api() != ANY_ERROR )
        api_init = InitializationSucceeded;
      else
        api_init = InitializationFailed;
    }
  }

  void finish()
  {
    if ( api_init == InitializationSucceeded )
    {
      finish_external_api();
      api_init = NotInitialized;
    }
  }
} // namespace


I guess that either of the two methods works but since this is one of the first times I seriously think about this issue with globals I want to try to do it right from start instead of banging my head into some unnecessary complication at a much later stage where the code will be harder to "just fix".

I read a little from Google's C++ Style Guide and they specifically mention that using objects have potential pitfalls:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Static_and_Global_Variables
Short version: Object order creation/deletion as an application starts up / shuts down are only partially specified in the C++ specs and can vary between different builds.

Anyway I have a question to you, or anyone else for that matter.
Do you know if there are possible advantages over the object method as compared to using a namespace with POD?
Last edited on
andywestken (4094)
If you are writing code that will be used by other people then using the singleton approach to prevent people from mistakenly constructing more than one instance. For internal code, I don't tend to go to the trouble. Neither MFC or ATL bother (which is prob. poor practice) so Windows programmers will normally be OK with the approach I suggested.

That aside, I don't think the global functions in a namespace is any worse. I use that kind of approach when I'm writing DLLs. I think both are equally easy to follow, which is the key point.

The choice will depend on how you want to report back to the use, should the initialization fail. As you can see, I prefer a return code in this case. This is not only sensible in a console app, where I thing the code needed for exception handling is a bit overkill, but also comes from working with DLLs a lot.

If you go with your class, I would move consider moving the initialize() call out of the constructor calling it directly.

The construction of objects within the same translation unit is guaranteed. So if you ensure you declare them all in the same cpp file, you can be sure of the order. But you can't know what someone will add to code later. So I totally avoid active initialization in global constructors.

Andy
iCurzU (7)
Hi and thanks for the input again.

I miswrote... I meant the factory pattern, not singleton. Just to clear things up.

I agree with return codes which is why I changed the code from exceptions to return values / polling variables. Exceptions should be used with the same frequency as the name suggests. That's what I think.

Think I got a clearer picture now so thanks again for your feedback!
andywestken (4094)
Singleton would be more suitable. As you only need one! Some singleton implementations new the instance when it's first requested.

I definitely agree with you about exceptions.

Andy
iCurzU (7)
Hi again,

I actually can create several objects but in order to create several of them you need to be very careful of how to set it up.
Otherwise I would prefer a singleton approach of course.

/davidl
andywestken (4094)
Several of the same type? Or several one off objects?
iCurzU (7)
I can create several separate objects but it isn't the same "cloned" object so it's not suitable with a prototype pattern if that is what you meant.
Last edited on
andywestken (4094)
Yep! Just clarifying things in (what's left of) my mind!
Topic archived. No new replies allowed.