CABAListic wrote:nullsquared wrote:I smell design issues. Not only is it a global, but it is a global with a non-trivial constructor that needs to log something?
So, next thing you're going to tell me that you have never, ever inserted a log statement inside a constructor, not even temporarily for debug purposes?
In a constructor, yes. Not in a global variable's constructor.
So tell me again, what makes your global variable better for the log system?
I can create as many of it as I want. I can even create derived types (such as, you know, std::ofstream
)
JohnJ wrote:And if you really need your own class for it:
Code: Select all
class logger { ... };
// .hpp
extern logger log;
// .cpp
logger log;
// elsewhere
log << "logging stuff, yay!";
That's almost exactly how my singleton implementation works, but mine is a little more flexible with construction. After seeing this, I'm starting to think the main disagreement between you/me on this is simply due to an inconsistency of the definition of "singleton".
Perhaps. What you're talking about is not a singleton. That is simply a global variable. Nothing to do with
singletons.
Input is cause, stuff happening is effect. Processing, applying, or halting program execution with a prompt are all effect. Input does not care about effect, as it is the cause. The only thing it does (and, well, the only thing it can do) is capture input.
My point was "input" is a vague verb, and is therefore ambiguous in many cases. You're little code example proves this very nicely:
Not exactly self documenting.
Alright, so my input class is named a bit ambiguously because its namespace is also named input. But you can't just mash those parts together like that - they have a context (one which I cut out so I can only show you the input part - imagine there's more to it in the /* ... */ blocks).
In the case of the code you posted, it looks like it's more likely to be problematic with chains of backward dereferences, like "body.world.root.input.keyboard.isKeyDown()" (just an example). Rather than "Keyboard->isKeyDown()". I'll assume for now that I don't need to explain why the former is horrible design, but let me know if you need me to explain it. Now I'm not saying you have a problem like this, but that it's my best guess as to where a singleton could help, considering I don't know much about the rest of your code.
No. Once again, I smell design issues.
body is a physics body class. The world is the physics world class (they'e both in the phys namespace). The body might need to know about it's world, for example, to be created properly (for example, when I use Newton, to create a body, you need to pass the World it is created within). It does NOT need to know anything about keyboards, input, roots, etc. Give me a better example for your "reference chaining" and I'll show you how I don't have that problem.
You've even posted some yourself. "std::clog", for example, is just as much a singleton as "Ogre::LogManager::getSingleton()" for all practical purposes.
No. I can create as many ostreams (or types derived from ostreams) as I want. std::clog is simply a global.
To be perfectly honest I don't like arguing for singletons, because I don't like them in most cases anyway. I already gave you the example of the keyboard class. I could type up a long code example demonstrating exactly why it's advantageous, but at this point I've wasted enough time already on this thread.
Well then instead of wasting your time on
telling me about the "keyboard example," you should've simply
shown me the "keyboard example." Besides my ambiguous naming, my method is perfectly fine. Look at it this way if you really can't get past the simple naming:
Code: Select all
engine::input::inputSystem input(root->hwnd());
while (...)
{
...
input();
...
}
No singletons. No globals. No need.
steven wrote:I don't see why I would need more than one EngineManger, GraphicsManager, LogManager, NetworkManager, InputManager and some others.
So why should I bother to pass references or pointers everywhere and not simply access them via singleton?
It makes lots of constructors and methods simpler.
I'll assume that your GraphicsManager is the equivalent of my renderer (handles the graphics and actual rendering).
In my game, you'll be able to have in-game computers. These in-game computers can render scenes from the game (like a camera). In my non-singleton case, I can simply create a second renderer specifically for the in-game computer, and done:
Code: Select all
computer::computer()
{
// create our own scene manager so that the scene doesn't conflict with the game's scene
_sceneMgr = ...;
_renderer.reset(new forwardRenderer(_screen.viewport(), _sceneMgr));
}
void computer::tick(...)
{
_renderer.render(); // render to our screen
}
Since my entity manager (aka my game::world) is also not a singleton, my in-game computer can start spawning its own entities in its own little world, etc.
mirlix wrote:
Not excatly. My target with the getInstance() method would be that no error occurs, you simply get always the same instance when calling the method. This would solve the problem of two instances existing at the same time. I cant disagree that a singleton is always a global class, but this shouldnt be the reason for making a singleton, sometimes you have to but quite often there is another way.
I see what you're saying. What about, perhaps:
Code: Select all
// instead of thereShouldOnlyBeOne, I'll call it foo this time to be easy
class foo
{
private:
foo();
foo(const foo&);
foo &operator=(const foo&);
friend foo &createFooImpl(const char *file, const char *func, unsigned line);
};
foo &createFooImpl(const char *file, const char *func, unsigned line)
{
static bool alreadyExists = false;
if (alreadyExists)
{
std::cerr << "created second foo in " << file << " in " << func " on line " << line;
assert(false && "create only one foo");
}
static foo theFoo;
return theFoo;
}
#define createFoo() createFooImpl(__FILE__, __FUNCTION__, __LINE__)
void bar(foo &ourFoo)
{
foo theFoo; // compiler error
foo &theFoo2 = createFoo(); // "created second foo in main.cpp in void bar(foo &ourFoo) on line 123" <assert>
}
int main()
{
foo &ourFoo = createFoo();
bar(ourFoo);
}
This way you find the issue, instead of simply avoiding it.
