Why are static variables bad?

Started by
56 comments, last by Sacaldur 10 years, 7 months ago

As a concrete example of why they are bad:

I am using Cocos2dX for a project. And we had to send network call. Great, cocosx provides a client HTTP built on top of Curl.

It's a singleton class (Which means a global var basically) with most of its variable static or globally declared in the CPP. It didn't cause problem at first. Everything is queued when I make multiple call. Ok, perfect.

But then, we started to want to send some calls simultaneously. LIke uploading a file in the background, but continuing navigating in the app.

But we can't, because there is only 1 instance of the HTTP client! And we can't instantiate another one, because everything is static in it. And they even had global vars in the CPP!! A design that sounded good at first ended up a nightmare. I was able to remove the static vars and globals. I can now create multiple instance of the class.

It's a good example I believe. Also if your game end up supporting multiplayer or split-screen. If all you have are static, you will be in hell. (Been there also)

Advertisement

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."
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).

Crealysm game & engine development: http://www.crealysm.com

Looking for a passionate, disciplined and structured producer? PM me

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.


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.

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.


SRP is a decent idea, but everyone has a different definition of what a 'responsibility' is. A responsibility is as complex or as simple as you define it to be. You can come up with a sentential description of an image class that has and in it no matter how you try to reduce the definition of responsibility, and similarly come up with a sentential description of an image class that does everything but has no and in it.

Programmers can be grouped into those who want to do everything by the book, those who just throw code together until it works, and those who know that *neither* approach should be adhered to when it doesn't make sense. SRP is similar to Design Patterns; programmers who follow the idea rigidly without rationally considering simpler alternatives will end up producing worse code than those that know when to ignore the book.

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.

Yes, except often a lot of objects do just that. A lot of people also design objects in that way, which is something I prefer not to do.

I'm not sure what you're getting at with your reply when I flat out said that I prefer to always have a default constructor and have additional constructors simply set properties in an easy way.

You're basically reiterating the exact same thing I said for no real reason.

The constructor is not the only place to inject dependencies, it's just a convenient and fail-safe one.

I fail to see how it is "fail safe." If something were going to fail being set or passed as a paramater later it would certainly cause problems in the constructor, in terms of errors the constructor is the most volatile part of the object for error checking. Bloating the constructor with checking code is a bad idea anyway.

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.

I really don't get what your point is, my point about default constructors is that not having one implies an object cannot properly function without requiring a reference to whatever at creation time, which to me is a silly requirement and I personally have found many points in my code where I want to create an object and then set parts of it later.
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."

So what would be the correct approach in the example case, with the struct where a new member may be added? Where would the error be "reported" to, and in what way? More importantly, how would we know "whatever" was uninitialized, when its value as uninitialized may be undefined?

E.g.

FooContext fooContext;

fooContext.bla = <smth>; // old code that didn't know about the new "whatever" feature

// member whatever could have an undefined value, so how does this check that and "report" it, fail gracefully, etc.?

Foo *foo(new Foo(fooContext));

As like he says, it seems you need a constructor of some kind in fooContext to prevent this. Even if not one to force the programmer to pass a parameter, then at least a default one to initialize the variables so their values are all defined (perhaps so as to indicate special uninitialized states that Foo can then check for).

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.


SRP is a decent idea, but everyone has a different definition of what a 'responsibility' is. A responsibility is as complex or as simple as you define it to be. You can come up with a sentential description of an image class that has and in it no matter how you try to reduce the definition of responsibility, and similarly come up with a sentential description of an image class that does everything but has no and in it.

Programmers can be grouped into those who want to do everything by the book, those who just throw code together until it works, and those who know that *neither* approach should be adhered to when it doesn't make sense. SRP is similar to Design Patterns; programmers who follow the idea rigidly without rationally considering simpler alternatives will end up producing worse code than those that know when to ignore the book.

I think part of the problem is that people take these things as laws when they're intended to be taken as instruction. Knowing about SRP doesn't really help you. Knowing why SRP is good can help you a lot - and also let's you see more clearly where a line should be drawn.

Having an image loader is fine, but having an image that loads its own data is also fine, so long as each fits correctly into its own program and behaves sanely.

It's not a violation of SRP for an object that encapsulates an image to be responsible for doing its own loading. An SRP violation would be if you tried to make it do the work of a sprite as well.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

I guess what I'm getting at is the idea that if you have a class X that requires an instance of Y to function, then that requirement is best expressed by either passing it to the constructor at initialization, or by passing it as an argument to a required parameter of any method it's necessary. It's not really all that silly to design a class that can't function without its dependencies. There are lots of good reasons why an instance of an object should not exist unless its dependencies have been met. What's an iterator without something to iterate over? Or a StreamWriter without a stream? It's a runtime exception waiting to happen.

The "fail safe" I refer to is that your code will not even compile if you've left out an argument to the constructor or method. Setting parts of it later invites bugs because it's easy to forget to provide dependencies later. Sure, decent testing will catch these errors, and documentation "This class doesn't completely work unless you set property Z after construction" can help to mitigate these issues as well... if a programmer is paying attention. I would rather the compiler tell me. And let's talk about a year from now when the class is updated with a new dependency. If you had set the dependencies as separate fields, you get no helpful compiler juju to smack you at the back of the head helpfully informing you that you forgot to update 16 separate initializations of the class.

Just curious: What are your thoughts on bloating a constructor with error checking code? For me, checking to ensure arguments to the constructor are valid (for example, testing for null pointer arguments) and throwing exceptions when they are invalid would be preferable to encountering an exception during the some multi-step operation involving said class because the dependencies were not yet available.

This topic is closed to new replies.

Advertisement