Followers 0

# Why are static variables bad?

## 57 posts in this topic

I've read a few threads here that people advise against static variables, and the argument is usually because anything global is bad, but besides this, why are static variables considered bad?

Do they perform badly or something like that?

I'm asking because I've got my engine structure with modules, and I'm using static variables for each module so my base classes can access it.

class GameEngine
{
public:
GameEngine()
{
resourceModule = &_resourceModule;
};

static ResourceModule* resourceModule;
private:
ResourceModule _resourceModule;
};


#include "GameEngine.h"

{
//Texture2D* _texture;
}


Images are meant to be used like this:

void SomeGameFunction()
{
Image* newImage = new Image();
}


I'm currently doing some encapsulation and blocking accesses to base classes that I don't want exposed, so other programmers don't ever access a "Texture2D" class for example, and when I looked to these static variables I thought they might be an issue but I couldn't find of any good solutions for it, that is, if they require a solution...

So I remembered the topics against static variables I thought I should ask, why are static variables considered bad?

1

##### Share on other sites

It's not that static variables are bad necessarily, but they are often misused and that can lead to code that is rather unpleasant to work with.

-Josh

1

##### Share on other sites

Your original example has conflated the use of static (or class) variables and public access to data.  The two topics are unrelated and orthogonal.

There's nothing at all wrong with using a class variable, where appropriate.

There are serious problems with using a publicly accessible variable of static storage duration, whether at namespace level or class level (these are known as 'global variables' in the C language and as a consequence colloquially also in the C++ language).

The biggest problem with the original example code is the unnecessary coupling between the GameEngine class and the Image class.  With that design, not only is the coupling brittle (ie. if i change the GameEngine, I have to change the Image) but the design prevents me from reusing the Image class in, say, a game editor or image editor that does not need a GameEngine.  Writing unit tests for your Image is next to impossible.  None of these problem have anything to do with the storage duration of your data but are a direct result of the accessibility of your data.

2

##### Share on other sites

I'm asking because I've got my engine structure with modules, and I'm using static variables for each module so my base classes can access it.

In addition to what the others have said, this is an inevitable bad design you are applying here. Having your modules globally accessible might seem tempting at first, but it will inevitably lead to a pile of cluttered spaghetti code. You might think "well, if I stop doing so I might end up having to pass my modules to everything and every constructor will have like 5 arguments just because of this", but this is not as it seems. You can end up in such a situation, but the main benefit from making your modules non-global are the you'll have to think about a clever design that will avoid you having to pass like the graphics-module 10 layers down into 100 different classes. This will force you to try to decouple your classes as much as possible, enforce proper encapsulation, etc, etc. In the end, you'll end up with much more usable classes, without hidden dependencies, and, I predict you, you'll actually save more work than you'd have if you made all those global.

And, since the others have already given a proper shot at the original question, and since I'm on it, here are some more hints regarding your overall design (only read on if you want some feedback over your original thought process):

While I do not see the rest of the class, it appears the resourceModule has too much responsibility. I suppose it has methods for loading and creating textures, meshes, etc... and will do so on? That violates the single responsibility principle, and while this and other rules aren't necessary important for you to follow right now, its never too early to start using them. In short, you are better of splitting the different resource caches, and also split the loading functionality into its own class.

Image* newImage = new Image();

Also, I'd rather split the loading-method from the image class. Such a design like here makes it hard to extend on things, if you have a seperate loader class its easier to add new loading methods, without breaking apart the other code.

Just some additional thoughts, if you ever feel the need to improve something or feel like your current design isn't working anymore as you'd expected...

2

##### Share on other sites

and every constructor will have like 5 arguments

And if you do need to pass in five dependencies to one constructor (which is extremely unlikely if you design your classes properly), you can always wrap them in an instance of a FooContext class.

0

##### Share on other sites

and every constructor will have like 5 arguments

And if you do need to pass in five dependencies to one constructor (which is extremely unlikely if you design your classes properly), you can always wrap them in an instance of a FooContext class.

There's nothing wrong with a constructor having five arguments per se, if that class has five dependencies and/or required parameters. If your class genuinely has five dependencies, passing them to the constructor is better than the alternative of global variables. If your class feels like it has too many dependencies, maybe it really does and needs refactoring. That's going to be a judgement call based on the specific situation.

1

##### Share on other sites

No I agree, it depends on the situation. But having too many arguments - especially of the dependency injection variety - is at the very least a potential code smell that needs to be investigated. Often there is a better solution.

0

##### Share on other sites
Personally I like to make objects that can always be default constructed and design paramaters as simply ways to set specific properties on creation. I'm not really a fan of objects that do complicated behavior just by being created.

If anything, if an object has a lot of dependencies that you can't get rid of then that means they're going to need to be set somewhere anyway, otherwise you're just reaching out to global code and doing the same thing. Dependencies never disappear if you really need them, you just throw a blanket over their existance by using globals.

An object doesn't have to be able to do everything possible at creation it should just be in a valid state that will not crash the program.
0

##### Share on other sites

While I do not see the rest of the class, it appears the resourceModule has too much responsibility. I suppose it has methods for loading and creating textures, meshes, etc... and will do so on? That violates the single responsibility principle, and while this and other rules aren't necessary important for you to follow right now, its never too early to start using them. In short, you are better of splitting the different resource caches, and also split the loading functionality into its own class.

The resource module only allocates the resource, I actually call a GetResource(name) and it'll check if it's already loaded, if it is, it'll return a pointer to it, else, it'll create a new object of that type, add to the list of loaded objects, and call the proper object loader.

I used to make the resource module load the objects, but then read about this single responsibility principle, and changed it so objects load themselves. But I still have to make them go through the module to manage and avoid duplicates.

And with this design, to get rid of the static modules, I'd need to send a bunch of pointers a few layers down just so a new object can be created.

Simplified, it would be:

GameEngine -> ResourceModule(GraphicModule*) -> New Object(GraphicModule*)

So my object can load it's data to the graphic card. And this would be deep hidden in the resource module which once I finish the managing memory part, I don't really think I'll be coding anything else in there.

By keeping it in the objects, I can always just change the object's load function or just add new objects that use the basic primitives from the resource.

For example, I just thought I don't have any "sound" object yet.

If I want to make an object that will trigger a sound effect for example, I'd need to also send the SoundModule pointer through Engine->Resource->Load, instead of just doing:

void NewSoundObject::Load()
{
//Sound* primitive
_sound = ResourceModule::GetSound("soundName");
}


I'm also trying to make the objects as detached as possible, and this was the only way I could think of.

Objects have pointers to primitives, and the Resource Module manages these primitives. Primitives I shouldn't add any more once I'm done.

I call "primitives" these base resource classes that only have data in it about that resource, and that can be used by multiple objects and shouldn't ever have more than one (sounds, meshes, textures, etc).

Changing it to pointers and sending to the constructor seems it would just move the dependency from Load() to Constructor().

Ideally, I'd like it to be:

class Object
{
public:
{
_texture = new Texture2D();
}
private:
Texture2D* _texture;
};


And the Texture class manages everything about itself, but I couldn't think of how without using static variables in the texture as well. I tried to keep primitives as clean and independent as possible, so they don't know anything about the GameEngine or modules.

It's higher up objects that uses them do, though.

I implemented your suggestion, it was getting pretty messy after all, so now it's:

GameEngine::resourceModule->Textures.GetTexture(textureName);

GameEngine::resourceModule->Meshes.GetMesh(meshName);

I'm always looking to improve, and I'm still not 100% happy with the design now, I think it's still not really good, but that's the best I could come up with for now.

I was looking to change my static modules for pointers if static variables had some performance issue or some other deep problem, so that's the main reason I asked why they're considered bad besides the "it's bad practice having globals".

Edited by Danicco
0

##### Share on other sites

I've read a few threads here that people advise against static variables, and the argument is usually because anything global is bad, but besides this, why are static variables considered bad?

Do they perform badly or something like that?

I'm asking because I've got my engine structure with modules, and I'm using static variables for each module so my base classes can access it.

class GameEngine
{
public:
GameEngine()
{
resourceModule = &_resourceModule;
};

static ResourceModule* resourceModule;
private:
ResourceModule _resourceModule;
};

#include "GameEngine.h"

{
//Texture2D* _texture;
}


Images are meant to be used like this:

void SomeGameFunction()
{
Image* newImage = new Image();
}


I'm currently doing some encapsulation and blocking accesses to base classes that I don't want exposed, so other programmers don't ever access a "Texture2D" class for example, and when I looked to these static variables I thought they might be an issue but I couldn't find of any good solutions for it, that is, if they require a solution...

So I remembered the topics against static variables I thought I should ask, why are static variables considered bad?

I got no trouble with the global static arrays - they probably are most performant as cache friendly (maybe it is also depending on the layout of data inside) - but I am writing in c

small global variables can bring a trouble

-1

##### Share on other sites

I used to make the resource module load the objects, but then read about this single responsibility principle, and changed it so objects load themselves. But I still have to make them go through the module to manage and avoid duplicates.

Hah, it appear you haven't fully understood the principle about the single responsibility principle ( haha ) yet. SRP in one wording means that a class should have only one well defined task. So what does your image class now does? It at least acts as a image-data container and knows how to load an image... oups. See that bold word? Thats a very bad sign to begin with, when talking about one responsiblity.

Seeing how you tried to avoid the responsiblity from the resourceModule is a good thing, but you shouldn't have moved it to the image class. I'd personally made a seperate ImageLoader-class. This class knows how to load and create an i mage, and store it in the resourceModule.

And with this design, to get rid of the static modules, I'd need to send a bunch of pointers a few layers down just so a new object can be created.

Aye, and let met tell you, thats a very good thing! This will most importantly keep you from creating objects all over the place, and force you to seperate the graphics resource creation into their own classes, instead of messing with them wherever you want. What you show, is what you want to be able to do in a script language, and not in low/mid-level game code. In my own engine, I mostly have to pass in my different resource loaders 3 layers: It starts in the game state, there it goes into the game system, and there I'll have some loader class (terrain loader, particle loader), that takes it. Let me show you a perfect way of "hiding" those kinds of dependencies away, but in a good way:

namespace acl
{
namespace dx11
{

{
public:

MaterialLoader(const gfx::Effects& effects, const gfx::Textures& textures, gfx::Materials& materials);

gfx::IMaterial* Load(const std::wstring& stName, const std::wstring& stEffectName, unsigned int effectPermutation, const std::wstring* pTextures, unsigned int numTextures, const SubsetVector* pSubsets = nullptr) const override;

private:

gfx::Materials* m_pMaterials;
const gfx::Effects* m_pEffects;
const gfx::Textures* m_pTextures;
};

}
}


Thats my material-loader. A material needs both an effect (shader) and textures, as well as some other variables. Granted this would be already a good example for wrapping parameters in a structure, but look at the constructor. I pass in the effect and texture cache (const), as well as the material cache. Thats a perfect visible dependency: Whoever looks at this class sees it needs the effect and texture cache for read, and possibly writes to the material cache. Also, this dependency is only there in the constructor. The load-method is completly independant of this. Now I construct my material loader, and after that, there is no difference in handling it that where if the material had its own loading-method.

I implemented your suggestion, it was getting pretty messy after all, so now it's:

GameEngine::resourceModule->Textures.GetTexture(textureName);

GameEngine::resourceModule->Meshes.GetMesh(meshName);

Thats already a leap ahead IMHO, one thing you'll want to improve in the future simply to save you time and doublicated code is maybe make a generalized templated ResourceCache-class, and derive your Textures/Meshes from that. Other than that, I can just tell you from my own experience, the more sophisticated your "high" level structures get, the better those resource handling etc. will become. For example, if you were to implement a clever gamestate-system, maybe with some entity/component-system, you wouldn't need to store the resourceModule in the GameEngine-class as static anymore. You would construct it in the method that creates the very first game state, and pass the resourceModule in to that. From there on, you'd forward it to your systems if needed. Oh, I think thats a perfect example of why globals are totally unnecessary really. Since you mentioned sounds/audio sources, I'll show you how my ECS handles it. First of all, there is the audio-source component.

		struct AudioSource :
public Component<AudioSource>
{
AudioSource(const std::wstring& stFile, bool bLoop);
~AudioSource(void);

std::wstring stFile;
bool bLoop;
audio::Emitter* pEmitter;
};


See how its constructed? From a string and a boolean. I can construct this whever I want, without having to pass in anything at all. If a boss event down the complex instance handler -> ai handler -> event dispatcher -> and so on wants to emit a sound, this is what it creates. Now my audio-system on the other hand does this:

		void AudioSystem::Update(double dt)
{
auto vEntities = m_pEntities->EntitiesWithComponents<AudioSource>();

for(auto pEntity: vEntities)
{
AudioSource* pSource = pEntity->GetComponent<AudioSource>();

if(!pSource->pEmitter)
{
pSource->pEmitter = new audio::Emitter(*m_pDevice, *m_files[pSource->stFile], pSource->bLoop);
pSource->pEmitter->Play();
}

}
}


It creates the actual sound emitter, using a file that has been loaded to the audio file cache (m_files). The audio-system only gets updated once per frame in the game state (one layer down). Its not so much about that design, but about how you can decouple things like loading, and even initialization altoghether. I hope you get the idea and can take something from this, I unfortunately lack an example that is directly tailored to your problem here...

Changing it to pointers and sending to the constructor seems it would just move the dependency from Load() to Constructor().

Thats not exactly true. You've now removed the loading code from the image-class itself. Now any external structure can construct an image the way it wants, and not depend on the image class itself. If we are talking about the same thing, namely:

class Object
{
public:
Object(Texture2D* texture): _texture(texture)
{
}
private:
Texture2D* _texture;
};


I'm not talking about simply passing in the resourceModule to the constructor, that would be bogus. You should let the Object (shouldn't it be image) be initialized from an external source, via the constructor. Then you have an ObjectLoader-class, that loads the fitting texture from the cache, and assigns it. See? Though I can't make a puff of smoke and a "Puff!" sound effect, the depencency has magically disappeared ;)

I never do that. I am hardly pressed trying to identify an object that is "useful" in its default state.

What's the point of having a vertex buffer that points to nothing? You'll have to handle that useless situation everywhere both inside and outside the class. Just pass in the stuff needed to create a "useful" VB in the constructor and you're done in one line. In my experience this kind of code:

Seconded. A constructor (not default) is there for a reason, and using it in a sane way is perfect for not hiding, but easing the issues of dependencies.

foo->init(); // THIS ONE, I HATE IT WITH A PASSION.. JUST INIT INIT IN THE CONSTRUCTOR!

Ah, I used to do this all the time in my past projects. Its so tedious. Half of the time, you'd forgot to init and deinit() afterwards. And whats the point? Pass in the constructor. Its a little more complication for no real gain.

I got no trouble with the global static arrays - they probably are most performant as cache friendly (maybe it is also depending on the layout of data inside) - but I am writing in c

Let me lean myself out of the window by saying that if anything, statics are anything else than cache friendly. For statics, we have no control over how and where they are created. I know little about memory optimization, but that sound wrong to me. Linear memory used concurrently, thats cache-friendly from what I know. Statics? Not so much...

Edited by Juliean
1

##### Share on other sites

The fact that it is global is unrelated to cache, and on some systems (such as Nintendo DS) it means slower access time.

There is nothing the heap offers that the stack does not except for raw size.  If the amount of data you need is sufficiently small enough that stack overflow is of no threat, it is never better from a performance standpoint to have data on the heap rather than on the stack.  The stack is never slower than and sometimes faster than the heap.

In either case, anything that is accessed often, as is suggested by the nature of it being global (a product of the misuse often applied to globals mentioned already) will almost always be readily available in the cache anyway.

And the sizes of globals is not a cause for troubles.

The problem with globals is related more to coding philosophy than to performance etc.

A global lacks restrictions on how and by whom it can be accessed which tempts programmers to lazily access them from anywhere rather than to pass only the minimum number of needed pointers around.

The easiest way to exemplify this as a problem is to consider the 2 cases when you look at existing code that someone else wrote but that you need to maintain.

If no globals are used, you can look at the inputs to a given function or method and just from that you can be sure about what the function or method accesses or on what it relies as far as inter-modular dependencies go, and you can easily mentally compartmentalize the scope of it.

If globals are used throughout the project, you will never be able to reduce functions/methods in scope mentally without checking the whole body for references to statics/globals.  When you look at the next function/method you don’t have a single confined place to look to check the scope of its access and so each time you have to start over by studying the inputs and the whole body.

This isn’t necessarily the specific problem with globals—the problem with globals is not just that you might have a hard time compartmentalizing the scope of access of code someone else wrote before—it simply exemplifies the main issue, which is that there is little or no control over dependencies on it, and as such it easily reaches a point where just because one thing depends on that global, it must also depend on a whole library of unrelated code just to get that dependency.

L. Spiro

globall acces static arrays are related to old school way of programming if using them in c (and it is more cache friendly imo than more oop alternatives - but i do not want to speak about it here (got an awfull days recently) - such code i would say is array-centric

as to encoding dependency by passing - It is possible that it can be useful when improving some bigger code written by the others - I am not sure to that but maybe, I am writing arraycentric c code mostly so I got no clear practical view on the opposite - but I can say that my arraycentric approach never bring any troubles to myself - and is very nice to myself (really)

other thing is small set of global state small variables (something like you do not want to pass it so you make it a global state  variable- it can be very bad (probably because when you pass it you bring a reference to some track of changing this value with you, so you can deduce and be conscious about its previous state - when you set it global, and use it far away in the different place you lost the track to code who was responsible about its state (just forgot it - so if your code depends of such state,  you have no back trace what just bring its value - you have no simple dependency path here so if such code works it works just by chance that this backtraced value is ok  )

- I am not sure to that exactly clearly (*) (and how to clearly say it) but this 'lost track' is something in a way related  to such (previous programming age, one can say) troubles - but as I said I do not encountered such problem with large arrays probably because mechanics of my code treats them in very clear and simple architectural way so this was not the case)

(I could probably name this type of trouble bringing global variables in special name like "orphan variables" "lost variables" or something like that to ease the process of reflexing about them)

(*) the least clear thing here is if passing values can repair such lost variable occurances, or how to avoid them in other way I do not see it clearly at the moment

Edited by fir
-1

##### Share on other sites

I never do that. I am hardly pressed trying to identify an object that is "useful" in its default state.
What's the point of having a vertex buffer that points to nothing? You'll have to handle that useless situation everywhere both inside and outside the class. Just pass in the stuff needed to create a "useful" VB in the constructor and you're done in one line. In my experience this kind of code:

Foo* foo=new Foo();
foo->init(); // THIS ONE, I HATE IT WITH A PASSION.. JUST INIT INIT IN THE CONSTRUCTOR!
foo->setThis(bla);
foo->setThat(whatever);

Is just a ad for bugs.. as opposed to this:

Foo* foo=new Foo(bla, whatever);

The problem with that is you're assuming it is an object that is useless without its full set of data and useful with it. Which in reality is not the case most of the time, especially concerning games. A good example might be a game entity, say you wanted to create it and set certain data about it and then add it to a world. Should it ask for a world and add itself in the constructor? Well it could but that means you have to add it to the world the second you create it, it also means you are essentially required to pass all of the information the object needs into the constructor and to also assume it is all correct.

I prefer to treat the constructor as a mechanism to create the object and that the constructor should not -use- the object at all, the constructor should just provide a simpler way to set certain information about the object. The funny thing is that I said that I prefer the rule to be that you CAN default construct an object but don't have to. In your example of a vertex buffer I wouldn't see any issue with having that constructor you love AND having the default one.

This fits different scenarios while also keeping the design of all objects similar so that you can assume an object can always be default constructed and not break anything, or you can manually set bits after construction without assuming it is too late and the object has already began to work magic behind the scenes.

One line, the compiler works on your side by making sure you provided the required information to create a useful object that'll actually be able to do stuff.

There are objects that don't need non-paramater dependencies to do things, a lot of them can do certain tasks just by paramaters and member variables and they may not need a certain dependency for every piece of behavior they have.

Oh and pls.. I know about the fact that new cant report errors but, in my experience, the number of classes that can fail a new (out of memory) without bringing down the entire game is a very limited amount.. if that really becomes a problem, work those as an exception to the rule.

In my experience doing any real work in the constructor is just asking for a mess both because of how difficult it is to adequately debug problems and the fact the constructor deals with a lot of minute rules about how it can modify an object since it is still in the state of being constructed.

And I dont define a constructor for it, I am asking for a crash:

Personally I think this is a HUGE problem in that you code should not be designed in such a way that if you fail to pass something to it, that it crashes. It should if anything report it, or if it causes a fatal error it should crash with a thrown exception in a way that the programmer knows they messed up and it needs to be fixed immediately, rather than a user error.

The whole methodology behind the default constructor object is to treat the object as the most important thing in the world, that it should be concerned only with its own state and not trust any information received from other parts of the program, by verifying that data is both correct and exists it prevents the program from crashing without having to worry about things outside of its own code like "did the coder set this object right before creating me." Edited by Satharis
0

##### Share on other sites
In my humble opinion and my experience:

- statics are very usefull within specific functions, not global. For example in game specific functions, to keep track of locallly (in the function) needed vars like counters etc.
- when you need vars accessible by different classes, I'd draw out a design of the classes and determine who should be able to communicate with who

Summarized, my opinion is that, the bigger/ better structured the project, the less statics I use (except in the conscious/ aimed cases described above).
1

##### Share on other sites

We have one area of code that (non-const) static variables are allowed.  Not just allowed, but encouraged.

In doing so we needed to answer many of the issues listed above. We control who is modifying it and when, we limit what can be stored in it, have requirements that it never be null, it does not impede automated testing, does not impact thread safety, and actually lends itself to code reuse.

That one area is tunable constant values.

The values are assigned a default at creation and shared across all instances of the objects. Our engine allows the values to be adjusted both at game load time and while running, but in order to modify mid-game the world is placed in a special 'stop the world' state to ensure thread safety.

The values are treated as constants everywhere in the game, with the one small exception of the tuning updater that can modify the values.

Otherwise it is extremely rare that multiple objects require a shared state.

0

##### Share on other sites

Personally I like to make objects that can always be default constructed and design paramaters as simply ways to set specific properties on creation. I'm not really a fan of objects that do complicated behavior just by being created.

Just because you are passing dependencies to the constructor does not mean that the constructor is doing anything more than storing a reference to those dependencies. It doesn't have to use them or access them in any way.

The problem with that is you're assuming it is an object that is useless without its full set of data and useful with it. Which in reality is not the case most of the time, especially concerning games. A good example might be a game entity, say you wanted to create it and set certain data about it and then add it to a world. Should it ask for a world and add itself in the constructor? Well it could but that means you have to add it to the world the second you create it, it also means you are essentially required to pass all of the information the object needs into the constructor and to also assume it is all correct.

The constructor is not the only place to inject dependencies, it's just a convenient and fail-safe one. Obviously if you need a parameterless constructor, or otherwise need to construct an object before you have a reference to its dependencies, or that dependency is not available within the scope that it is being constructed, then you'll have to use another means. Personally, in the case of game objects, I typically require the "world" reference as an argument to any methods that depend on it, likewise for any other dependencies. The parameterless constructor is needed for deserialization and/or constructing entities to add components to via configuration files or using some sort of fluent interface.

Edited by smr
0

## Create an account

Register a new account