Sign in to follow this  
icecubeflower

global variables

Recommended Posts

Hey I have about a million global variables. My game program became huge and then I split it into about 20 separate .cpp and .h files so it's managable again. To keep using the global variables I had to say [souce] extern int inum; [/source] and so on for every global variable I need in all my .h files. Anyway it's working fine but everyone says if you use global variables then you'll die and go to hell so what exactly is the other option? I could get rid of all the global variables and declare them in main() and then pass them to all the functions that need them but some of those functions would need like 25 variables passed to them. I'm not doing that. As far as I can tell global variables are the best option unless there's something I don't know. (That's happened before.)

Share this post


Link to post
Share on other sites
Quote:

pass them to all the functions that need them but some of those functions would need like 25 variables passed to them. I'm not doing that. As far as I can tell global variables are the best option unless there's something I don't know. (That's happened before.)

This is commonly considered to be the solution, however it's only halfway there. Yes, you need to pass the data to every place that needs it -- but the larger problem, the problem you have got yourself into using globals (and part of why globals are thus considered bad) is that you have too many dependencies on too many objects, so the result is that in order to pass everything needed by the various subsystems to those subsystems... you have to pass everything to everywhere.

The solution is to reduce the number of dependencies. This is a major change and will involve extensive iterative refactoring of both your design and implementation, such that it may not be something you are willing to do at this point with this project (although it is a good idea, and good practice). If that's the case make sure you remember this for the next project, so that you don't dig yourself into the hole you're already in.

If you provide specific examples of your systems and how they are designed and how they talk to eachother, we may be able to suggest alternative designs that can help reduce dependencies.

If you choose to perform the refactoring, make sure you do it iteratively, a small change at a time. Demote one global at a time into main() (or whatever) and pass it down the chain until everything that needs it has it. Make sure everything compiles, runs, and passes your tests. Repeat until all globals have been demoted. At that point you can consider how you can revise the design of these subsystems to remove the dependencies on that data, and apply a similar refactoring process.

It is possible to do this cleanly and elegantly, although you may not be able to see that from here. All of the code I've written in the last few years, in both a personal and professional context, uses no globals and rarely passes data more than two or three 'levels of isolation' deep.

Share this post


Link to post
Share on other sites
If you need to pass a couple of variables, declare them in a struct and pass the struct.

Also, 25 arguments? Seems like you'd need to split those functions or atleast refactor your code.

I have no global variables at all, I would do everything to avoid them.

Share this post


Link to post
Share on other sites
Here's all the global variables:

Gizmo hero;
Gizmo *dummygiz;
GLuint base; // Font Display List (nehe's lesson32)
int irow, icol;
int irow2, icol2;
int fx,fy;
int fxstore, fystore;
int istate;
ifstream infile; //the currently loaded map
fstream savefile; //player's saved game, stays open all the time?
ofstream outfile; //opens for only a brief moment to create a save file
int ix, ij;
int itrash;
//float flayer6;
string rgcfilename; //i forgot if these are re-usable or if they are off limits
string rgcloader; //i think just used at beginning menu time so re-usable after that?
string rgcplayername;
string rgclayer1; //re-usable in most situations, it saves something during links so watch out
char ctrash;
Iceint icetrash;
Iceint iceblock; //value of block you're moving to
Iceint icex;
Iceint icey;
Iceint icetype;
Uint32 elticko;
GLUquadricObj* quadratic;
GLdouble dval;
int itrick;
//int itrick2; //used for animation
GizBox monsterbox;
PicBox monsterstore;
int ilinkspeed;
float rot;
ImageVault picbox;
SDL_Event event;
int done;
Uint8 *keys;
WeatherBox nature;
AudioBox boombox;
Icematrix icemap;
//Mix_Chunk *chunktest;
//delete ijunk
int ijunk;


Some of those I could get rid of pretty easily. For instance ix and ij are just iterators. Obviously I could have functions declare their own iterators. When I started this project I didn't know ANYTHING. For some reason I jumped right in and started programming a massive Zelda-like game editor and engine without doing anything small first.

So back then I was thinking I would just make those iterators global and then all the functions that use iterators wouldn't need the extra memory for their own iterators. Technically I was correct, I believe. But in the days of 2 gigabytes of RAM I'm starting to think it's a silly thing to worry about.

This is the .h file that contains my function for drawing the game:

extern void drawblocks();

extern int istate;
//extern GLfloat rgfmap[];
extern string rgcloader;
extern int irow2;
extern int icol2;
extern GLUquadricObj* quadratic;
extern GLdouble dval;
extern WeatherBox nature;
extern Iceint iceblock;
extern Uint32 elticko;
extern float rot;
extern GLuint imgmap[];
//delete ijunk
extern int ijunk;

void draw();


istate - I need that because it draws differently depending on what state the program is in. A few functions need istate. The function that deals with key presses, for instance.

rgcloader - I need that because the characters the user is inputting are stored in that and the draw function is displaying them

I need probably over half of those variables. I don't see how I can make my draw function not depend on them.

Also there are a couple other global variables that my draw() function uses but for some reason the compiler doesn't care that I didn't declare them with extern above. I have no idea why.

Share this post


Link to post
Share on other sites
Quote:
Original post by icecubeflower
Some of those I could get rid of pretty easily.


So, do the easy ones, and then we can talk.

Quote:
For instance ix and ij are just iterators.


No, they aren't. They are loop counters.

Quote:
Obviously I could have functions declare their own iterators.... So back then I was thinking I would just make those iterators global and then all the functions that use iterators wouldn't need the extra memory for their own iterators. Technically I was correct, I believe.


No. When control flow reaches the point where the variable is declared (this is a simplification; optimizing compilers can rearrange stuff), the memory is allocated for it on the stack. When that variable's scope is exited (e.g. at the end of the function, or within the loop it was declared within, if applicable), the memory is deallocated from the stack. The variable takes the same amount of memory whether it is in on the stack or (as the global) in static storage. So there is nothing to save here, except in cases where it would be incorrect to do so (e.g. for recursive algorithms, where the global would mean each recursive call re-uses the same variable data, and everything will get messed up).

Quote:
istate - I need that because it draws differently depending on what state the program is in. A few functions need istate. The function that deals with key presses, for instance.


So, you declare it in main, and pass it around to the functions that need it.

Quote:
rgcloader - I need that because the characters the user is inputting are stored in that and the draw function is displaying them


Similarly.

Quote:
Also there are a couple other global variables that my draw() function uses but for some reason the compiler doesn't care that I didn't declare them with extern above. I have no idea why.


Because they aren't used in other translation units.

Share this post


Link to post
Share on other sites
Oh, then yeah, they are loop counters. Sorry, my vocabulary is a work in progress.

The biggest problem I had was that I couldn't plan ahead, I didn't know anything. It was like I had to build a skyscraper without knowing anything about structural engineering. There were no blueprints, I just grabbed a hammer and some nails and went to town.

See that stream, infile? I open a file that contains information about the enemies and walls and what png files to use for the map. That thing just stays open all the time and all my functions use it. If I'm going to make it stop being global then I'll be passing it to many functions. Would it be wiser to close the file whenever I'm not using it and pass a string with the filename instead and just have the functions reopen it? Or is it fine to just open a file and leave it open?

Share this post


Link to post
Share on other sites
Maybe this gives you a clue on how to not use globals:

Global:
[source code="cpp"]int i;
int main()
{
for (i = 0; i < 10; ++i)
{
// loop
}
}


For local:
[source code="cpp"]int main()
{
for (int i = 0; i < 10; ++i)
{
// loop
}
}


Function local:
[source code="cpp"]int main()
{
int i;
for (i = 0; i < 10; ++i)
{
// loop 1
}

for (i = 0; i < 10; ++i)
{
// loop 2
}
}


Passing it to functions:
[source code="cpp"]void function(int number)
{
std::cout << number; // 10
}

int main()
{
int i;
for (i = 0; i < 10; ++i)
{
// loop 1
}

function(i);
}


I hope this helps you a bit...

Share this post


Link to post
Share on other sites
Quote:
Original post by icecubeflower
Oh, then yeah, they are loop counters. Sorry, my vocabulary is a work in progress.

The biggest problem I had was that I couldn't plan ahead, I didn't know anything. It was like I had to build a skyscraper without knowing anything about structural engineering. There were no blueprints, I just grabbed a hammer and some nails and went to town.

See that stream, infile? I open a file that contains information about the enemies and walls and what png files to use for the map. That thing just stays open all the time and all my functions use it. If I'm going to make it stop being global then I'll be passing it to many functions. Would it be wiser to close the file whenever I'm not using it and pass a string with the filename instead and just have the functions reopen it? Or is it fine to just open a file and leave it open?


Open it, copy it into a buffer, close it, pass the pointer of the buffer to functions or interpret the data into arrays or variables...

Share this post


Link to post
Share on other sites
Nah, I already knew that. I'm talking about variables like istate and rgcloader that functions need. I'm gonna backup my programs and try getting rid of my globals one by one. It just seems to me that some functions will need a lot of variables passed to them.

Share this post


Link to post
Share on other sites
Actually I load the bulk of the file info into a dynamic 2D array. The maps vary in size so when a new map loads I deallocate the array, read the dimensions of the new map, and then load info about every block. I only use the file after that point to grab extra stuff like the name of a side link or a cave link or something. Is there any real reason to load that stuff into memory too? What's wrong with just reading the link info from the file?

Actually I used to read EVERYTHING straight from the file, it was awesome. I had like 100 bad guys on the map at once and every one of them would seekg the file to see if they could walk left or up or down or wherever. It actually worked plenty fast enough for the type of game I'm making. But these days I load that stuff into memory.

Share this post


Link to post
Share on other sites
Quote:
Original post by icecubeflower
Actually I load the bulk of the file info into a dynamic 2D array. The maps vary in size so when a new map loads I deallocate the array, read the dimensions of the new map, and then load info about every block. I only use the file after that point to grab extra stuff like the name of a side link or a cave link or something. Is there any real reason to load that stuff into memory too? What's wrong with just reading the link info from the file?

Actually I used to read EVERYTHING straight from the file, it was awesome. I had like 100 bad guys on the map at once and every one of them would seekg the file to see if they could walk left or up or down or wherever. It actually worked plenty fast enough for the type of game I'm making. But these days I load that stuff into memory.


Well, reading/writing to/from a harddrive is SLOW, if the file isn't too large it's not too bad to load it into memory. Memory is relatively quick (CPU memory / registers are even faster, but thats not relevant now).

Share this post


Link to post
Share on other sites
I'm not so sure reading from a file is necessarily reading from the hard drive. I realize the way I used to rely completely on seekg was the dumb way to do it, I'm not arguing about that. But after I loaded all the info into memory and read it from a 2D array I didn't notice any speed difference.

Those files were only about 40K, I think fstream must keep a big chunk of the file in a buffer or something because it worked plenty fast. I'm not exactly programming Quake 5 here, I could probably get away with never loading anything into memory and it would work just fine. I'm just trying to get rid of my bad habits.

I loaded the map info into memory because that stuff is accessed in all the loops over and over and over. The link information is accessed only when you reach the end of a map or go into a house or something. It just reads the file to grab the name of the next file to open. Having that in memory instead of in a file wouldn't make any difference at all. Unless it loaded a new map a fraction of a nanosecond sooner.

Share this post


Link to post
Share on other sites
Something like might help you (taking no position in what I personally think you should program like (i.e not using globals, using globals etc...))

I know some of these have been posted but I'll add them anyway (see similar examples above in another post)

struct myStruct
{
myType aName,
anotherType aName,
.
.
.
};

returnType funcName(const struct * myStruct)
{
static myType localPersistentData;
myType localInpersistentData;

...Doing my Jazz...

return returnType;
}

Note that I have not cleared this as compilable or fully correct, there are things you need to do, however this should give you some idea on how to do it.

The notion here is that it is better to use structs as they can alter without breaking code for other functions should you add something later (unless you remove something of course). Also if you use local data you can do it in two ways, persistent or non-persistent. That way you can "store" data in your function but then you also need to be aware that the data *might* be different next time you enter the function. And by using "normal" non-persistent data you can be assured that the data will be the same each time you enter the function and also have the relevant values always within the function.

Note that you should name such values (that stay the same) to a name like *myAmmo* = 5 or something like that. Then you can stay away from "magic numbers" and not having to guess what they mean (the numbers that is).

But as noted in other posts, I would strongly suggest you pass structs and make them *const* (if at all possible) to help you stabilize the code.

Just keep in mind that it is most important that you code in a style and way that you are most comfortable with. Change is always good but do it in your own pace and time. Don't change because others tell you to. You need to understand why first, and then when you do only if you agree with them.

Share this post


Link to post
Share on other sites
Quote:
Original post by icecubeflower

Those files were only about 40K, I think fstream must keep a big chunk of the file in a buffer or something because it worked plenty fast.


The OS is primary cache. Even if you don't do anything in your application, most of disk access will be cached.

There's a performance counter Memory/Cache Bytes, which shows you current size of cache. It will be in tens of megabytes.

40kb is really small in this respect.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this