Sign in to follow this  
Sc4Freak

How to get rid of all these globals?

Recommended Posts

My game code is filled with globals. For example, I have a "Renderer" namespace, which contains a whole bunch of globals that anyone can access. And a "SoundManager" namespace, "Input", etc. etc. ad nauseum. I don't think you could call them singletons, since they're not really *objects* that are instantiated only once. They're merely groupings of globals. As I start adding more and more, the system seems to be getting messier and messier. Functions from anywhere, at any time, can access and modify the internal state of the renderer - something that only the renderer itself should be able to do. And I can think of some other problems which could arise from my current setup. What I want to know is: what would be an elegant solution to my problem?

Share this post


Link to post
Share on other sites
A simple solution would be encapsulation.

You need to encapsulate certain logic in their (logical) classes. For example, all the rendering code should be encapsulated in the Renderer class. All data are to be private members of that class, which only the class itself can access.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sc4Freak
My game code is filled with globals. [...] a whole bunch of globals that anyone can access.

Change the way of accessing that information:

Put the actual data in the class that manages it and let the rest of your app get the data only through the instance of that data manager class.


namespace FooThings
{
class Foo
{
#region Variables
//Repeat this for each former global
private int formerGlobal;
public int FormerGlobal
{
get { return formerGlobal; }
set { formerGlobal = value; }
}
//---
#endregion

#region Constructor
public Foo(int formerGlobal)
{
this.formerGlobal = formerGlobal;
}
#endregion
}
}

namespace MainThings
{
class FooUser
{
#region Variables
private Foo fooBar;
#endregion

#region Constructor
public FooUser()
{
this.fooBar = new Foo(42);
}
#endregion

#region ACCESS TO NON GLOBAL
private void Method()
{
int a = this.fooBar.FormerGlobal;
}
#endregion
}
}



Share this post


Link to post
Share on other sites
Zanshibumi: Why are you doing this:

private int formerGlobal;
public int FormerGlobal
{
get { return formerGlobal; }
set { formerGlobal = value; }
}


When you can just do:
public int FormerGlobal;

You don't gain anything from making FormerGlobal a property as you can always change it later without having to change any code which uses it. It doesn't increase encapsulation and it leads to duplicated information which is bad.

Better yet though why not do:

// I personally don't like the idea of a central "Renderer" class but
// its the simplest to change to from your current setup
struct Renderer
{
Renderer( /* ... */ ) { }
void SemanticallyMeaningfulBehavior( /* some params */ )
{
// do something possibly involving formerGlobal
}

private:
FormerGlobal formerGlobal;
};

// now use it...
struct Foo
{
void SomethingRenderingRelated(Renderer& renderer)
{
renderer.SemanticallyMeaningfulBehavior( /* some param *);
}
};

Share this post


Link to post
Share on other sites
Quote:
Original post by Julian90
You don't gain anything from making FormerGlobal a property as you can always change it later without having to change any code which uses it. It doesn't increase encapsulation and it leads to duplicated information which is bad.

It's a common convention not to use public class-level variables.

google "coding style" or "bad practices" and you should find info about why not to do it.


P.S.: I do as you say some times, but when trying to explain something, I try to use standard practices as much as I can.

Share this post


Link to post
Share on other sites
Quote:

Zanshibumi: Why are you doing this: ....

I must agree with Julian here. Trivial properties (that just return or set the underlying field) in C# are a waste. Unlike C++, changing a field from public to private-but-exposed-via-a-property is trivial even without the powerful refactoring capabilities offered by most IDEs for C#, because in C# the change does not require a change at sites that use the public field, whereas in C++ it would (you'd have to add parentheses for the accessor call and potentially reshuffle the line). The arguments for trivial accessors in C++ are already pretty frail, but they fall down completely in C#.

Quote:

It's a common convention not to use public class-level variables.
google "coding style" or "bad practices" and you should find info about why not to do it.

It's a common C++ convention. I'm not arguing that encapsulation is bad, but a trivial property, a public pair of trivial set/get methods, and a public field are all equally "encapsulated" (that is to say, not at all). The rest is just implementation detail. The implementation detail makes for more work in C# and adds no benefit, whereas in might in C++ (using get/set methods does allow you to put constraints in later without changing client code, but that does not apply to C#).

Trivial properties are not good C# convention.





Of course, all of this is moot. The original poster isn't using C# (because he has global variables in namespaces). The notes about encapsulating all his globals does, of course, still apply.

Share this post


Link to post
Share on other sites
I read about that a lot of time ago and decided to follow the standard in this case.

For whoever reads this, Here you have a much more detailed discussion about the same.

I decide what are good conventions or bad conventions for me when I make my own code. If it's for someone else, I try to stick to the standard. At work I just use the one imposed.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sc4Freak
For example, I have a "Renderer" namespace, which contains a whole bunch of globals that anyone can access.


Could just make those globals static (file scope)...

Same with internal functions.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zanshibumi
Change the way of accessing that information:

Put the actual data in the class that manages it and let the rest of your app get the data only through the instance of that data manager class.
You don't really improve encapsulation that way though. Consider:

Case 1: We have two global variables, x and y. These variables can be altered from anywhere in the program.
x = 1;
y = 1;


Case 2: We take our two global variables and make them public properties of an object. If the object has global scope the variables may still be altered from anywhere in the program, but the line of code to do so will be slightly longer.
someobject.x = 1;
someobject.y = 1;


Case 3 (your suggestion): We make the two properties private and add public accessor(get)/mutator(set) methods. If the object has global scope the variables may still be altered from anywhere in the program, but the line of code to do so will be slightly longer.
someObject.setX(1);
someObject.setY(1);


So, where's the benefit of Case 3 as compared to Case 2 in terms of encapsulation?


Quote:
As I start adding more and more, the system seems to be getting messier and messier. Functions from anywhere, at any time, can access and modify the internal state of the renderer - something that only the renderer itself should be able to do.
Why did you design it like that?

Encapsulate the data correctly and then pass the information only to where it's actually required. We'd need to see specific examples to make more concrete suggestions.

Share this post


Link to post
Share on other sites
Quote:
Original post by Kazgoroth
someobject.x = 1;
someobject.y = 1;


Case 3
someObject.setX(1);
someObject.setY(1);


So, where's the benefit of Case 3 as compared to Case 2 in terms of encapsulation?

Case 3 would be
someObject.X = 1;
too. (in C#)

Which means no benefit or loss, from outside the class.

However, the class internal storage of that information could change in the future without changing in any way the access from outside nor having to recompile.

In case3, if you release a dll and some people start using it in environments you can't interfere with. You can still change the way that information is stored inside the class (for example, X and Y in a Point structure, a file or a remote server) without modifying the external access to that information.

You could argue that the same could be done simply by having:
public int x;
and later changing it to:
public int x
{
get {return GetVariableFromFile("x");}
set {SaveVariableintoFile("x", value);}
}
However it could bring problems


Anyway, I just set it like that because that's how I've seen teachers usually put code; every standard followed (to the extent of my knowledge and lack of custom).
As you all seem to think it's simply wrong to use that way of coding in explanations, I'll stop using it.

Share this post


Link to post
Share on other sites
Kaz's point was mostly that just putting global variables into a class, which is itself still globally accessible, is not really encapsulating anything. The different is merely in the syntax, the implementation cruft, regardless of what language you use. In some cases, you do gain a minor bit of encapsulation in the ability to modify the underlying field or impose constraints via the accessor or property methods. However, that ability is not really related to the larger design issue of the wide-open global variable spill.

Consequently, while wrapping up the variables in a class is a good first half-step, the rest of the step is accomplished by doing something more akin to what Julian suggested -- replacing the trivial access with something more semantically meaningful. That increases encapsulation, not some flimflamming about with properties/accessors/whatnot.

Share this post


Link to post
Share on other sites
Quote:
Original post by jpetrie
Consequently, while wrapping up the variables in a class is a good first half-step, the rest of the step is accomplished by doing something more akin to what Julian suggested -- replacing the trivial access with something more semantically meaningful. That increases encapsulation, not some flimflamming about with properties/accessors/whatnot.
Exactly. To continue the example, our x and y variables might indicate the coordinates of whatever our object respresents. We might therefore make them private properties of the object, but rather than providing accessors (you may still want these) and mutators (you probably don't want these) we could provide a moveObject(int x, int y); method which allows us to change these values as a semantically meaningful action.

After all, when you're playing a game do you want to change the x coordinate of your unit, or do you want to move it left or right?

Share this post


Link to post
Share on other sites
This is an example of what might currently be in my program:


namespace Renderer {
IDirect3D9* D3D9Interface;
IDirect3DDevice9* D3D9DeviceInterface;

void RenderSprite(/*...*/);
}



Where anything can access those variables. The input manager could call D3D9DeviceInterface->Release(); if it really wanted to. After reading the thread, I get the idea that I should do it like this instead:


class Renderer {
private:
IDirect3D9* D3D9Interface;
IDirect3DDevice9* D3D9DeviceInterface;

public:
void RenderSprite(/*...*/);
}



Where only RenderSprite can access the internals of the Renderer. Does this seem correct?

Share this post


Link to post
Share on other sites
Quote:

After reading the thread, I get the idea that I should do it like this instead:
<snip>
Where only RenderSprite can access the internals of the Renderer. Does this seem correct?


Yep, you've pretty much hit the bullet on the head :). There are a whole heap of "principals" that govern what you should have in a class but it's up to you how many of them you want to follow. Even if you decide not to follow them for this project because it would be to much work to change things (which I think is quite likely and reasonable) there still good to know about even if you just keep them in the back of your head when designing so heres some links:
SRP (Single Responsibility Principle), "A class should have one, and only one, reason to change."
OCP (Open Closed Principle), "You should be able to extend a classes behavior, without modifying it."
LSP (Liskov Substitution Principle), "Derived classes must be substitutable for their base classes."
DIP (Dependency Inversion Principle), "Depend on abstractions, not on concretions."
ISP (Interface Segregation Principle), "Make fine grained interfaces that are client specific."

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

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

Create an account

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

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this