Jump to content

  • Log In with Google      Sign In   
  • Create Account

Singleton pattern abuse


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
45 replies to this topic

#1 neotms   Members   -  Reputation: 124

Like
0Likes
Like

Posted 08 June 2012 - 09:50 AM

Alright so I started coding as a hobby a few years ago and im slowly learning new things and finding out what is good practice and what not, but recently after reading some articles about the singleton I've become confused. I'd like to discuss and get advice on the singleton pattern use. I know it's a very discussed subject on the internet and my questions may already be answered on some other post but I haven't managed to find a good place answering my questions.

I have a project I'm working on which is constantly growing and becoming more structured and complicated. At some point I read using globals is bad (try reading that in professor Mackey's voice from South Park, mkay? Posted Image). And at that point I was using many globals. So I searched for an alternative and some people were suggesting the singleton pattern. I was excited with the idea and replaced my globals with singleton classes (as necessary, some globals were probably removed Posted Image) and by the time I was done I had a few singleton classes.

Now I'm reading around some articles saying the singleton pattern is bad. I understand their arguments and realised that some of my singleton classes were an overkill so I changed them and I'm still in the process of rechecking my files and seeing if I can remove more singletons (because I honestly have too many atm). But I just can't get rid of some of them.

Is the singleton pattern such a curse that if I find I have to use it there is something wrong with my class structure? The problem is that in a multiple file project some data will need to be available in more than one place. So how should I achieve this state of "global" access? (Not entirely global, but more than local anyway)

Here is an example for those interested. My TextureManager class is a singleton. It's mostly used in an object loading function to find a texture if loaded in memory or load it otherwise. How can I access my texture manager in the object's load function if it's not a singleton? Pass it as an argument? Keep a pointer to it as a member of the class? None seem very efficient to me. Even if passing it as a reference isn't slowing performance, it makes functions more complicated than they need to be, since the texture manager reference would have to be passed down many functions to reach the load function.

Sponsor:

#2 Serapth   Crossbones+   -  Reputation: 5755

Like
1Likes
Like

Posted 08 June 2012 - 10:18 AM

There are lots of post on this subject.


I do not mean to pick on you, but I wonder if some people don't know a few things about google.

When you go to google, you can specify a site to constrain your results to. To search just GameDev.net, just add site:gamedev.net to your query string.

So, you can search for singleton information on Gamedev using the string "site:gamedev.net singleton bad". You can then narrow down the search results by date range if the results are not relevant.



To actually answer your question, there are a few problems with Singletons.

First, they don't scale for #$@#$, so if you end up multithreading your code, your singleton is going to quickly become a locking bottleneck, or worse, you don't lock, in which case you are going to have some nasty bugs.

Second, they are generally abused, in the end, a singleton is mostly just a global in a pretty dress. They suck for most of the reasons a global sucks.

There are times to use globals, and thus there are times to use a singleton; but they aren't nearly as common as people think they are.

#3 neotms   Members   -  Reputation: 124

Like
0Likes
Like

Posted 08 June 2012 - 10:55 AM

I've used the search function (though I have to admit I didn't use google that way, thanks for the tip :P) and I've mostly found posts explaining why they are bad and have to be avoided. I'm more interested in hearing alternatives that replace what a singleton provides, specifically about how singletons can be used to refer to the same data globally.

Perhaps I didn't stress that enough, I'm sorry.

I understand the sollutions may be specific to the code so a generic answer can't be provided, but how else could something like that be achieved? I've provided an example of a singleton class I can't see how I could replace. Do you (or others, for that matter) think this is a case where a singleton is necessary or am I not thinking outside the box?

#4 Washu   Senior Moderators   -  Reputation: 5423

Like
5Likes
Like

Posted 08 June 2012 - 11:01 AM

I've used the search function (though I have to admit I didn't use google that way, thanks for the tip :P) and I've mostly found posts explaining why they are bad and have to be avoided. I'm more interested in hearing alternatives that replace what a singleton provides, specifically about how singletons can be used to refer to the same data globally.

The general answer is: You don't need to refer to something globally. Usually doing so is a case of a design gone bad, A code smell.

Perhaps I didn't stress that enough, I'm sorry.

I understand the sollutions may be specific to the code so a generic answer can't be provided, but how else could something like that be achieved? I've provided an example of a singleton class I can't see how I could replace. Do you (or others, for that matter) think this is a case where a singleton is necessary or am I not thinking outside the box?

Not really, most of the use cases for singletons revolve around: "I want to access X, but its not 'OO' to use a global, so I'll dress it up in a class and call it 'OO'!".

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.
ScapeCode - Blog | SlimDX


#5 mrbastard   Members   -  Reputation: 1573

Like
1Likes
Like

Posted 08 June 2012 - 12:27 PM

With respect, you haven't given an example of a singleton you can't see how you could replace - you've given an example of a singleton you can see a couple of ways to replace. You just don't like them! Posted Image

How can I access my texture manager in the object's load function if it's not a singleton? Pass it as an argument? Keep a pointer to it as a member of the class? None seem very efficient to me. Even if passing it as a reference isn't slowing performance, it makes functions more complicated than they need to be, since the texture manager reference would have to be passed down many functions to reach the load function.

Don't worry about performance here: if your resource loader ever sees any heavy duty use you're going to want to it to be thread-safe, and it'll be more work to lock your singleton instance for every possible eventuality than it is to pass around a few references.

Likewise, you may have the wrong instinct on complexity here - it's much much easier to reason about chains of functions passing references than devil-may-care global accesses everywhere. You've probably already seen some of this while removing the other singletons you talked about, but (at least in my experience) it only gets worse as your codebase gets bigger. Complexity in terms of typing/reading is just par for the course - complexity in understanding side effects is what you should really be aiming to reduce.

It's hard to give practical advice without seeing your code - maybe be more specific about where your loader func is used, where else uses the texture manager, and what you envisage should own the texture manager.


#6 L. Spiro   Crossbones+   -  Reputation: 14258

Like
-1Likes
Like

Posted 08 June 2012 - 12:44 PM

Any use of a singleton is abuse.


Is the singleton pattern such a curse that if I find I have to use it there is something wrong with my class structure?

Yes. There are no exceptions to this rule.


The problem is that in a multiple file project some data will need to be available in more than one place.

No, the problem is that you think a singleton is the answer to this “problem”.
Design better.


Here is an example for those interested. …

Pass a structure down that contains what will be necessary for loading a texture, including any managers that may be involved.
You are passing a single parameter down barely a few functions deep. Talking about passing things down “many” functions is non-sense. It simply means you have never done it.
Once you start doing it you will realize how shallow most of that passing can be with, again, proper design.


L. Spiro
It is amazing how often people try to be unique, and yet they are always trying to make others be like them. - L. Spiro 2011
I spent most of my life learning the courage it takes to go out and get what I want. Now that I have it, I am not sure exactly what it is that I want. - L. Spiro 2013
I went to my local Subway once to find some guy yelling at the staff. When someone finally came to take my order and asked, “May I help you?”, I replied, “Yeah, I’ll have one asshole to go.”
L. Spiro Engine: http://lspiroengine.com
L. Spiro Engine Forums: http://lspiroengine.com/forums

#7 Narf the Mouse   Members   -  Reputation: 318

Like
1Likes
Like

Posted 08 June 2012 - 09:15 PM

Ok, so where is your texture manager going to be needed? Probably where you load content into your game and...That's it.

Moreover, your texture manager can be abstracted to a content manager. Same place.

If BobMob needs to load BobTexture - BobMob shouldn't load BobTexture. It should request BobTexture from the content manager.

Even more OO, BobMob shouldn't even know it needs BobTexture - BobMob is the model, not the view or the controller.

BobMob is also trying to do too much. BobModel, BobView, BobController. Bobs' personal data doesn't need to know how it's drawn. Bobs' drawing code doesn't need to know how Bob is controlled. And BobController doesn't need to know anything about Bobs' personal data, only the results of queries on that data.

(That's my admittadly beginners' perspective on Model-View-Controller. Someone else can probably put it a lot better)

And all of those should be churned out by a Factory class which can be replaced with another in an instant.

All of this is the ideal, as I understand it from personal experience and reading. :) Programming constraints aren't always ideal. :)

#8 ButchDean   Members   -  Reputation: 110

Like
-2Likes
Like

Posted 08 June 2012 - 11:29 PM

I actually am pro singleton for the following reasons:

1. It is a generally recognized useful design pattern that is easy to grasp.

2. It makes a single instance of an object easily accessible.

3. It is not the same as a global because it has global scope. A well written singleton can have very controlled access even though it can be referenced globally.

4. If a singleton wrecks havoc in your code, blame the programmer and not the tool.

5. Singletons make the game states that they represent easy to debug.

These are just five points that come to mind.

#9 Narf the Mouse   Members   -  Reputation: 318

Like
3Likes
Like

Posted 09 June 2012 - 12:19 AM

I actually am pro singleton for the following reasons:

1. It is a generally recognized useful design pattern that is easy to grasp.

2. It makes a single instance of an object easily accessible.

3. It is not the same as a global because it has global scope. A well written singleton can have very controlled access even though it can be referenced globally.

4. If a singleton wrecks havoc in your code, blame the programmer and not the tool.

5. Singletons make the game states that they represent easy to debug.

These are just five points that come to mind.


1) So is coding everything in Main( ) - Until you need to change something.

2) If you have an easily-accessible hammer...

3) And yet, it can still be accessed anywhere in your code. That's a downside, because now your code is locked to the global.

4) Yes. Blame the programmer who used them.

5) Here's a concept for you: One point of access failure. Local variables can only fail in the scope they're in. Globals and Singletons have multiple point-of-access failure, A singleton or global can fail *Anywhere it's been called*, yet if the error only appears *in the accessor*, good luck finding the glitch.

Here's a free #6: They're terrible for refactoring. Imagine a well-built machine. Everything works perfectly and smoothly. Except, *everything* connects to this black box.

Starting to see the problem?

I've used Singletons. I've hated it every single time and always found a better way, at some point. So, the reason to use Singletons?

1) You don't know how to not use Singletons in that context, yet, and the code needs to work.

#10 SimonForsman   Crossbones+   -  Reputation: 6305

Like
2Likes
Like

Posted 09 June 2012 - 03:53 AM

a singleton is a global with the additional restriction that there can never be more than one instance, you should only use them when you need a global instance and there would be an error if two were created. (This is insanely rare in application/game code).

Globals aren't automatically bad, globally mutable state however is usually a bad idea and wrapping your globals in a singleton class does nothing to change this, it only makes your code less flexible. (a singleton is still a global).

If you need global access to an instance you should make it an immutable global, not a singleton. If you need globally mutable state then you should rethink your design. (Allthough it is often far more important to get the job done than it is to get the design perfect).
I don't suffer from insanity, I'm enjoying every minute of it.
The voices in my head may not be real, but they have some good ideas!

#11 neotms   Members   -  Reputation: 124

Like
0Likes
Like

Posted 09 June 2012 - 04:45 AM

I'd like to thank everyone for their time, I've realised from answers in the post that the way to get rid of singletons is review my design and get rid of the dependencies even if I don't like the idea of it. I'll start working on separating rendering functions from my objects when I get the time. :P

Bobs' personal data doesn't need to know how it's drawn.


But a map (talking about 2D) which is rendered tile by tile is rendered differently from, say, a checkbox or any simple object that only needs to be rendered with one procedure. Mustn't the Map class know of the logic by which it'll be rendered? (nested loops etc). Otherwise, that information must be hardcoded somewhere else (renderer?).

#12 Narf the Mouse   Members   -  Reputation: 318

Like
2Likes
Like

Posted 09 June 2012 - 05:18 AM

I'd like to thank everyone for their time, I've realised from answers in the post that the way to get rid of singletons is review my design and get rid of the dependencies even if I don't like the idea of it. I'll start working on separating rendering functions from my objects when I get the time. Posted Image


Bobs' personal data doesn't need to know how it's drawn.


But a map (talking about 2D) which is rendered tile by tile is rendered differently from, say, a checkbox or any simple object that only needs to be rendered with one procedure. Mustn't the Map class know of the logic by which it'll be rendered? (nested loops etc). Otherwise, that information must be hardcoded somewhere else (renderer?).

All the map class need know is it has an array[][] of tile data; all the renderer need know is it's received an array[][] of tile data.

And the renderer doesn't need to be hard-coded to draw specific tiles; a factory can fill out a lookup table of some sort.

Classes shouldn't have more than one concern, even if that concern is "Running the game". Even the class that "Runs the game" shouldn't concern itself with, say, whether or not BobMob gets drawn.

Of course, as always, real life is not ideal and sometimes what works still works well enough.

#13 phantom   Moderators   -  Reputation: 7563

Like
4Likes
Like

Posted 09 June 2012 - 05:37 AM

But a map (talking about 2D) which is rendered tile by tile is rendered differently from, say, a checkbox or any simple object that only needs to be rendered with one procedure. Mustn't the Map class know of the logic by which it'll be rendered? (nested loops etc). Otherwise, that information must be hardcoded somewhere else (renderer?).


And you've run smack into a problem of doing too much; what does the 'map' represent in this?

The information doesn't have to be 'hard coded' anywhere but it should be abstracted in some manner. A 'map' shouldn't have to care HOW it is rendered, or even what is rendered, all it needs to care about is the fact it has something which needs to be rendered.

If you step back and think about it for a moment ALL rendering is the same; you provide the GPU with a vertex and index stream, some constants, some render states (including shaders) and some textures. That is is.

So at the lowest level you have a 'renderable work packet' object which bundles this state together and that is all your render cares about; give it these packets of information and it can render you the world.

It doesn't matter if you have a 2D map, a rigged 3D character or a box the basic rendering remains the same.

So then pull back out one more level; you still need something to contain and send that work down to the renderer. This is the point when you start to specialise a bit; for your 2D map example the renderer would have been told 'here is an object which needs to be rendered and will provide you with packets of work'. In this instance the 'renderable' knows how to render a 2D map and can provide the render with work packets to do just that.

Finally you can back up one more level to the 'map' object itself, or the logical game represenation of it; this doesn't know how to render the map at all, all it knows how to do is respond to certain events and other high level map related details. It will more than likely have a reference to the 2DMapRenderable object which will do the rendering work but it has no direct means of communicating with the renderer. It could talk to the map renderable depending on the game; for example if you have a game which has a map consisting of 2D 'screens' which instead of scrolling 'flip' you might well have a function which tells the renderable which screen to send to the renderer when it is next asked.

The renderer backend itself deals with talking to the renderable, including asking it for (culled) work, and doesn't care what the game above it is doing.

Short version; all rendering is the same at the lowest end; high level logic doesn't care about how things are rendered, abstract it away.

#14 Fredericvo   Members   -  Reputation: 463

Like
1Likes
Like

Posted 10 June 2012 - 09:21 AM

If the idea that globals are bad because you need to keep track of so many potential modifications in different functions then I can hardly see how passing a reference to them solves this problem since you can still modify the original value from different places. At some point you'll have states that the entire program needs to keep track of so this jihad seems a bit too dogmatic. Not all programmers have a phd in computer science so these considerations seem to paralyse by over-analysis rather than just let you code away. I'm victim of this.

#15 Narf the Mouse   Members   -  Reputation: 318

Like
0Likes
Like

Posted 10 June 2012 - 09:41 AM

If the idea that globals are bad because you need to keep track of so many potential modifications in different functions then I can hardly see how passing a reference to them solves this problem since you can still modify the original value from different places. At some point you'll have states that the entire program needs to keep track of so this jihad seems a bit too dogmatic. Not all programmers have a phd in computer science so these considerations seem to paralyse by over-analysis rather than just let you code away. I'm victim of this.

If I understand your argument, it's that passing, say, TextureManager everywhere is just as bad as a Singleton, so why not use a Singleton?

1) That sounds like an argument against passing TextureManager everywhere.
2) The thing is, if you want to change what a pass-by-value system uses, you only need to change a few upstream values. If you want to change what a Singleton system uses, you have to re-write it to a pass-by-value system.

#16 Fredericvo   Members   -  Reputation: 463

Like
0Likes
Like

Posted 10 June 2012 - 09:52 AM

No I'm not saying it's better or worse, just that the alternative isn't that obvious to me. Let's say in scenario A you have a global, gState = 1;
then you have somefunc1(); reading/modifying it.
And func2() and func53() and func89 () and you forgot that func15() also modified it. They say "see! Globals are evil"
Now in scenario B you made it local and passed a reference to all of the above functions and maybe you also forgot that func15() changed it.
Then did you really solve the problem?
But you polluted the namespace less I guess?

#17 rip-off   Moderators   -  Reputation: 8726

Like
3Likes
Like

Posted 10 June 2012 - 11:07 AM

At some point you'll have states that the entire program needs to keep track of so this jihad seems a bit too dogmatic.

What possible state would you need to have access from every point in the program? Does the standard library's generic container classes or sort routines access to this state? Does DirectX/OpenGL access to this state? Your audio library, physics library, ...?

No; because well-designed modules can be used independently regardless of the rest of the program. Seek to emulate, where possible, this de-coupled design in the high level architecture of your own program.

I believe this mindset is often the result of "singleton cancer" (which is itself just a hardy mutation of globalitis). Simply by making state accessible globally practically guarantees that accesses to this state will swarm and multiply across the codebase. This infection will stunt the overall design. Module walls, if they managed to form at all, will corrode and collapse under the weight of these virulent pathogens, until putrified spaghetti code is all that remains.

At least, when the project begins to scale. Smaller programs are restistant because of their own simplicity. Without the high level structure necessary to feed on the cancer's growth rate is severely stunted.

Now in scenario B you made it local and passed a reference to all of the above functions and maybe you also forgot that func15() changed it. Then did you really solve the problem?

The problem is coupling. func 1..99 don't actually need to see gState. Maybe only fun1, func13 and func42 do. It isn't about converting the singleton/global into a parameter and keeping the code the same - it is about restructuring the code so that the "global" state is now "local" to a particular sub-system.

For example, by separating the core game logic from the presentation logic, you decouple the core from knowledge of classes such as a "TextureManager". You can separate input polling and mapping from the game logic actions by using a callback system. Thus the input routines are de-coupled from the game logic, connected by injecting the correct functors.

Edited by rip-off, 10 June 2012 - 11:08 AM.


#18 phantom   Moderators   -  Reputation: 7563

Like
3Likes
Like

Posted 10 June 2012 - 11:10 AM

You don't solve the 'forget' problem BUT you do solve hidden dependencies which is the problem with globals.

void func1()
void func2()
void func15()
// vs
void func1(SomeState &)
void func2(SomeState &)
void func15(SomeState &)
// or
void func1(const SomeState &)
void func2(const SomeState &)
void func15(SomeState &)

In the first 3 examples the dependencies are hidden away, there is no way by looking at the code you can say what dependencies those functions have.
The second group make it clear they require an instance of 'SomeState' to do work.
The third group make it clear that 2 of the functions won't modify the state, the third will/can.

The other problem with globals is that they are just that; global.
They are everywhere, there get everywhere and they are had to track depenency wise and, most of the time, they really don't need to be global.

Often people say 'oh, but I need to do X' everwhere but when you get down to it they rarely need to do it 'everywhere' but in a very small select area of code.
People think 'everywhere' because they haven't put the effort into thinking about the problem beyond 'I need to do X'.

Nor is this about 'having a phd in computer science'; it is about sitting down and thinking about your design and thinking beyond the first solution which appears. Yes, a global often seems the easiest to start with, and if you want to run with it then go ahead, but don't be surprised when you run into problems or when people say you've come up with a bad solution.

Globals are, by and large, the lazy solution.
Singletons are, by and large, the wrong solution.

Edited by phantom, 10 June 2012 - 11:31 AM.


#19 mrbastard   Members   -  Reputation: 1573

Like
0Likes
Like

Posted 10 June 2012 - 11:19 AM

I don't think anyone in this thread has a PHD in computer science (though I may be wrong). On the other hand, I'm 100% confident that the more vehement posters have lost months of their lives to singletons that seemed 'convenient' to someone at some point.

+1 phantom's mention of const - another invaluable tool in making it possible to reason about your code.

Edited by mrbastard, 10 June 2012 - 11:24 AM.



#20 Sir Timothy   Members   -  Reputation: 163

Like
1Likes
Like

Posted 10 June 2012 - 12:03 PM

2) The thing is, if you want to change what a pass-by-value system uses, you only need to change a few upstream values. If you want to change what a Singleton system uses, you have to re-write it to a pass-by-value system.


Just a thought... whether you're using a pass-by-argument sort of thing, or a singleton, you're still using that object in the same places. In the example given, the TextureManager is being used in function used for loading stuff. Whether the Mgr is available as a global/singleton or it's passed as an argument (or some other object containing some context data that could be used in lieu of the Mgr), if you make a change to it, you need to account for the change in that function. It really doesn't matter. Putting it in a singleton just makes it easier to call the loading function. If anything, the singleton requires an equal or lesser number of changes, since you don't have to change upstream references, just the actual usage.

There's been mention of not being able to see at a glance what data/objects a function uses, and forgetting about updating references when changes are made... true, you don't get the same immediate indicator that a load function is going to use some sort of content system as you would if you passed it as an argument. Instead you've got, well, the fact that it's a loading function, plus perhaps some documentation for the function that says something like "gets the appropriate texture from the TextureManager". As far as forgetting to update references, I'm sure the IDE can help you with that; just do a search for the class name, it's a singleton, so you probably don't have to worry about it being referenced using some subclass (there may be a subclass, but you're likely using the base class' name).

2) If you have an easily-accessible hammer...

... then when you need a hammer, it's easy. If that hammer is complicated, you're going to start using the butt of a screwdriver or a shoe to pound nails.


Personally, I won't shy away from singletons (or really, anything) when I feel they are appropriate. An example for me, something I've used in pretty much any project: I generally have some sort of Event Dispatcher, which allows listeners to register for certain event types, and then anyone can fire out events as appropriate. Fairly similar in concept to Win32's message stuff. Pretty useful, I'm sure there are other solutions, but this is the one I like to use, and it has evolved over several projects. It means, for example, that an explosion class doesn't need to get a list of all nearby actors and manually call a dealDamage() function or anything. Instead, damageable actors register for the EXPLOSION event, and the explosion fires the event (either the actors check if they're in range, or I could implement some sort of filter to decide which listeners will actually get the event). Sure, a lot of things end up being connected to the EventDispatcher singleton, but that's never been a problem for me. You might say "oh, but that can't be thread safe!" Well, it's no different than getting nearby actors and calling dealDamage() on each of them. And there's no way I want to pass an EventDispatcher around to every single object I create (not every object uses it, but we don't want to lock that sort of assumption into the code now, do we)


My general opinion is that you shouldn't dismiss something just because everyone says it's bad. I feel pretty confident in saying that any construct has some case where it is appropriate. Sure, there are places where a singleton is far from the right tool, but sometimes it's going to be much cleaner than the alternative. I see several people here saying that singletons are pure evil and should never be used under any circumstance. I say use whatever makes sense. If using a global object or a singleton is going to make your code cleaner and simpler than passing extra context objects around, use it! If down the road you find that you're running into problems with it, then you can change it up. Especially since the OP stated outright s/he's doing this as a hobby, and wants to learn. What could be better for learning what works and what doesn't, than trying it out? If you try something and it works through to project completion, great! If you find that at some point it's no longer sufficient, then you'll understand why it doesn't work and can move to a better solution. Sure, it may take a bit of time to refactor, but the experience is, I think, a lot more valuable that someone just telling you "it's bad, never do it."

Of course, I'm not saying that everything can/should be a singleton either, just do what makes sense to you.

And back to the OP's example of the TextureManager... my opinion would be that you could instead have a single ContentManager, which defers requests to the TextureManager, ModelManager, SoundFileManager, WhateverElseManager as appropriate based on resource type; so it's one singleton instead of many. Or instead of separate *Managers, provide separate *Loaders for each resource type, and ContentManager maintains a list (or whatever your favorite container is) of generic Resources (which is subclassed or something for each resource type) provided by those *Loaders.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS