1

I have several subsystem managers for various uses, for example:

AudioManager
CollisionManager
InputManager
etc.

At first I wanted them to be all singletons, however now want to make the architecture a little bit cleaner. The main idea is to use only one big singleton (for example System) and it will contain all of these objects.

So interaction would be like this :

System::getAudioManager()->playSound("Explosion");

//Before it was like this
// AudioManager::playSound("Explosion");

However, now classes who need to use these "managers" must get access to the instance of the actual class, with singletons I would use just an include to a file which is needed:

#include "AudioManager.hpp"

Now I have to make this:

#include "System.hpp"

It means that I have to include the main singleton file which contains all the includes, all the other subsystems created and used. Does it have some overhead (performance) or cons?

6

Both of these are a huge violation of the interface segregation principle. It also makes your dependencies implicit rather then explicit. This comes at a cost you wont notice until you try to make changes to your existing system. It will be very hard to move a part of the system elsewhere without tracking down and rewriting the way it accesses those things that it depends on.

If you don't care about that then fine. Go nuts. Just please don't ask me to fix it later.

Why? Cause I've seen this before. It's called the service locator pattern. It works. But there are better alternatives.

  • You might clarify that it's not just the changes he's proposing that hides dependencies, but the original as well. – Samuel Oct 24 '17 at 22:43
  • 1
    @Samuel better? – candied_orange Oct 24 '17 at 22:44
  • @CandiedOrange What would you suggest then ? Storing a pointer to all of these systems into entities? – Pins Oct 25 '17 at 6:45
  • 1
    @Pins only the ones used by the class in question – Caleth Oct 25 '17 at 6:58
  • @Pins reference passing, AKA dependency injection, especially simple constructor injection can make your dependencies explicit. It's more work. But it's worth it in the long run. I'm not saying parameter objects are wrong. But if you use one parameter object for everything that's almost as bad as a service locator. Each one should have a clear focus. – candied_orange Oct 25 '17 at 13:08
0

If you really want to go down the singleton route over dependency injection, which I strongly recommend erring against doing just as much as CandiedOrange, then your second solution is not much better in my opinion.

If you are actually going to have persistent, globally accessible objects in your system that are lazy initialized and open Pandora's box, you might as well exploit that idea. I like the solution of simply having, at this point:

play_sound("Explosion");

... cute little code, like the days of turbo C that's easy for beginners to comprehend with minimalist code snippets for tiny applications without understanding layers of abstractions.... just as we can start a console application and output to the global cout immediately without constructing a console object ourselves and having to pass it around as a parameter.

If you are going to have globally accessible, persistent states, you might as well use it to create this cutesy API you can invoke anywhere. There is at least a cute benefit there to being able to just start up a new project and immediately invoke functions like outputting to a console, playing a sound, or drawing a line to the screen without initializing, acquiring, and passing around resources. That can all be done with a very simplistic procedural syntax and not like Drawer::get().draw_line(...), simply draw_line(...).

Why do I care about the definition of a System in this context or even the full definition of an AudioManager and have compile-time dependencies to both? I want to play a sound, that's it. The implementation of play_sound could still retrieve the singleton instance for your System and retrieve the AudioManager from it and invoke the play_sound method on it, but that can be an implementation detail of play_sound. If I'm not concerned with specific instances of a state, which I won't be if there's supposed to be persistent global state instantiated once, then I'm only concerned with what to do, not who provides it. I just want to call play_sound and have a sound play. I don't care who or what provides the functionality if I'm not required to have this info.

void play_sound(const string& sound)
{
    System::getAudioManager()->playSound(sound);
}

When I see devs go happy crazy with singletons and Singleton::get().do_something() calls all over the place, the first thing that comes to mind is why not just do_something() and make the retrieval of the singleton an implementation detail and hide the singleton and its type definition from the client? I can't see any negative consequences to doing this that aren't already introduced by the singleton itself.

Does it have some overhead (performance) or cons?

Those calls to get() a singleton can be relatively a little bit expensive if you are doing them en masse in a loop that only does trivial low-level things with the singleton object. That gets cheaper if you forego lazy construction and initialize everything in a way such that it's guaranteed to be constructed already by the time the client attempts to retrieve it. Also making the singleton get() method thread-safe if you lazy construct is not entirely trivial (ex: double check locking). It's easier to just avoid the lazy construction than trying to make get() thread safe when it lazy constructs, which will also yield a more efficient solution that makes singleton retrieval very cheap when the get() method no longer has to deal with lazy initialization.

The best SE solution is still not to use singletons at all, but if performance is a concern and you insist on sticking with them, it at least helps to forego the lazy initialization and initialize in advance prior to calls to get().

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.