Is This A Bad Habit?

Started by
53 comments, last by superpig 17 years, 11 months ago
Stop, and read this (especially sections 15 and 16).
Advertisement
Quote:Original post by ZMaster
@superpig: But wouldn't it be even better OO design to have CEnemySoldier and CCutsceneSolider derive from CSkinnedCharacter which implements a rendering routine for itself. Let's say via a virtual protected function which can be accessed by both CCutsceneSoldier and CEnemySoldier to render themselves and which could be overridden for any other CSkinnedCharacter which wants to implement it's own rendering function though?
I'm not sure about this... just asking.


Remember that by inheriting from a class, you don't just inherit its functions and fields; you also inherit the references to it (because your derived class could be type-cast to the base class). By inheriting CEnemySoldier from CSkinnedCharacter, CEnemySoldier could now be messed with both by code that refers to CEnemySoldier and by code that refers to CSkinnedCharacter. If you go and modify that code, you now need to consider that your changes aren't going to break CSkinnedCharacter or CEnemySoldier. That's quite a lot of extra dependencies, and it's something I'd consider particularly precarious because the dependency isn't obvious on the surface - you need to know that CEnemySoldier inherits from CSkinnedCharacter when you're changing the code.

And furthermore, if CSkinnedCharacter's principal purpose is to handle rendering, and you're talking about deriving from it but then replacing the rendering routine with your own stuff... well, then why derive from it at all? You wouldn't be using any of the stuff that you inherit.

It feels like inheritance should be the correct approach, because CEnemySoldier is-a CSkinnedCharacter. However, you can also spin things around a little and treat CEnemySoldier as a controller object - one that coordinates and controls other objects, like models and guns and sounds and so on. In that context, CEnemySoldier has-a CSkinnedCharacter, so composition is the appropriate relationship.

Richard "Superpig" Fine - saving pigs from untimely fates - Microsoft DirectX MVP 2006/2007/2008/2009
"Shaders are not meant to do everything. Of course you can try to use it for everything, but it's like playing football using cabbage." - MickeyMouse

In my opinion having to use globals means a bad design. There are ALWAYS other ways of doing it. When I first started programing, many, many years ago I would use a global here and there because it was much easier and seemed to be the right way to do it. As I learned I found out that there are better ways to accomplish the same things. Look at your design and figure out how to get rid of the globals.

theTroll
Quote:Original post by superpig
It feels like inheritance should be the correct approach, because CEnemySoldier is-a CSkinnedCharacter. However, you can also spin things around a little and treat CEnemySoldier as a controller object - one that coordinates and controls other objects, like models and guns and sounds and so on. In that context, CEnemySoldier has-a CSkinnedCharacter, so composition is the appropriate relationship.


Yep.

But then, any time the name suggests a relationship that doesn't seem right in code, that name is suspect. So let's instead go with CCharacterSkin, or perhaps CSkinnedAvatar. :)
Fair enough [smile]

Richard "Superpig" Fine - saving pigs from untimely fates - Microsoft DirectX MVP 2006/2007/2008/2009
"Shaders are not meant to do everything. Of course you can try to use it for everything, but it's like playing football using cabbage." - MickeyMouse

From my own experience, globals have helped my games with performance due to less passing to the stack. Say, a drawPixel(int x, int y, char color). Now imagine that being called every single frame, if not hundreds to thousands of times each. You have three variables being passed to the stack constantly which is really bad. Although globals are an excellent choice to get out of stack usage, consider ways to avoid globals if possible, such as the 'inline' keyword for your drawPixel() function. Sure, it makes your program bigger, but performance is more efficient, and your code is much cleaner as well. For DirectX, you'll find yourself using globals for handles etc.

What you can also do is add an annonymous namespace to your global variables. This keeps your globals accessable in one file, thus your globals are safer from misuse in another file. How do you do that? For example...

namespace {
int g_x, g_y;
}

That's it. You could also make a defined namespace for your global variables.

namespace Data {
int g_x, g_y;
}

Now, to access this in any file, all you do is
Data::g_x = 5;

As you can tell, there is a variety of solutions, so it just comes down to which one you'll need for the problem. My point in all of this is use globals, but do so in a way that it doesn't get in the way with code misuse as much as possible. I hope that helps!
Quote:Original post by nullsmind
From my own experience, globals have helped my games with performance due to less passing to the stack. Say, a drawPixel(int x, int y, char color). Now imagine that being called every single frame, if not hundreds to thousands of times each. You have three variables being passed to the stack constantly which is really bad.

...And how would you propose using globals to alleviate this situation?
I'll explain further. Say you have this...

void drawPixel(int x, int y, unsigned char color) { // ... }

You're passing three variables to the stack here, and this is pretty bad because you know this is a frequently used function. There's a few things you can do for optimization... get rid of the stack completely, for one. You can set these up as globals...

#define SCREEN_WIDTH 800

namespace {
int g_x, g_y;
unsigned char g_color;
}

void drawPixel() {
videoBuffer[g_x + SCREEN_WIDTH * g_y] = g_color;
}

Cool, there's no passing to the stack here now, and the annonymous namespace helps keep your globals accessable in only one file. You can use the static keyword instead of the annonymous namespace, but it has been declared deprecated. Now, there's one other way we can get rid of these globals still - simply inline the drawPixel() function.

inline void drawPixel(int x, int y, unsigned char color) {
videoBuffer[x + SCREEN_WIDTH * y] = color;
}

Now, your globals are no longer needed. So, my point again is there will be times when you will need globals for certain tasks, but avoid globals as much as you can like we did above. We found that globals weren't necessary for this particular task, and thus our code here is more compact.
That's a questionable optimization at best. As you point out inlining is the better solution - and if you pass the parameters as globals then the compiler may not recognize that the function can be inlined. If you use the function parameters correctly the compiler can choose the most appropriate method of passing the parameters, which may be the stack or it may be in registers - but the compiler can make better decisions than you.

Furthermore as a number of people have said already - page faults are going to be a major issue. The cost of passing parameters on the stack is no more than a handful of push/pop instructions (if it's more than you should be passing by reference anyway). On the other hand if the global variable is on a page that isn't currently swapped in you will page fault when you access the variable. The cost of a single page fault is higher than thousands (if not more) of push/pop instructions. Using a global variable in this case is SLOWER than just using the stack even if you call the function thousands of times.

Moral of the story (as with all optimizations like this) is that unless you have profiler output proving that a significant percentage of your program time is spent pushing the parameters to the stack there is absolutely no reason to consider doing this.
I, too, fail to see that as a compelling example, nullsmind (especially since you continue on to illustrate how it can be achieved without globals anyway). Pushing parameters on the stack is an amazingly fast operation when done correctly (i.e., don't pass by value so copy cts don't need to be called, et cetera), and is unlikely to be the cause of the slowdown in most cases.

For the example you gave, perhaps the fact is that you are drawing in an extremely inefficient manner (pixel by pixel) and should instead optimize that instead of trying to gain some small measure of performance by senselessly using globals to avoid parameter passing. You've optimized the wrong thing.

This topic is closed to new replies.

Advertisement