-2

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?

Ryan B.
  • 1,270
  • 10
  • 24
khofez
  • 65
  • 1
  • 8
  • 1
    _"I know the "mydata" pointer will be just available on this particular cpp file,"_ No, that's wrong. Would be true if you put a `static` there. Also with c++ use a class with private data and public member functions instead. – user0042 Jul 25 '17 at 23:41
  • Yes it will be a problem. Don't create functions that return void and take no parameters, and don't create variables at file scope. –  Jul 25 '17 at 23:42
  • 1
    Please don't use the C tag on a C++ question, they aren't the same. – Ryan B. Jul 25 '17 at 23:44
  • What exactly are you doing there? For what do you use this kind of code? Why don't you use proper classes when your functions clearly sound like methods? I mean, I hope that this is an example that looks different in your real code, without generic names like init outside of any namespace and global variables that are not even mentioned outside a cpp. – Aziuth Jul 25 '17 at 23:58
  • Off topic: On the off chance you have't left them out to save on space in the question, strongly recommend reading up on and using Include Guards. – user4581301 Jul 26 '17 at 00:06
  • So "mydata" will be available on other cpp files if I include this .h file? I thought that I couldnt access it – khofez Jul 26 '17 at 00:18
  • 1
    @khofez It willl be available at an other `.cpp` file if you put a `extern myData* mydata;` declaration there, yes. – user0042 Jul 26 '17 at 00:25

2 Answers2

1

I know the "mydata" pointer will be just available on this particular cpp file

That's false. mydata will be a global variable. If you have another mydata global variable in another .cpp, they can collide (or at least they will violate the one definition rule).

Use unnamed namespaces for mydata:

namespace {
    myData* mydata = nullptr;
}

This way, mydata won't be accessible to other translation units.

But: global variables are not recommended. Don't use them. They almost always have alternatives.

There is a technical reason, which related to C++ only: static initialization order fiasco

And there are design decisions behind:

geza
  • 28,403
  • 6
  • 61
  • 135
1

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;
}
Nikos C.
  • 50,738
  • 9
  • 71
  • 96
  • Thank you very much for the explanation. Oh and about the init method, it was just an example, i dont usually do that, it was just to express what I was saying, but again, thank you – khofez Jul 26 '17 at 00:28