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

#21 rip-off   Moderators   -  Reputation: 8070

Like
1Likes
Like

Posted 10 June 2012 - 12:23 PM

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 my experience, once an object becomes globally accessible, the probability of "unnecessary" accesses increases enormously. So, no, I think in practise one finds that your average global will be accessed by considerably more areas of the code than the same state would be accessed in a less coupled design.

Also, once there is at least one globally accessible object, the probability of other (arguably far less worthy - it is easy to argue for a global texture cache) candidates being "promoted" to globally accessible increases too.

Sponsor:

#22 Sir Timothy   Members   -  Reputation: 163

Like
0Likes
Like

Posted 10 June 2012 - 12:58 PM


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 my experience, once an object becomes globally accessible, the probability of "unnecessary" accesses increases enormously. So, no, I think in practise one finds that your average global will be accessed by considerably more areas of the code than the same state would be accessed in a less coupled design.

Also, once there is at least one globally accessible object, the probability of other (arguably far less worthy - it is easy to argue for a global texture cache) candidates being "promoted" to globally accessible increases too.


True enough, it is important to practice some self control to avoid overusing them. But just because there's a risk of over-/misuse is no reason to completely discard something.

I refer you to this lovely piece I wrote some time ago. I'm actually quite proud of it (especially the second version, near the bottom of the page). In it, just about everything I've used has been misused. But it's no reason to condemn any of them (can't say for loops are bad just because it's possible to put the entire loop body inside the conditional and/or increment sections); only those particular cases. But, that's getting a bit off-topic I think.

#23 Aardvajk   Crossbones+   -  Reputation: 5857

Like
0Likes
Like

Posted 11 June 2012 - 06:39 AM

<br />I refer you to this lovely piece I wrote some time ago.<br />


I don't understand your point here. That code is over-complicated, difficult to understand and hard to maintain and can be implemented far more simply at no significant cost. Many of the arguments against singletons would apply equally to that code.

There are many reasons to condemn the majority of your code I'm afraid. "It works" is about the only thing that can be claimed in its favour, which is subject to testing to confirm.

#24 larspensjo   Members   -  Reputation: 1526

Like
0Likes
Like

Posted 11 June 2012 - 06:52 AM

I refer you to this lovely piece I wrote some time ago. I'm actually quite proud of it (especially the second version, near the bottom of the page). In it, just about everything I've used has been misused.


That's a nice one! I like the way you use the commutative property of the indexing operator..

I once created an encryption program. It used C as input, and produced compilable C as output. If you change all identifiers and expand most local header files, the source code can become really hard to understand.
Current project: Ephenation.
Sharing OpenGL experiences: http://ephenationopengl.blogspot.com/

#25 Cornstalks   Crossbones+   -  Reputation: 6974

Like
2Likes
Like

Posted 11 June 2012 - 07:54 AM

<br />I refer you to this lovely piece I wrote some time ago.<br />


I don't understand your point here. That code is over-complicated, difficult to understand and hard to maintain and can be implemented far more simply at no significant cost. Many of the arguments against singletons would apply equally to that code.

There are many reasons to condemn the majority of your code I'm afraid. "It works" is about the only thing that can be claimed in its favour, which is subject to testing to confirm.

I think the key comes from the next sentence: "In it, just about everything I've used has been misused." I think the code is more satyrical in nature, showing you all the wrong ways to use otherwise useful aspects of C. Which would make sense in the context of his earlier post, where he's neither really for nor against singletons, but mentions it's easy to abuse them (i.e. I think his point his: do you condemn the pattern or do you condemn the use of the pattern?).

I won't repeat my personal opinions (which I expressed here), as this topic was beat to death on these forums within the last 3 months. I will say, however, that I have never seen the singleton used properly. Ever. It's always used as a "OOP global variable" (I mean, really people?), or as an absolutely unnecessary restraint on some aspect of the program that's also inappropriately given static duration. People just need to stop using it so they stop using it wrong.
[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

#26 rip-off   Moderators   -  Reputation: 8070

Like
0Likes
Like

Posted 11 June 2012 - 08:09 AM

I don't understand your point here...

I believe Sir Timothy's point is just because something can be abused, doesn't make it inherently bad. There is some merit to this argument, but I think that there are more shades of grey here. Goto, we all can agree, has rare utility in modern languages - but on those rare occasions it might be the most elegant and maintainable solution.

While there might be similarly rare cases where a singleton is an acceptable solution, I believe in general it is not. Advanced users will (hopefully) have the sense to weigh the engineering trade-offs against one another and make this decision. However, without a specific scenario to discuss, or if we are in For Beginners, I believe that we can avert harm by saying that singletons are evil.

True enough, it is important to practice some self control to avoid overusing them. But just because there's a risk of over-/misuse is no reason to completely discard something.


Arguing that "there is a place for every little thing", in For Beginners, is distracting and can easily send the beginner down the wrong road.

I wouldn't be a fanatic about "no globals". I have in the past used a global for implementing resource caching before. I see caching of resources as an optimisation, not something that necessarily needs to bleed into the high level architecture (though I have implemented it this way too). You can DI this global into the content loading areas of the code, bypassing the need to pass it into this layer.

However, I really feel there is no use for the singleton pattern. You write a bunch more code, which could contain a bug, in order to enforce the "singleness" of the class. As opposed to:
template<typename Resource>
class ResourceCache : noncopyable {
    // ...
};

// Later
ResourceCache<Texture> getGlobalTextureCache();

// And finally
ResourceCache<Texture> &getGlobalTextureCache() {
    static ResourceCache<Texture> cache;
    return cache;
}
Simple, effective, unlikely to contain a bug (multi-threading aside). Just add water.

However, even though I would prefer such a design to a singleton, I still would not recommend it to a beginner.

#27 Sir Timothy   Members   -  Reputation: 163

Like
0Likes
Like

Posted 11 June 2012 - 10:12 AM

I don't understand your point here.

Cornstalks and rip-off above are correct, the point was to show that anything is subject to abuse (and quite easily, too), but you don't often hear people say "don't ever use if statements, because it's possible to put more than just comparisons and logic operations in the conditional."

I believe that we can avert harm by saying that singletons are evil.

Okay, I like that FAQ's definition of "evil". It's much better than the people above who say flat out that such-and-such a thing should never be used and there are absolutely no exceptions. There are always going to be exceptions to anything.

In any case, I still stand by my opinion that the OP should use whatever makes the most sense. I'll revise it to emphasize that you should consider alternatives (really, you should probably do this for most design decisions), but if it still looks like the simplest approach to you, do it and see what happens. That's what learning is all about!

#28 Aardvajk   Crossbones+   -  Reputation: 5857

Like
0Likes
Like

Posted 11 June 2012 - 12:03 PM

My apologies. My bad. I see.

#29 yckx   Prime Members   -  Reputation: 1163

Like
0Likes
Like

Posted 12 June 2012 - 07:25 AM

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."


A possible solution to having the event dispatcher be a singleton might be to use an object factory. Pass the dispatcher to the factory during its construction and let it handle event registration of game objects. For event firing my first thought would be maybe to use functors to encapsulate the dependency.



I agree that learning hy doing is often the most effective way to learn what not to do, but the OP did ask about singleton (ab)use, so I don't have an issue with the chorus of "don't do it." True global necessity is extremely rare; typically the object is needed in 2 or 3 disparate parts of code, and passing it in feels messy due to some combination of a failure to appreciate the full benefits of passing in dependencies and a need to refactor/redesign the code. Both of these reasons diminish as experience is gained.

#30 Tachikoma   Members   -  Reputation: 552

Like
0Likes
Like

Posted 12 June 2012 - 10:45 AM

I would be interested to read comments on how some of you mitigate duplicate resources without a global-like resource pool pattern that uses reference counting, and so on.

I suppose laungages with GC capability helps in that respect, and so does auto_ptr/shared_ptr to some extent...
Latest project: Sideways Racing on the iPad

#31 mrbastard   Members   -  Reputation: 1573

Like
0Likes
Like

Posted 12 June 2012 - 11:02 AM

By having multiple references to a single resource, and passing by reference to where the 'other' references are initialised.

It's essentially the same for smart or raw pointers (though you should prefer const references whenever possible) except that raw pointers are best passed by value. Smart pointers depend on the implementation in question, but the general guideline of preferring to pass by const ref holds.

TLDR: const ref is win.


#32 rip-off   Moderators   -  Reputation: 8070

Like
0Likes
Like

Posted 12 June 2012 - 05:03 PM

I would be interested to read comments on how some of you mitigate duplicate resources without a global-like resource pool pattern that uses reference counting, and so on.

Load all objects through a common object. This object does not need to be global (i.e. reachable from any point in the code). You might not even need to enforce singularity.

For example, one game I wrote used a per-level "reference stealing" resource cache. Basically, when the first level was loaded, it populates its level-specific cache directly from the disk when a cache miss occurred. When the next level is loaded, a reference to the previous cache was (temporarily) added to the new one. During content loading, resources that caused a cache miss in the new cache were loaded from the older level's cache, and encached in the new one. When level loading was complete, the old cache reference was then removed and disposed of. Any subsequent cache misses would hit the disk (though there should be none during gameplay).

This meant that resources that were common between levels were preserved and copied forward, while level specific resources were disposed of promptly. No globals and no singletons in sight. In fact this approach could not work without the possibility of multiple simultaneous resource caches.

The main downside is that there is a period of time where you have two level's resources in memory at once, but this was a PC game so I was not overly concerned about minimising memory.

#33 Promit   Moderators   -  Reputation: 6350

Like
4Likes
Like

Posted 12 June 2012 - 05:12 PM

My current working theory has survived for several years now. The theory is:
There exists no legitimate use for the singleton pattern, anywhere.
If you want to use a global, fine, but don't use a singleton for it. All you're doing in that case is taking a global and slathering it in stupidity.

Edited by Promit, 12 June 2012 - 05:12 PM.


#34 Sir Timothy   Members   -  Reputation: 163

Like
0Likes
Like

Posted 12 June 2012 - 05:52 PM

I've been working on a Content Manager (Cache, if you don't like the term "manager") class, for loading in and caching various content files. ContentLoader subclasses can be registered via a template function, to load certain types of file. The manager itself is not a singleton, and not global (may make it globally available some time in the future, but not at the moment), however the way I'm dealing with the loader registrations requires that they be kept in a singleton-esque structure. They aren't really kept in a single structure, but through static fields in a couple of templated structs. So I guess it's not technically following the "singleton pattern", but it does follow the concept of global access to a singular collection of data.

The registration records are only used internally by the CM, and exist until they are no longer needed (I use reference counting along with a static, again singleton-esque, collection of records to ensure they stay alive until all CM objects are deleted and the program explicitly clears the static collection). The overall benefit is that the code to register loaders and to load resources is quite nice, without any extra setup on content types, content loaders, etc:
[source lang="cpp"]// ImageLoader is the default loader for Textures.ContentManager::RegisterLoader&lt;Texture, ImageLoader>();// DDSLoader will be used to load "*.dds" files.ContentManager::RegisterLoader&lt;Texture, DDSLoader>("dds");// This will use ImageLoaderTexture *tex1 = myContentManager->Load&lt;Texture>("an_image.png");// This will use DDSLoaderTexture *tex2 = myContentManager->Load&lt;Texture>("another_image.dds");[/source]
Although not shown, each CM keeps a cache of all loaded resources, so that loading the same resource multiple times only does the work once (if a file is loaded once as BinaryData and once as Texture, then it's loaded twice, but that's because it requires different work).

To get the appropriate loader for a resource, there is no runtime lookup into a std::map or anything, I just create a templated struct on the stack based on the resource type (Texture), and that struct gives me access to an existing loader for that type (creates a new one if it doesn't exist yet). As I mentioned, the records are reference-counted, so the loader will be properly freed when it's no longer needed.

A single instance of each loader is used, instead of instantiating a new one for every load operation. If I want to make it multi-threaded, I may add support for creating multiple loaders of a single type, or just make sure the loaders don't maintain internal state.

I imagine I could have written the loader registrations in such a way that each CM could keep its own separate list of them, but it probably would require a bit more work, and really... why? It's not likely that two different CMs are going to need totally different loaders for each resource type, and would require setting the loaders individually for each CM.

I think that having these loader records in a singleton-esque set-up is a small price to pay for ease of use.

#35 flodihn   Members   -  Reputation: 238

Like
2Likes
Like

Posted 13 June 2012 - 06:01 AM

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


I see your point, but the issue is if you need to access one thing all over the place, your design is suboptimal.
For example, in my client, I only have two classes that are accessing the texture manager, that is my OgreView and OgreObject classes. My point is if you give parts of your program certain responsibilities and does not leak those responsibilities outside, you will likely not need to pass around for example a texture manager all over the place.

Edited by flodihn, 13 June 2012 - 06:02 AM.


#36 y2kiah   Members   -  Reputation: 937

Like
1Likes
Like

Posted 13 June 2012 - 12:21 PM

I prefer to use a factory-like pattern to avoid having to use *Manager classes throughout the code base. In reality implementation relies on CRTP but basically it's objects that exist to create other objects.
For example, if I'm loading in a Material object from disk, I need to grab and store some shared pointers to textures that are referenced in this material. Why the **** should I have to know about how to use a resource caching system to request my texture. Why would I have to make any hard-coded assumptions about which cache that texture might be in? In fact, I don't care at all about some manager or cache that might exist to handle assets, I just want the texture! Not to mention the need to #include and link your managers all over the place grows your compile times.

Here's the point - using the factory pattern you can effectively DECOUPLE the code that uses your assets from the code that manages the cache and prevents duplicates, etc. If you don't use this or a similar pattern, you are tightly coupling your code to the existing texture caching system, so you have not achieved encapsulation and you compromise your code in many many places due to external dependencies. If you are spreading the usage of high level *Manager classes throughout your code, then it doesn't matter whether you use Singleton or dependency injection or public statics, or plain old globals, they are all the same thing so you might as well just use Singleton and get it over with. Any way you want to justify one method over another, you are still tightly coupled to that interface for that manager.

I suggest stepping away from the notion of master ownership of assets. The bottom line is, the lifetime of an asset should be controlled by a reference count and nothing else - use shared_ptr (and weak_ptr where appropriate). A cache does NOT control the lifetime of an asset directly, however a cache will allow the asset to be released from memory IF the reference count is 1 and it falls out of the LRU list (meaning only the cache itself is holding onto a reference). LRU is only one way to do it, and probably a naive way in most cases, but the point is that if you have a need for that asset somewhere in your frame, it WILL be in memory. If you ever run out of memory or you get disk thrashing, you need to scale back your detail level.

Edited by y2kiah, 13 June 2012 - 12:23 PM.


#37 Holland   Members   -  Reputation: 155

Like
0Likes
Like

Posted 13 June 2012 - 08:49 PM

I use singletons in my game. Not many, but I use them. DebugLogger is a singleton class I have. Every method is protected against multiple threads, and it CAN slow down my performance. Haven't seen it yet, but I know the chance is there. This is only available in debug and debug/release builds of the game. In release, it's not active and doesn't do anything.

It's scenarios like this (and I honestly can't think of another scenario) that are okay to be a singleton. EVERYTHING will need to log something, and you don't want to pass around a pointer/reference to the logger to everything.

Now, what I do with things I know I only need 1 of, but I don't want to make a singleton?

They get held on to by some other object. My RenderSystem class (hands loading and storing textures, animations, models, etc.) is what holds on to the Renderer class. Renderer is something I should only have 1 of...
[source lang="cpp"]public abstract class AllocateOne{ private static int mAllocations = 0; public AllocateOne() { if (++mAllocations > 1) { // assert, exception, log, something to let you know... } } public ~AllocateOne() { --mAllocations; }}[/source]
I use something like this, and inherit whatever class from it. And, again, I'd only do something like this in debug or debug/release....the final build of your game shouldn't need this type of protection because it should just work. Posted Image

#38 joew   Crossbones+   -  Reputation: 3635

Like
2Likes
Like

Posted 14 June 2012 - 09:09 AM

I'm not sure why you would make a logger a singleton when using C++ quite honestly. If you did then either you'd be restricted from making a second log file if you wanted to, or if you add that functionality into the singleton you're just adding onto the responsibilities by throwing more management into it.

Also if using C++ and only a single log file I'm not sure why you wouldn't use std::clog() either on it's own or using some small object on the stack that sets the output stream (if using multiple logging locations / types). A singleton is actually a step back from doing something simple like this.

#39 Waterlimon   Crossbones+   -  Reputation: 2442

Like
0Likes
Like

Posted 14 June 2012 - 09:18 AM

I would design everything in a way which allows you to have multiple of them so that theyre not connected in any way (maybe not if its some values that need to be calculated that are always the same)

o3o


#40 Cornstalks   Crossbones+   -  Reputation: 6974

Like
0Likes
Like

Posted 14 June 2012 - 09:20 AM

I use singletons in my game. Not many, but I use them. DebugLogger is a singleton class I have. Every method is protected against multiple threads, and it CAN slow down my performance. Haven't seen it yet, but I know the chance is there. This is only available in debug and debug/release builds of the game. In release, it's not active and doesn't do anything.

It's scenarios like this (and I honestly can't think of another scenario) that are okay to be a singleton. EVERYTHING will need to log something, and you don't want to pass around a pointer/reference to the logger to everything.

Look at rip-off's post. There is *no* need for it to be a singleton. Go ahead, use a global variable. But don't use a singleton. Additionally, you can just use functions, and nothing will need to touch the logger.

// Thread safe function that logs to a global logger
logThis(const std::string& msg);

void someFunction()
{
    logThis("hello world");
}

void anotherFunction()
{
    logThis("like how I don't even need to touch a Logger instance?");
    logThis("logThis() is a threadsafe function that logs to a global Logger instance");
    logThis("but it's nice, because all that is abstracted away. I don't care how logThis() is implemented. I just have a message to log.");
}

void thisFunctionLogsToACustomLogger()
{
    Logger logger("customLog.txt"); // open a new log, don't use the global one (this can be useful in certain situations)
    logger.log("and look! I'm not abusively restricting myself to having only one logger! I can use"
               "another Logger instance if I need, or I can use the global log if I need. Yay!");
}

Edited by Cornstalks, 14 June 2012 - 09:23 AM.

[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]




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