Good OO design - The Singleton Design Pattern

Question

All of a sudden I'm having a bit of an OO crisis. Over the last couple of years I've made quite good use of Singleton objects. I used them in many places.

For example, in designing an MVC Java application, I'd create a Singleton 'SystemRegistry' class to store the model and view classes (I've only worked on simple apps and the need for multiple views never came up).

When I create my model and view objects (which weren't singletons, just normal objects), I'd do something like:

SystemRegistry.getInstance().setModel(model);

In my controller classes (which were pretty much event handlers for different GUI items), I'd get access to the view or model as follows:

SystemRegistry.getInstance().getView();

I would never use the SystemRegistry class in the model portion of my app but would, at times, use it in my view to access (but rarely, if ever, to modify) information from the model.

From what I've read (notably Steve Yegge's article), this seems like a poor way to design my application. Any ideas as to better ways of structuring my code.

Also, another aspect of how I design classes, which may, or may not be related to Singletons, is the use of 'Manager-type' classes. An example is a (very simple) OpenGL-based game engine I created in C++.

The main class was GameEngine. It was the over-arcing class that stored a bunch of Managers and handled the main loop and what not. Some of the Managers stored in this class were things like: ObjectManager, RenderingManager, LightingManager, EventManager (includes input), HUDManager, FrameRateManager, WindowManager, etc. There were probably a few more as well.

Basically these classes handled the different aspects of the game engine. The names are pretty straightforward so you should be able to get a good idea of how they're used.

Now, this was meant to be a reusable base that I could use in different projects with the need to change it ideally.

In each new game, I would create an instance of the GameEngine as a class-wide variable (most of the game logic was stored in a single class) and set up the different managers (for example, loading the window co-ordinates or lighting details from a file, setting the FPS, etc). To register an object in the ObjectManager I would do something like:

Player player = new Player();
gameEngine.getObjectManager().addObject(player);

This object will now be stored in a vector in the ObjectManager class and will be drawn when the GameEngine calls the ObjectManager drawObjects() method in each frame.

I might've gotten a bit paranoid now after that article on Singletons (and might not have had enough time to wrap my head around it), but I'm starting to second guess and wonder if the way I designed my GameEngine was proper (for lack of a better word) and didn't just fall into the same pitfalls shared by the Singleton pattern.

Any comments on my post would be much appreciated.

Edit: Thanks for the answers. I appreciate them greatly. If possible, I'd love if someone could give me some hints regarding the two project scenarios posted above. How could I have avoided the use of Singletons / Managers?

With the first one, would DI have been the correct response? Should I have even given the view access to the model (this is probably more of an MVC response)? Would the view benefit from implementing an interface (so that multiple different views can be plugged in)?

In the second case, how else could one have structured the application? Is the gripe simply the use of Manager classes as opposed to more specific names? Or is it that, in some cases, the classes can be further broken-down (e.g. ObjectHolder, ObjectDrawer, ObjectUpdater)?

Answer 1

Well, why did you write singletons? If you understand what went wrong in your singletonitis, you know what to watch out for in the future.

Why did you believe for a second that singletons were a good solution to any kind of problem? Because a book said so? Don't blindly trust books. Books need to justify their claims just like anyone else. If I tell you your code looks much better if you turn your monitor upside down, the burden of proof is on me. Even if I'm some kind of code god. Even if millions of developers across the world worship me daily, my advice is still worth nothing if I can't justify it, and if I can't make you go "ok, that makes sense. I understand why you recommend this, and I can't think of a better way to do it".

And the same is true for a certain design patterns book. So they wrote that "design patterns are awesome", and that "the singleton is a design pattern, and therefore it, too, is awesome". So what? Can they justify this claim (no they can't, at least not in the singleton case), so ignore it.

If someone else suggested that you use a singleton, the logic is the same: did these people actually present a good argument for why it was a good idea?

And if you came up with a justification yourself, can you, today, see what was wrong with it? What did you forget to take into account?

The only way to avoid making too many mistakes in the future is to learn from the mistakes you've already made. How did singletons or manager classes slip by your defenses? What should you have been watching out for when you first learned about them? Or for that matter, later on, when you were using them liberally in your code. What warning signs were there that should have made you wonder "are these singletons really the right way to go?" As a programmer, you have to trust your own mind. You have to trust that you'll make sensible decisions. And the only way to do that is to learn from the bad decisions when you make them. Because we all do, all too often. The best we can do is to make sure we won't make the same bad decisions repeatedly.

Regarding manager classes, their problem is that every class should have exactly one responsibility. What is the responsibility of a "manager class"? It.... uh.... manages stuff. If you can't define a concise area of responsibility, then the class is wrong. What exactly needs managing about these objects? What does it mean to manage them? The standard library already provides container classes for storing a group of objects.

Then you need a bit of code with the responsibility for drawing all the objects stored there. But that doesn't have to be the same object as stores the objects.

What else needs managing? Find out what "managing" these objects means, and then you know what classes you need to do the managing. It's most likely not a single monolithic task, but several different kinds of responsibilities, which should be split out into different classes.

Regarding singletons, I won't repeat what I've said so many times before, so here's a link.

One final piece of advice:

Screw OOP. Really. Good code is not synonymous with OOP. Sometimes classes are a nice tool for the job. Sometimes, they just clutter everything up, and bury the simplest pieces of code behind endless layers of abstraction.

Look into other paradigms. If you're working in C++, you need to know about generic programming. The STL and Boost are nice examples of how to exploit generic programming to write code to solve many problems that is both cleaner, better and more efficient than the equivalent OOP code.

And regardless of language, there are many valuable lessons to be learned from functional programming.

And sometimes, plain old procedural programming is just the nice and simple tool you need.

The key to "good design" is not to attempt "good OO design". If you do that, you lock yourself into that one paradigm. You might as well try to find a good way to build a house using a hammer. I'm not saying it's not possible, but there are much better ways to build much better houses if you also allow yourself to use other tools.

As for your Edit:

With the first one, would DI have been the correct response? Should I have even given the view access to the model (this is probably more of an MVC response)? Would the view benefit from implementing an interface (so that multiple different views can be plugged in)?

DI would have been one option for avoiding singletons. But don't forget the old-fashioned low-tech option: simply manually pass a reference to the objects that need to know about your "former" singleton.

Your renderer needs to know about the list of game objects. (Or does it really? Maybe it just has one single method that needs to be given the list of objects. Maybe the renderer class in itself doesn't need it) So the renderer's constructor should be given a reference to that list. That's really all there is to it. When X needs access to Y, you pass it a reference to Y in a constructor of function parameter. The nice thing about this approach, compared to DI is that you make your dependencies painfully explicit. You know that the renderer knows about the list of renderable objects, because you can see the reference being passed to the constructor. And you had to write this dependency out, giving you a great opportunity to stop and ask "is this necessary"? And you have a motivation for eliminating dependencies: it means less typing!

Many of the dependencies you end up with when using singletons or DI are unnecessary. Both of these tools make it nice and easy to propagate dependencies between different modules, and so that's what you do. And then you end up with a design where your renderer knows about keyboard input, and the input handler has to know how to save the game.

Another possible shortcoming of DI is a simple question of technology. Not all languages have good DI libraries available. Some language make it near impossible to write a robust and generic DI library.

In the second case, how else could one have structured the application? Is the gripe simply the use of Manager classes as opposed to more specific names? Or is it that, in some cases, the classes can be further broken-down (e.g. ObjectHolder, ObjectDrawer, ObjectUpdater)?

I think simply renaming them is a good start, yes. Like I said above, what does it mean to "manage" your objects? If I manage those objects, what am I expected to do? The three classes you mention sound like a good division. Of course, when you dig into designing those classes, you may wonder why you even need an ObjectHolder. Isn't that exactly what the standard library container/collection classes do? Do you need a dedicated class for "holding objects", or can you get away with simply using a List<GameObject>?

So I think both your options really come down to the same thing. If you can give the class a more specific name, then you probably don't need to split it out into multiple smaller classes. But if the you can't think of a single name that makes it clear what the class is supposed to do, then it probably needs to be broken down into multiple classes.

Imagine that you put your code away for half a year. When you come back to it, will you have any idea what the purpose of the "ObjectManager" class was? Probably not, but you'll have a pretty good idea what the "ObjectRenderer" is for. It renders objects.

Answer 2

Steve Yegge is right. Google goes even further: they've got a Singleton detector to help root them out.

I think there are a few problems with Singletons:

1. Harder to test.
2. Stateful Singletons have to be careful with thread safety; easy to become a bottleneck.

But sometimes you really do have to have just one. I'd just not get carried away with Small Boy With A Pattern Syndrome.