khofez khofez - 3 years ago 50
C++ Question

Holding data on .cpp and function on .h

My question is more like a doubt.

Few days ago I started organizing my code in a way that I wasnt very used to, but I liked it a lot, the problem is I dont know if its bad or if its good, I mean its working but I wonder if the code gets bigger it will eventually be problem. This how I'm doing with my code:

.h file

void init();
void someFunction1();


.cpp file

struct myData
{
int someVar = 0;
int someVar1 = 21;
}
myData* mydata = nullptr;

void init()
{
mydata = new myData();
mydata->someVar = 1;
}

void someFunction1()
{
mydata->someVar = mydata->someVar1;
}


I know the "mydata" pointer will be just available on this particular cpp file, and the functions are the only thing I can call when I include de .h file, but thats basically what I want and its what I'm doing, is that any problem on that? I mean, will I get errors or anything weird in the future using it like this?

Answer Source

mydata is available in other translation units as well. This can create a problem if in another source you use another global variable using the same name. It will result in an identifier collision.

To make sure this doesn't happen, you can either make mydata a static variable:

static myData* mydata = nullptr;

Or put mydata in an unnamed namespace:

namespace {
    myData* mydata = nullptr;
}

In this case, myData does not have to be static.

The second option is usually preferred in C++ nowadays, but the first option will accomplish the same result and there's nothing inherently wrong with it.

Note that variables at global scope have some issues associated with them, related to initialization. For example, if you initialize a global variable to the value of another global variable defined somewhere else, you can't know which of the variables is going get initialized first, and thus your variable might be initialized with garbage data. However, in this case, you're just initializing it to a nullptr, so it's fine. It's a good habit however to still avoid global variables, since there's a better way to do it. You can wrap the variable into a static function instead, that returns a static variable defined within it:

static myData& mydata()
{
    static myData mydata_instance;
    return mydata_instance;
}

void init()
{
    mydata().someVar = 1;
}

void someFunction1()
{
    mydata().someVar = mydata().someVar1;
}

Also, note that no pointer is used. mydata_instance is an object, not a pointer, and this avoids having to use new to allocate memory, which means you don't have to worry about calling delete to later free the memory again. mydata() will create a myData object on the stack the first time it's called, and since that object is static, it will persist across every future call to mydata() and the same object will be returned every time (static local variables are only initialized once.) Note that return type of mydata() is a reference (denoted with &), and that means when you return the object, a reference to that object is returned instead of a copy. That gives you direct access to the mydata_instance object outside the function.

Now, even though the above will work, it is probably a good idea to get rid of the init() function and redesign your myData struct to not need it. In this case, just set the someVar member to 1 in the struct itself. There is no reason to initialize it to 0 inside the struct to then later have to call init() to set it to 1. So overall:

struct myData
{
    int someVar = 1;
    int someVar1 = 21;
}

static myData& mydata()
{
    static myData mydata_instance;
    return mydata_instance;
}

void someFunction1()
{
    mydata().someVar = mydata().someVar1;
}
Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download