Jump to content

  • Log In with Google      Sign In   
  • Create Account


Why are static variables bad?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
57 replies to this topic

#21 Daivuk   GDNet+   -  Reputation: 358

Like
4Likes
Like

Posted 04 September 2013 - 07:32 AM

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)



Sponsor:

#22 Satharis   Members   -  Reputation: 949

Like
0Likes
Like

Posted 04 September 2013 - 01:27 PM

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, 04 September 2013 - 01:29 PM.


#23 cozzie   Members   -  Reputation: 1539

Like
1Likes
Like

Posted 04 September 2013 - 03:10 PM

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

#24 frob   Moderators   -  Reputation: 19624

Like
0Likes
Like

Posted 04 September 2013 - 03:36 PM

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.


Check out my personal indie blog at bryanwagstaff.com.

#25 smr   Members   -  Reputation: 1588

Like
0Likes
Like

Posted 04 September 2013 - 03:58 PM


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, 04 September 2013 - 04:03 PM.


#26 Nypyren   Crossbones+   -  Reputation: 3937

Like
0Likes
Like

Posted 04 September 2013 - 04:51 PM

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.

#27 Satharis   Members   -  Reputation: 949

Like
-1Likes
Like

Posted 04 September 2013 - 05:23 PM

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.

#28 mike3   Members   -  Reputation: 149

Like
0Likes
Like

Posted 04 September 2013 - 05:28 PM

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


Edited by mike3, 04 September 2013 - 05:50 PM.


#29 Khatharr   Crossbones+   -  Reputation: 2909

Like
0Likes
Like

Posted 04 September 2013 - 07:55 PM

 

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.

#30 smr   Members   -  Reputation: 1588

Like
0Likes
Like

Posted 04 September 2013 - 11:18 PM

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.



#31 kunos   Crossbones+   -  Reputation: 2203

Like
0Likes
Like

Posted 04 September 2013 - 11:29 PM

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.

 

because it will put the object in a "ready to do stuff" state and will clearly express dependency, stopping you from doing nasty circular dependencies.

For example, your way I can do:

 

A a;

B b;

 

a.setPointerToB(b);

b.setPointerToA(a);

 

Bang, you have coupled 2 classes, created a non clear construction dependency.. multiply this by hundred of thousands of lines and you'll see the mess you are putting yourself into.

And let's talk about validity check.. if I have a class A that depends on B.. I write a constructor:

 

A(B& b)

 

I am stating a FACT, that A cannot possibly work without B PLUS that B will have to outlive A. Ex in some engines, a texture cannot be loaded without a reference to a graphics device. Plus I move the problem of having a VALID B to the caller.. if he has a nullptr he'll have to deference it IN HIS CODE, he will get the crash IN HIS CODE. I won't have him walking to my desk saying the game crashes in my code. I have made my intentions totally clear.

I am also clearly pointing out a creation order, if you want an A you need to have a B FULLY CONSTRUCTED first.. I am using the compiler to express this concept, I don't need to walk to my colleague's desk and explain him and the rest of the team what come first and what's the correct member function sequence to call.. it's ALL IN THE CODE.. you mess it up? It won't compile.

 

 

I dont have to pollute my class A code of "if (b)" checks.. basically you are saying your code is always checking for every possible member value... if you design your classes properly, you just don't need to do that.

 

By having a default constructed class you are creating a custom initialisation scheme different for EVERY class... some might need to have method X called after Y called after Z to init properly.. some might need Z Y X.. you're just forcing the user of the class to get into your code and understand how it works.. that's not cool man.

Or probably you are not working in C++ and missed the last 15 years of C++ evolution.. I don't know.. but I am happy to disagree with you deeply and enforce proper construction and destruction in my team.


Edited by kunos, 04 September 2013 - 11:33 PM.

Stefano Casillo
Lead Programmer
TWITTER: @KunosStefano
AssettoCorsa - netKar PRO - Kunos Simulazioni

#32 smr   Members   -  Reputation: 1588

Like
1Likes
Like

Posted 04 September 2013 - 11:31 PM

RAII :)

#33 fir   Members   -  Reputation: -448

Like
0Likes
Like

Posted 05 September 2013 - 12:01 AM

As an example about such orphan global way shared variables 

i was taking about 

 

I got small 

 

Ship {float x,y, angle; } ship

 

structure and some code to draw it, move, maybe firing a bullets, below that. This code needs to translate angle into some nx, ny by taking a cos and sin on angle. It is needed in say 5 places so you got to calculate it five times or maybe you would like make nx, ny as a some shared cache like temporary values so you can need calculate it once and than use it five times

 

It is optymistaion but it brings the trouble I was naming as an orphan variabled: there becomes to be a gap (some dependency gap) between 'producers' code who set up nx, ny values and the 'clients' code which are using them - this gap in general is problematic indeed  - it is much more troublemaking than writing five times explicitely the hard dependency and calculating such sin cos 5 times.

 

But there is a question how would you (some person) as a programmer resolve that trouble?


Edited by fir, 05 September 2013 - 01:43 AM.


#34 kunos   Crossbones+   -  Reputation: 2203

Like
0Likes
Like

Posted 05 September 2013 - 12:11 AM

The obvious way in C++ is to have:

 

class Ship

{

public:

void rotate(float angleRad); // THIS WILL ROTATE BOTH ANGLE AND UPDATE THE DIRECTION VECTOR SO THEY ARE ALWAYS ALIGNED

float getAngle() { return angle;}

Vector2 getDirection() {return direction;}

 

private:

Vector2 position;

Vector2 direction;

float angle;

};

 

The calculation is only done once, every time "rotate" is called, Ship is always valid. Welcome to 2013.. enjoy your 1960 language ;)

IMO, "angle" is not needed at all.


Edited by kunos, 05 September 2013 - 12:17 AM.

Stefano Casillo
Lead Programmer
TWITTER: @KunosStefano
AssettoCorsa - netKar PRO - Kunos Simulazioni

#35 mike3   Members   -  Reputation: 149

Like
0Likes
Like

Posted 05 September 2013 - 01:22 AM

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.

 

So how do you suggest to avoid crashing when someone "fails to pass something" as you suggest should be avoided? And in the example given, that something was to be passed to a constructor.


Edited by mike3, 05 September 2013 - 01:23 AM.


#36 Satharis   Members   -  Reputation: 949

Like
0Likes
Like

Posted 05 September 2013 - 04:55 AM

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

The ideal thing to do would be to treat the context as a volatile object and check for null on .bla before attempting to use it.

And before someone gets their panties in a twist about nullptr checking, I could say the same thing would actually work with a constructor because it is perfectly possible to pass a null pointer to a constructor expecting a pointer to an object, either way it would crash. You could also pass a valid pointer but then the object pointed to may be in an invalid state, so of course you would want to check for that as well.

That was my point about not trusting outside data, because if it can fail it probably will at some point. As for how to report it that sort of depends on the context of the object, if it is imperative to the program running or whatever. Personally I would log it and throw an error if it is a serious programmer error or just log it and skip over if it is something that may have been user caused.

because it will put the object in a "ready to do stuff" state and will clearly express dependency, stopping you from doing nasty circular dependencies.

But what if you don't necessarily need that dependency for all functions? Why force someone to pass it if they don't need it for the functionality?

For example, your way I can do:
 
A a;
B b;
 
a.setPointerToB(b);
b.setPointerToA(a);
 
Bang, you have coupled 2 classes, created a non clear construction dependency.. multiply this by hundred of thousands of lines and you'll see the mess you are putting yourself into.

Honestly I find your answer here kind of contrived because you're making an assumption that you have two identical objects asking for a pointer to the same type. That's already a weird situation and even taken out of that situation you could create the same circular dependency by using three objects with constructors, using a constructor as a prevention to passing circular dependencies is odd at best. I'd hazard to say the object design is a bit of a problem if you end up in a situation like that in the first place.

But if you have a more real world example, I'd love to hear it.

And let's talk about validity check.. if I have a class A that depends on B.. I write a constructor:
 
A(B& b)
 
I am stating a FACT, that A cannot possibly work without B PLUS that B will have to outlive A.

Well no you're actually stating a requirement that a reference to an object be passed in and are assuming that object will stay live longer than your object intends to use it, but that of course could be false.

Ex in some engines, a texture cannot be loaded without a reference to a graphics device. Plus I move the problem of having a VALID B to the caller.. if he has a nullptr he'll have to deference it IN HIS CODE, he will get the crash IN HIS CODE. I won't have him walking to my desk saying the game crashes in my code. I have made my intentions totally clear.

The thing is what you're suggesting really isn't fundamentally any different from having an object passed in later and checking it for validity before using it, it would have the same consequence of passing it in the constructor. The only real point you're presenting with a constructor like that is that, yes, you expect the user to know that the object cannot be created without a reference to B.

But really thats just a matter of style and varies by object, what if B was something that could change and didn't neccesarily need to be a reference? Would you pass a pointer in? What makes that information clear to the caller that they could change it later? You'd have to provide a set function of course at which case ambiguity starts to set in about how "safe" that object being passed in to the constructor really is. Which is why my method of doing it removes ambiguity, it makes an assumption that an object will not do anything upon creation and will only fail if asked to perform a task without the required dependency being set and available at that time. In your choice of object, sure, just passing in one thing could be more clear. But that will vary highly by object and have no continuity between them.

I dont have to pollute my class A code of "if (b)" checks.. basically you are saying your code is always checking for every possible member value... if you design your classes properly, you just don't need to do that.

I'm not sure where that assumption comes from, you're implying that your objects always rely on passed in references to dependences at construction time and are making a total assumption that those objects are always valid. If you accepted a pointer at any point in a constructor you would have to do the same null check for the object itself. In terms of using the object that was passed in, the code is exactly the same and you can't rely on values being valid, that code can't really change by passing a reference vs something else.

If anything you would save a little code on doing nullptr checks when using pointers. You're also making a rule that your classes can never change their dependencies without being destroyed and recreated, which is something I find debatable as a good rule.

By having a default constructed class you are creating a custom initialisation scheme different for EVERY class... some might need to have method X called after Y called after Z to init properly.. some might need Z Y X.. you're just forcing the user of the class to get into your code and understand how it works.. that's not cool man.

That's not true at all, in fact I'm not sure how you would possibly order how setting properties is called on an object, the only assumption my method makes is that calling certain methods will fail, likely logging some sort of error message if a dependency is not present when it needs to be. In most cases I would encourage the user of my code to use the overloaded constructors that fill the dependencies and only set them manually after construction if they have reason to change them or delay construction.

A good example would be a game enemy, perhaps you want to construct it and set information about it like it's position and stats or other information. But how do you set all that information at construction time? Use a factory object? Of course you could but in reality you're doing the same method as I suggest and shoehorning it into a factory interface, so the thing isn't really as important as much as the method of construction here. Also what if you want to add the entity to a world after creation? Or perhaps change a dependency of it and spawn it again in the world without destroying it. In all of these cases really, you couldn't do them if you assume the object is fully completed and "alive" at construction time by passing dependencies.

By following your methodology you're making a point that if an object requires a reference then it cannot be changed after construction, otherwise you lose the positives you even mentioned.

Or probably you are not working in C++ and missed the last 15 years of C++ evolution.. I don't know.. but I am happy to disagree with you deeply and enforce proper construction and destruction in my team.

Okay, except I am working in C++ and tend to work in C++. I'm not sure where the smart-ass attitude comes from just because someone has a different strategy for creating objects than you do, I see plenty of flaws in both methods, mine simply attempts to create consistency among how objects can be expected to behave.

--I'll cut off here until I get more replies, this reply is already extremely long between two different people to reply to!

#37 Juliean   GDNet+   -  Reputation: 2330

Like
3Likes
Like

Posted 05 September 2013 - 08:25 AM


But what if you don't necessarily need that dependency for all functions? Why force someone to pass it if they don't need it for the functionality?

 

Its still better than having to call a specific setter method, or pass the dependency in the actual method directly. For me, an object should be fully usable after being constructed. How does it make sense to you, for not being able to call certain methods unless you manually set specific private fields of the object? Maybe its just me, but thats not much better than having static global variables to avoid passing stuff. Its kind of like:

Car myCar;
myCar.Drive(); // error message: can't drive without an engine!
Engine engine;
myCar.SetEngine(engine);
myCar.Drive(); // error message: engine is missing spark plugs
SparkPlug plug;
engine.SetPlug(plug);
myCar.Drive(); // error message: car can't drive without a fuel tank

Wouldn't that drive anybode crazy? If a car can't drive without an engine, it shouldn't be constructable without an engine. You might want to have a setter method to change the engine later, but whats the point of building/constructing it so that you can't even use it without adding all other parts?

FuelTank tank;
SparkPlug plug;
Engine engine(plug);
Car car(engine, tank); 
car.Drive();

There. Clear dependencies, and if you pass in references you can save some unnecessary error checkings. If someone passes in a dereferenced nullptr by will, its their fault.

 

I think you have to look at it from another perspective: Its not how you design your classes, but how the objects actually work. If an object can't work without certain dependencies, then they should go in the constructor. Again, whats the point of having an object being default constructable if all it does is crash on half of the methods? If an object works fine without certain constructor arguments, then leave it a default constructor. But forcefully designing classes so that they don't need a special constructor seems a tad bit backwards, and will only confuse any users other than the one who wrote the code.


Edited by Juliean, 05 September 2013 - 08:28 AM.


#38 Sacaldur   Members   -  Reputation: 477

Like
2Likes
Like

Posted 05 September 2013 - 09:08 AM

(I have to admit I didn't read everything...)
There are some slightly different statements which should not be mixed up.

1.) Global variables are bad. They infringe the object oriented design, no one likes them, and they don't have any friends!
2.) Dependencies should be validated during the objects initialization by passing corresponding objects as constructor arguments or by initializing them inside the constructor. There should be no initialization Method to be called from the outside after the object was created. The object already has to work.
3.) Other stuff, the object ist not dependent on should not be passed to the contructor.

In my opinion the 1. statement is true. There are in most cases other different and better solutions.
While talking about the 2. and 3. statement it's important to know what is an object depending on and what it is not depending on. A player may need to know his environment (map/level/world/...), but a wizard wand or warrior sword refining machine doesn't depends on these objects the same way. Thats why the world may should be a players constructor parameters, but a weapon should not be a refining machines constructor parameter - instad it should be the "refineWeapon"s method parameter.

An other example for a dependency could be Character and the CharacterState's subclasses. A character always needs to know his state - what the character is doing. "Nothing" could be one of the subclasses so a null pointer would be an invalid state. The constructor should set an initial state no matter if there is a parameter for the state or not (the questen would be: is there a default character state in your game?). A SetState Method would still be a valid possibility, but it's not a setters job to initialize the object.

In my opinion passing an object to the constructor and passing an object to a method are suitable for different cases. Both should be used, but not exchangable.

#39 Satharis   Members   -  Reputation: 949

Like
0Likes
Like

Posted 05 September 2013 - 09:16 AM

Its still better than having to call a specific setter method, or pass the dependency in the actual method directly. For me, an object should be fully usable after being constructed.

You say that but honestly I've rarely seen situations, especially with popular libraries where an object will be fully usable and only has one constructor. Often it has multiple overloads that set specific information about the object. For instance in SFML you can create a sprite without having assigned a texture to it and it won't draw anything, consequently you can also create the sprite and pass a texture into the constructor if you so choose. Same thing really.

Honestly my main complaint with the examples given thus far is that they're all using the point of there only being -one- dependency and assuming the object will be a floating error withou tit, and in reality I just have rarely if ever had that happen. Unless you're passing around some super god-class like "engine" or something to every single object, which is about the same as using a global.
 

How does it make sense to you, for not being able to call certain methods unless you manually set specific private fields of the object? Maybe its just me, but thats not much better than having static global variables to avoid passing stuff.

I don't think it's at all the same, a global is a completely hidden dependency and can be included in the code anywhere, whereas often a well designed object will be self documenting in what it needs in order to complete it's job. For instance if you have a game entity and don't have a world set for it, clearly you wouldn't expect a "spawn" command or something to work without setting such.

Which is where we come to an impasse, you say that making a single constructor that requires a world is more clear that the object cannot properly do all its functionality without a world object, of that I agree. But it often isn't just a single world object, it's other subsystems and setting information about the object that relates to how it will be placed in the world or handled. At what point do you consider it lacking information?

To me it makes sense to offer a constructor that takes a world and also one that doesn't, so setting a world can be delayed until a later period and if you attempt to use an object without setting it properly it alerts you as such. It'd be like having a game state manager and trying to pop every state off it and then attempt to use it. Clearly a constructor will not fill a dynamic job like that, even if you may use one to set an initial state.
 

Its kind of like:

Car myCar;
myCar.Drive(); // error message: can't drive without an engine!
Engine engine;
myCar.SetEngine(engine);
myCar.Drive(); // error message: engine is missing spark plugs
SparkPlug plug;
engine.SetPlug(plug);
myCar.Drive(); // error message: car can't drive without a fuel tank
Wouldn't that drive anybode crazy? If a car can't drive without an engine, it shouldn't be constructable without an engine. You might want to have a setter method to change the engine later, but whats the point of building/constructing it so that you can't even use it without adding all other parts?

Yes, and in your example you're talking about one dependency. I don't see how it is much more clear to have a constructor that is default and one that asks for an Engine, and then to have one setter for an engine as well. Literally the only difference you're making is that you're forcing an assumption that some kind of engine must be passed into the object at creation.

However realistically there is merit to the idea of requiring someone to pass into the constructor objects in particular that the object cannot function without. But to me I actually -don't- find objects like that very often. In your example of an engine, sure, you basically can assume that the car cannot work without an engine. That would be fine, but the problem is that you're creating clarity on that one particular object and treating each one differently. To me that ends up confusing, it's like if you look at the car you see it needs an engine, well sure, lets give it an engine. How about the drivetrain? Maybe it's abstracted away to represent the wheels as well, well does the car need wheels to drive? How do you know? Should you require wheels in the constructor? What if you want the car to be able to drive without them? But poorly.

You get into a scenario where it is almost subjective on what is "required" for the object to work. One could argue basic subsystems like the rendering and updating are required, but what if you pass them in through paramaters and not through the constructor? At that point the constructor is a bit of a mystery, each object can be implemented completely differently depending on the developer's style.
 

I think you have to look at it from another perspective: Its not how you design your classes, but how the objects actually work. If an object can't work without certain dependencies, then they should go in the constructor.

Right, but how often is that true? I coudl just as easily, in your example, say that my car starts by constructing its own engine, and I allow you to set it later. In that case the dependency suddenly disappears, and yet the car still needs an engine. You'd still have to infer the behavior of the car if you want to change the engine. Otherwise you woudln't even know what you're affecting by changing it.
 

Again, whats the point of having an object being default constructable if all it does is crash on half of the methods? If an object works fine without certain constructor arguments, then leave it a default constructor. But forcefully designing classes so that they don't need a special constructor seems a tad bit backwards, and will only confuse any users other than the one who wrote the code.

I understand you're trying to give logical reasoning and I'm totally for that, but really, I see negatives to both methods. You're trading clarity for one object for clarity as a whole in how each object behaves similarly at a basic level. I tend to find that objects operating similar to each other makes them much easier to understand and predict the behavior of.

But whatever, people can downvote me if they like for having a different opinion, I'm not vain enough to assume my thinking is perfect or something. If I find a better compromise between the two methods, of course I would use it. I'm interested in what the most effective design is, and what makes my job and the job of others easier, not what popular opinion is.

Edited by Satharis, 05 September 2013 - 09:19 AM.


#40 Juliean   GDNet+   -  Reputation: 2330

Like
1Likes
Like

Posted 05 September 2013 - 10:10 AM

Appearently, this argument would end in us just repeating ourselfs (at least this would soon happen to me), so I quess we can say that different environments demand different solutions. Just like it was mentioned before how you should be aware of when and how to use common idioms/design patterns, in the end you'll have to decide what style of programming you want to apply. You won't be able to say which one works better for you though unless you tried both out. For me, the matter I proposed worked out much better, and I wasn't going there because of the popular opinion, but because of whats not been working in my latter projects, since I wanted those to improve. I've also come across a lot of classes that wouldn't be usable in a sane manner without having some sort of depency implied, unlike you. It depends a lot of code style though, just personally, I feel my style regarding this is working well for me right now, not saying that it is perfect, but I'll just say that regarding the "popular opinion" thing, there almost always is a specific reason why something is popularily accepted. You don't have to do so too "just cause", but the chance of something working for someone else if say 95% of all other people did use sucessfully is a lot higher than what the rest 5% might do differently (though not granted). Oh well, its getting a bit too philosophically here, never mind. Just one thing that stuck out, and thats a prime example of how different approaches justify differnt coding styles:

 


Which is where we come to an impasse, you say that making a single constructor that requires a world is more clear that the object cannot properly do all its functionality without a world object, of that I agree. But it often isn't just a single world object, it's other subsystems and setting information about the object that relates to how it will be placed in the world or handled. At what point do you consider it lacking information?

 

I'd personally do this completely different. I wouldn't align the game object to the game world at all. I'd give the game world a "RegisterGameObject"-method, if not a method to create and register a game object at once. That way, this is out of the question - neigther does an object need a constructor with world reference, nor a setter. A game object is then considered to being spawned as soon as it is in the game world. Anything like respawn mechanisms would be build seperately and simply add new game objects (probably as copy from an existing "template" object) to the game world. I must say I'm a little damaged from how awesome I find entity/component systems, but I hope you get the idea. Just to emphasise that, as I belive, this is in the end more of a result of the surrounding design, than it is a single class.






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS