Repeated Initialization of struct in Header

Question

I'm developing a library of objects and functions and have a header file, here named super.hpp, that contains some initialization tasks.

super.hpp

#ifndef H_INIT
#define H_INIT

#include <iostream>
#include <string>

static bool isInit = false;

struct settings_struct{
    std::string path = "foo";
    void load(){ path = "bar"; }
};

struct initializer_struct{
    settings_struct settings;

    initializer_struct(){
        if(!isInit){
            std::cout << "Doing initialization\n";
            settings.load();
            isInit = true;
        }
        // settings.load();
    }//====================

    ~initializer_struct(){
        if(isInit){
            std::cout << "Doing closing ops\n";
            isInit = false;
        }
    }
};

static initializer_struct init; // static declaration: only create one!

#endif

My intention with this header is to create the initializer_struct object once; when it is constructed, this struct performs a few actions that set flags and settings for the entire library. One of these actions is the creation of settings struct that loads settings from an XML file; this action should also occur only once when the init struct is constructed, so the variables (here, path) are saved from the settings file. The super.hpp header is included in all objects in the library because different objects are used in different capacities, i.e., there's no way to predict which ones will be used in an application, so I include the super.hpp header in all of them to guarantee it is called no matter which objects are used.

My problem is this: when I include super.hpp in multiple classes/objects that are all loaded by the main application, the struct init appears to be re-initialized and the variables set when the settings_struct is constructed are overwritten with the default values. To see this in action, consider these additional files:

test.cpp

#include "classA.hpp"
#include "classB.hpp"
#include <iostream>

int main(int argc, char *argv[]){
    (void) argc;
    (void) argv;

    classA a;
    classB b;

    std::cout << "Settings path = " << init.settings.path << std::endl;
    std::cout << "Class A Number = " << a.getNumber() << std::endl;
    std::cout << "Class B Number = " << b.getInteger() << std::endl;
}

classA.hpp

#ifndef H_CLASSA
#define H_CLASSA

class classA{
private:
    double number;

public:
    classA() : number(7) {}
    double getNumber();
};

#endif

classA.cpp

#include "super.hpp"
#include "classA.hpp"

double classA::getNumber(){ return number; }

classB.hpp

#ifndef H_CLASSB
#define H_CLASSB

class classB{
private:
    int number;

public:
    classB() : number(3) {}
    int getInteger();
};

#endif

classB.cpp

#include "super.hpp"
#include "classB.hpp"

int classB::getInteger(){ return number; }

To compile and run the example,

g++ -std=c++11 -W -Wall -Wextra -Weffc++ -pedantic -c classA.cpp -o classA.o
g++ -std=c++11 -W -Wall -Wextra -Weffc++ -pedantic -c classB.cpp -o classB.o
g++ -std=c++11 -W -Wall -Wextra -Weffc++ -pedantic classA.o classB.o test.cpp -o test.out
./test.out

I expect the output from test.out to be the following:

Doing initialization
Settings path = bar
Number = 7
Doing closing ops

However, when I run this, I instead get "Settings path = foo". Thus, my conclusion is that the initializer_struct, init, is being constructed more than once. The first time, the boolean isInit is false, and the settings structure load function sets path to "bar." For all subsequent initializations, isInit is true, so the load function is not called again and it seems that the variable values from the uninitialized settings (i.e., path = "foo") overwrite the previously loaded values, hence the output of init.settings.path in test.cpp.

Why is this? Why is the init object constructed every time the header is included? I would have thought the include guards would keep the header code from being called multiple times. If I make the init variable in test.hpp a non-static variable, then multiple copies are created and the output prints multiple iterations of "Doing initialization" and "Doing closing ops." Additionally, if I uncomment the settings.load() function call outside the conditional statement in the initializer_struct() constructor, then the output gives a settings path of "bar". Finally, removing the inclusion of super.hpp from classA.cpp results in a path value of "bar", further supporting my hypothesis that multiple inclusions of test.hpp result in multiple constructor calls.

I would like to avoid having settings.load()' called for every object that includessuper.hpp` - that's why I placed the command within the conditional statement. Any thoughts? How to I make sure the settings file is read only once and that the values loaded are not overwritten? Is this a completely obtuse method to set some flags and settings that my library uses? If so, do you have any suggestions to make the processes simpler and/or more elegant?

Thanks!

Edit: Updated to include two object classes to closer represent my more complex setup

Answer 1

In your header file you define these static global objects:

static bool isInit = false;

static initializer_struct init;

These static global objects get instantiated in every translation unit that includes this header file. You will have a copy of these two objects in every translation unit.

initializer_struct(){

This constructor, though, will only be defined in your application once. The compiler will actually compile the constructor in every translation unit that includes these header files, and in every translation unit the constructor will use the static global object from its translation unit.

However, when you link your application, the duplicate constructors in all translation units will be eliminated, and only one instance of the constructor will be a part of your final executable. It is unspecified which duplicate instances will be eliminated. The linker will pick one of them, and that's the lucky winner. Whatever instance of the constructor remains, it will only use the static global objects from its own translation unit.

There is a way to do this correctly: declare the static global objects as static class members instead, and then instantiate those static global objects in one of the translation units. The translation unit with your main() is an excellent choice. Then, there will be a single copy of everything.

Answer 2

Following the suggestions of Varshavchik, I have made a few changes. First, I have replaced the super.hpp header with a very basic class that all objects in my library can extend:

base.hpp

#ifndef H_BASE
#define H_BASE

#include <iostream>
#include <string>

struct settings_struct{
    settings_struct(){
        std::cout << "Constructing settings_struct\n";
    }
    std::string path = "foo";
    void load(){ path = "bar"; }
};

struct initializer_struct{
    settings_struct settings;

    initializer_struct(){
        std::cout << "Constructing initializer_struct\n";
    }

    ~initializer_struct(){
        std::cout << "Doing closing ops\n";
    }

    void initialize(){
        std::cout << "Doing initialization\n";
        settings.load();
    }
};

class base{
public:
    static initializer_struct init;
    static bool isInit;

    base();
};

#endif

base.cpp

#include "base.hpp"

initializer_struct base::init;
bool base::isInit = false;

base::base(){
    if(!isInit){
        init.initialize();
        isInit = true;
    }
}

The other files remain more or less the same, with a few changes. First, both classA and classB extend the base class:

class classA : public base {...}
class classB : public base {...}

Now, when any of the objects are constructed, the base class constructor is called, which initializes the settings and other variables once. Both isInit and init are static members of the base class, so all objects that include the base header or extend the base object have access to their values, which fits my needs. These values are accessed via

base::init.settings.path

and the output is now what I expect and want it to be, with Settings path = bar instead of "foo"

Answer 3

You almost have it, just move the static isInit to be a static member of your class and move the instantiation of init to a translation unit. This would be the resulting files:

super.hpp

#ifndef H_INIT
#define H_INIT

#include <iostream>
#include <string>

struct initializer_struct{
    static bool isInit;

    struct settings_struct{
        std::string path = "foo";
        void load(){ path = "bar"; }
    } settings;

    initializer_struct(){
        if(!isInit){
            std::cout << "Doing initialization\n";
            settings.load();
            isInit = true;
        }
        // settings.load();
    }//====================

    ~initializer_struct(){
        if(isInit){
            std::cout << "Doing closing ops\n";
            isInit = false;
        }
    }
};

extern initializer_struct init; // extern declaration, instantiate it in super.cpp

#endif

super.cpp

#include "super.hpp"

bool initializer_struct::isInit = false;
initializer_struct init;

However you would be better off using a singleton pattern. With the singleton pattern you make sure that only one instance of your class is instantiated. You can get a handful of information here: C++ Singleton design pattern

It would look like this:

singleton.hpp

#pragma once

class initializer_struct{
public:
    struct settings_struct{
        std::string path = "foo";
        void load(){ path = "bar"; }
    } settings;

    static initializer_struct *GetInstance() {
        if (_instance == NULL) {
            _instance = new initializer_struct();
        }
        return _instance;
    }
    ~initializer_struct(){
    }    
private:
    initializer_struct(){
        if(!isInit){
            std::cout << "Doing initialization\n";
            settings.load();
            isInit = true;
        }
    }

    static initializer_struct *_instance;
}

singleton.cpp

#include "singleton.hpp"

initializer_struct *initializer_struct::_instance = NULL;

You could even go for an on load initialization by changing _instance from pointer to just an object, declaring it as an object in singleton.cpp and changing GetInstance() prototype to:

initializer_struct &GetInstance() { return _instance; }

However with the latter beware of the static initialization order fiasco (http://yosefk.com/c++fqa/ctors.html#fqa-10.12). In short you can do the latter approach if your class initialization does NOT depend on another class initialization, as you don't know which one will be initialized first.