The Lone Rangers

  • Gandor 2012-09-17 08:05
    CodeSOD instead?
  • Mcoder 2012-09-17 08:09
    Why don't languages already come with a singleton modifier? It would remove an entire class of WTFs from the face of Earth (and create another entire and more entretaining class, so we can laugh more).
  • Remy Porter 2012-09-17 08:14
    Apparently the idiot that wrote this article picked the wrong category tag, Gandor.
  • charriu 2012-09-17 08:19
    Scala does that with the "companion object" as it's called, IIRC.
  • My name is unimportant 2012-09-17 08:25
    Well, that's what David gets for trying to use the singleton before creating an instance of the class.
  • snoofle 2012-09-17 08:26
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!
  • David 2012-09-17 08:26
    Well, it's a multigleton... an evolution of the poor old singleton.
  • Erik 2012-09-17 08:39
    TRWTF is the singleton pattern!
  • @Deprecated 2012-09-17 08:46
    snoofle:
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!


    this.





    PS., Although I do not like replies that contain a singleton 'this', I will make an exception in this case.
  • emaNrouY-Here 2012-09-17 08:48
    I see where it went wrong:
    public class KpiService {
    
    private static KpiService instance = null;

    public static KpiService getInstance() {
    return instance;
    }

    public KpiService() {
    try {
    instance = this;
    } catch (Exception ex) {
    }

    }
    /* SNIP */
    }
  • pure 2012-09-17 08:49
    TRWTF is languages which require everything to be a class. Like Math.
  • Andrew 2012-09-17 08:53
    I got the biggest shock seeing this as I've implemented a KPI-type solution in my history and thought this WTF originated from me...
    Until I realized mine was a simple webservice and didn't involve singletons.

    Phew!
  • History Teacher 2012-09-17 08:58
    That's perfectly sensible singleton implementation, which rightfully punishes the coder with brain-training bugs, if he is not already smart enough to use it correctly (or smart enough to get a new job).
  • szeryf 2012-09-17 09:01
    It's wrong because the return statement isn't properly commented. There, I fixed it:


    public static KpiService getInstance() {
    // That's it.
    return instance;
    }
  • Röb 2012-09-17 09:04
    Sooo in the interests of actually learning something what should they have done? Seen and used this structure in PHP before where classes can be instantiated by call_user_func_array() (eg: in WordPress). So rather than setting a global variable for an instance you use a get_instance() method if/where you need to...
  • maybe 2012-09-17 09:06
    Can anyone explain this for laymen?
  • Remy Porter 2012-09-17 09:08
    Rob, in a traditional implementation of the Singleton pattern the constructor should be private and the get_instance method should automatically create an instance if one doesn't already exist.

    And you should avoid using the Singleton pattern. It's one of those patterns that's so easy to implement and seems so useful that you're tempted to create a bunch of them, but they should be used rarely.
  • TL 2012-09-17 09:11
    Scala does.
  • @Deprecated 2012-09-17 09:13
    maybe:
    Can anyone explain this for laymen?


    Dear laymen:

    The only way to initialize 'instance' is to call new KpiService(); But, every time you call 'new KpiService();', instance will be rewritten with a new object.

    So, the only way to use it would be:
    1. Call new KpiService();
    2. Every time after that, call KpiService.getInstance();

    Sooo... which one do you call?

  • Röb 2012-09-17 09:21
    Cheers Remy

    Just realised the method I've used actually does pretty much what you say it should - get_instance() creates the instance if it doesn't exist and the constructor is just a constructor.

    Eg:


    add_action( 'init', array( 'some_class', 'instance' ) );

    class some_class {
    /**
    * Reusable object instance.
    *
    * @type object
    */
    protected static $instance = null;


    /**
    * Creates a new instance.
    * May be used to access class methods from outside.
    *
    * @see __construct()
    * @return void
    */
    public static function instance() {
    null === self :: $instance AND self :: $instance = new self;
    return self :: $instance;
    }

    public function __construct() {
    // stuff
    }

    }
  • TheSHEEEP 2012-09-17 09:22
    I don't get the bashing SIngletons get in general.

    The usual argument is: It is bad style.
    Uh-huh. Is that so?

    In a game I once made I had a singleton that allowed to play a fire-and-forget sound. It was very useful that I could do so from each place in the code, from where loading was finished to when a weapon was fired (both were obviously in very different parts of the code).

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?

    Of course, a singleton should actually be implemented correctly ;)
    And of course, some things should not be a singleton, as they really shouldn't be usable everywhere. But saying that the pattern itself sucks because you are not capable of using it correctly does really put me off.

    I guess, like with many things, a singleton can be used to do something wrong, and therefore, some seem to think that it is generally bad.
  • AGray 2012-09-17 09:24
    maybe:
    Can anyone explain this for laymen?


    Singleton objects are set up such that they only ever have a single instance in the application. This is useful for things that - as the name suggests - you want only one of, ever.

    Typically, in C# (which is what this WTF looks like), an implementation looks like this:

    public class Singleton
    
    {
    private static Singleton _Instance;
    public static Singleton Instance
    {
    get
    {
    return _Instance
    ?? (_Instance = new Singleton());
    }
    }

    protected Singleton()
    {
    }

    // ...Snip...
    }


    Here's how it works. We protect the Singleton so it can only be accessed internally to the class (or, a derived class if for some reason we want to derive from the Singleton...not sure why that would happen, but it's conceivable.)

    The Instance property's wierd syntax pretty much reads, "If _Instance is initialized, return it, otherwise set _Instance to be a new instance of Singleton." That's what allows only the single point of access to the rest of the app.

    In my own game projects, I typically use singletons for things like Settings, and I'm exploring other uses for myself...

    CAPTCHA: sino
  • Chris P. Peterson 2012-09-17 09:24
    At my work, if I was unable to use this singleton class, I would get the blame for not knowing what I was doing. Anyway, how anyone could butcher up a singleton class that badly is beyond me.
  • VinDuv 2012-09-17 09:29
    Fixed it:
    public class KpiService {
    
    private static KpiService instance = null;

    static {
    new KpiService();
    }


    public static KpiService getInstance() {
    return instance;
    }

    public KpiService() {
    instance = this;
    }
    /* SNIP */
    }


    (The constructor is still public so any disgruntled bombs which randomly create new KpiService instances will still work. Backwards compatibility!)
  • Severity One 2012-09-17 09:31
    TheSHEEEP:
    I don't get the bashing
    In a game I once made I had a singleton that allowed to play a fire-and-forget sound. It was very useful that I could do so from each place in the code, from where loading was finished to when a weapon was fired (both were obviously in very different parts of the code).

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?
    Well, why would you want to play a sound from your disc I/O classes? What you did was invent global variables in an OO-language.

    There are plenty of ways to implement access to sounds, but it really depends on the design of your application.
  • QJo 2012-09-17 09:39
    For the record ...


    public class KpiService {

    private static KpiService instance = null;

    public static KpiService getInstance() {
    if (instance == null) {
    instance = new KpiService();
    }
    return instance;
    }

    private KpiService() {
    /* SNIP */
    }

    /* SNIP */
    }


  • Remy Porter 2012-09-17 09:42
    Yes, you should limit what classes should be able to play a sound. I think OOP does a lot of harm in the way it forces us to live in a world of nouns, but one of the principal benefits of OOP is that it allows for clear separation of concerns. Part of that is making the interactions between modules explicit. This allows the code to be understood from a structural perspective.

    There are a lot of ways to provide sound functionality. One way that would be vastly superior to a Singleton is to use the Observer pattern. A single SoundManager instance registers interest in game objects. When sound-worthy events occur in those objects, the SoundManager matches them up with a sound.

    This approach lends itself to building a declarative system for linking sounds to events, making it easier to modify your game later. It also means that sound can be added to classes without changing the classes themselves. Why should a spaceship know how to make noises? It's a spaceship- noises are nothing more than a side-effect of what it does when it's doing far more interesting things.

    The Singleton is one of those "better hammer" problems. It's a pretty useful hammer, and it's so easy to use that people start to treat every problem like a nail.
  • not_snoofle 2012-09-17 09:50
    This is a great comment. And by this I mean that this, not this this.
  • Confused 2012-09-17 10:02
    Can't tell if you're being sarcastic, but the problem is that with this code you can't access the singleton until some code somewhere magically creates it. A singleton is usually responsible for it's own creation. The code should have been

    public class KpiService {
    private static KpiService instance = null;

    public static KpiService getInstance() {
    return instance ?? (instance = new KipService());
    }

    public KpiService() {
    instance = this;
    }
    /* SNIP */
    }
  • Jeff Dege 2012-09-17 10:08
    Remy Porter:
    A single SoundManager instance registers interest in game objects. When sound-worthy events occur in those objects, the SoundManager matches them up with a sound.

    And how do you enforce that you have only a single SoundManager instance?

    Answer? You make the SoundManager class a singleton.
  • MightyM 2012-09-17 10:09
    @Deprecated:
    Sooo... which one do you call?


    Ghostbusters! Oh, wait...
  • T 2012-09-17 10:10
    TheSHEEEP:

    (...)
    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?


    Obviously you should've used Dependency Injection framework.
    No matter that your game would be slower, larger and harder to maintain.

    But seriously, singletons are quite error prone in enterprise applications (clustering).
    Often they're used as lazy shortcuts.
    Environment can't manage them.
    Switching to another implementation can be painful.

    I see nothing wrong in using them in desktop applications or games.
  • lmm 2012-09-17 10:25
    Jeff Dege:

    And how do you enforce that you have only a single SoundManager instance?

    Answer? You make the SoundManager class a singleton.


    If you can't call getInstance() on it, is it still a singleton?

    I mean, sure there would only be one of them. The point is you make it possible to mock it for testing, your touchpoints with your SoundManager are explicit, and you can replace it with a different one (or even two or many) if you need to.
  • Remy Porter 2012-09-17 10:38
    I still probably wouldn't go Singleton, but that's because I'd want multiple SoundManagers governing different varieties of sound. One SM for effects, one SM for background music, one SM for menu operations, etc.

    This allows them to be compartmentalized in different threads if necessary (the effects SM should run in the game thread, the background SM should run in the global thread, ditto on the menu one).

    The real constraint would be something more along the lines of each game object can be observed only once by a given kind of SoundManager. The effects and music manager can both observe the spaceship, but only one effects manager can observe the spaceship- if another tries to register it either throws an exception or replaces the original effects manager.

    Further, in a situation like this, the GameEngine object should have the freedom to manage its child objects as it sees fit. If the GameEngine decides its a good idea to have one and only one SoundManager, then it can create only one. If it decides there should be more than one, it can make more than one. By making it a Singleton, you've said that the SoundManager knows best, and no one should ever create any more instances of the SM, which likely isn't true.

    Singelton enforces statically a condition that usually should be enforced by convention.

    Ironically, I have a singleton in the application I'm writing. I'm not opposed to the pattern itself, but it's definitely overused.
  • hquilo 2012-09-17 10:45
    Does ' instance' ever get initialized? From the snapshot, it does not. It seems that there is a conceptual misconception with the use of the singleton class: it has a unique member and this member has a name. So I don't see any valid reason to use 'this' in the 'getInstance' method.
  • letatio 2012-09-17 10:52
    QJo:
    For the record ...


    public class KpiService {

    private static KpiService instance = null;

    public static KpiService getInstance() {
    if (instance == null) {
    instance = new KpiService();
    }
    return instance;
    }

    private KpiService() {
    /* SNIP */
    }

    /* SNIP */
    }



    But don't you think about performance? While the program runs every single access to the singleton will execute that unnecessary "if", except the first. Do you think processor cycles grow on trees?
  • David 2012-09-17 10:56
    No, you can't do that.
    The main objective for a singleton is to have ONLY ONE INSTANCE, and additionally that you can reach it easily.
    So you can't let the constructor public, or at least not to change the instance variable (to be sure that every KpiService.getInstance() refers to THE SAME OBJECT, everytime, in any case (if 'fake' constructor had been called or not).
    So you MUST change the instance declaration and instanciation line to this one - AS IN EVERY THREAD-SAFE SINGLETON:

    private static final KpiService instance = new KpiService();

    And you MUST remove the instruction 'instance=this;' in the constructor (which would fail anyway to compile thanks ti the 'final' keyword above).

    And you SHOULD change the visibility of the constructor to private AND correct calling code.

    THAT would fix it.
  • TheSHEEEP 2012-09-17 10:57
    Severity One:
    TheSHEEEP:

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?
    Well, why would you want to play a sound from your disc I/O classes?


    The question is not why I would want to allow something, the question is why would I want to limit something that does no harm?

    Remy Porter:
    Yes, you should limit what classes should be able to play a sound. I think OOP does a lot of harm in the way it forces us to live in a world of nouns, but one of the principal benefits of OOP is that it allows for clear separation of concerns. Part of that is making the interactions between modules explicit. This allows the code to be understood from a structural perspective.


    I am absolutely convinced that a simple SoundManager singleton would be far easier to understand than the complex architecture you suggest. And it would also be much faster, an event system would simply introduce clutter.

    Mind you, I'm not talking about more complex stuff like 3D sound, that would be a pretty bad idea to have as a Singleton as it depends on a whole new layer of stuff, would have to watch objects, etc. And if you already built that 3D sound class, there's no point in building an extra singleton class for 2d sounds. Even if it means you won't be able to play a sound from anywhere.

    Remy Porter:
    There are a lot of ways to provide sound functionality. One way that would be vastly superior to a Singleton is to use the Observer pattern. A single SoundManager instance registers interest in game objects. When sound-worthy events occur in those objects, the SoundManager matches them up with a sound.


    I get the idea, but that would be an overly complex solution to a very easy problem.
    You know, we have a nice saying in our office: "You can solve any problem by adding another layer of indirection. Except too many levels of indirection."
    Some people should really take that to heart. ;)

    Also, people seem to be stuck on that sound example.
    Let's introduce another one: Logging. In the same game (and in all other applications I wrote and most I encountered) I had a Logging singleton class.
    Now Logging is really something you want to do from possibly everywhere without having to rewrite your class hierarchy/structure.

    And the possibility to do so from everywhere also doesn't break or harm anything. Except threaded stuff, but you can always make that Singleton thread-safe.
  • just me 2012-09-17 11:00
    TheSHEEEP:
    I don't get the bashing SIngletons get in general.

    The usual argument is: It is bad style.
    Uh-huh. Is that so?

    In a game I once made I had a singleton that allowed to play a fire-and-forget sound. It was very useful that I could do so from each place in the code, from where loading was finished to when a weapon was fired (both were obviously in very different parts of the code).

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?

    Of course, a singleton should actually be implemented correctly ;)
    And of course, some things should not be a singleton, as they really shouldn't be usable everywhere. But saying that the pattern itself sucks because you are not capable of using it correctly does really put me off.

    I guess, like with many things, a singleton can be used to do something wrong, and therefore, some seem to think that it is generally bad.


    A singleton is just a particular way of implementing a global variable; it has mostly the same benefits and problems as those.
  • G0TDF 2012-09-17 11:03
    Oh good Lord! To implement a Singleton in .NET C# all you need to do is this

    public class Singleton()
    {
    private Singleton()
    {
    // Do initialisation stuff
    }

    public static readonly Singleton Instance = new Singleton();
    }

    There! Simples...
  • Chris 2012-09-17 11:08
    [code]
    public class KpiService {



    private static KpiService instance = new KpiService();;



    public static KpiService getInstance() {
    return instance;
    }



    private KpiService() {

    /* SNIP */

    }



    /* SNIP */

    }
  • Garrison Fiord 2012-09-17 11:09
    Over the years I've read from this site, I've read countless comments criticizing the editors. I've read offensive and possibly-illegal posts by Zunesis. I don't know why I'm suddently being signaled out for harrassment. And yet I keep whining about it and not contributing to threads. It's like I'm the dumbest troll that was ever born.
  • Chris V 2012-09-17 11:13
    The non-static constructor for the singleton should be private, not public.
  • Garrison Fiord 2012-09-17 11:16
    Garrison Fiord:
    Over the years I've read from this site, I've read countless comments criticizing the editors. I've read offensive and possibly-illegal posts by Zunesis. I don't know why I'm suddenly being singled out for harassment.

    Fixed. Yes, now I don't contribute, since my comment is going to be deleted anyway. I'm not going to register so you can spam my email address either.
  • Remy Porter 2012-09-17 11:21
    It's not really a case of indirection. It's a case of making sure objects have one job. There is no reason why a game object should contain code for managing sound. But the Singleton approach essentially requires it.

    No one designs game engines such that their game objects contain rendering code (not good game engines anyway!). The core GameEngine usually contains a Renderer. The GameEngine manages the Renderer object, so there's no reason for the Renderer to enforce being a Singleton. For each frame, the Engine passes the pertinent portions of the object graph to the Renderer, which then Renders it.

    It makes sense to implement the sound engine the same way. Even if you don't implement the Observer pattern (that's only one way to implement it, after all, and probably not the best), the same process used by the Renderer makes sense for the SoundManager. Each tick, the Engine sends the pertinent parts of the object graph to the SM, and the SM decides what sounds to play and how.
  • Nagesh 2012-09-17 11:24
    Garrison Fiord:
    I don't know why I'm suddently being signaled out for harrassment.
    Because, from your posts, it is obvious that you are in the greatest need of it.
  • Garrison Fiord 2012-09-17 11:26
    Nagesh:
    Garrison Fiord:
    I don't know why I'm suddently being signaled out for harrassment.
    Because, from your posts, it is obvious that you are in the greatest need of it.

    Which are modified by the editors to lampoon me, ignoramus. Mostly because I deserve it for being a thread-jacking idiot.
  • Garrison Fiord 2012-09-17 11:30
    Remy Porter:
    It's not really a case of indirection. It's a case of making sure objects have one job. There is no reason why a game object should contain code for managing sound. But the Singleton approach essentially requires it.

    No one designs game engines such that their game objects contain rendering code (not good game engines anyway!). The core GameEngine usually contains a Renderer. The GameEngine manages the Renderer object, so there's no reason for the Renderer to enforce being a Singleton. For each frame, the Engine passes the pertinent portions of the object graph to the Renderer, which then Renders it.

    It makes sense to implement the sound engine the same way. Even if you don't implement the Observer pattern (that's only one way to implement it, after all, and probably not the best), the same process used by the Renderer makes sense for the SoundManager. Each tick, the Engine sends the pertinent parts of the object graph to the SM, and the SM decides what sounds to play and how.

    Spoken like someone who attended college in 1995.
  • Not Nagesh 2012-09-17 11:31
    Röb:
    Cheers Remy

    Just realised the method I've used actually does pretty much what you say it should - get_instance() creates the instance if it doesn't exist and the constructor is just a constructor.

    Eg:


    /* snip */
    null === self :: $instance AND self :: $instance = new self;
    /* snip */


    Yes, you can. But please don't. Any competent programmer should be able to look at any line of code you write and know what that line does without having to stop and think. Terseness of code is not the ultimate goal - clarity and maintainability should be the ultimate goals. I don't have to stop and think about an if/else (or even a simple ternary). I do have to stop and think (even if only briefly) about this line of code.

    This is intended to be constructive criticism - I've written my share of bad code. However, it is our responsibility as professional developers to ensure that other developers (and ourselves in six months) have to spend as little time looking at our code as possible so they can spend their time writing code.

    CAPTCHA: erat - The latest craze in electronic pets.
  • D-Coder 2012-09-17 11:37
    Garrison Fiord:
    Over the years I've read from this site, I've read countless comments criticizing the editors. I've read offensive and possibly-illegal posts by Zunesis. I don't know why I'm suddently being signaled out for harrassment. And yet I keep whining about it and not contributing to threads. It's like I'm the dumbest troll that was ever born.
    You're not even that. Nagesh is dumber.
  • Mathias 2012-09-17 12:11
    I like this new trend of Remy's comments in CodeSOD articles.
  • AGray 2012-09-17 12:22
    Remy Porter:
    I still probably wouldn't go Singleton, but that's because I'd want multiple SoundManagers governing different varieties of sound. One SM for effects, one SM for background music, one SM for menu operations, etc.

    This allows them to be compartmentalized in different threads if necessary (the effects SM should run in the game thread, the background SM should run in the global thread, ditto on the menu one).

    The real constraint would be something more along the lines of each game object can be observed only once by a given kind of SoundManager. The effects and music manager can both observe the spaceship, but only one effects manager can observe the spaceship- if another tries to register it either throws an exception or replaces the original effects manager.

    Further, in a situation like this, the GameEngine object should have the freedom to manage its child objects as it sees fit. If the GameEngine decides its a good idea to have one and only one SoundManager, then it can create only one. If it decides there should be more than one, it can make more than one. By making it a Singleton, you've said that the SoundManager knows best, and no one should ever create any more instances of the SM, which likely isn't true.

    Singelton enforces statically a condition that usually should be enforced by convention.

    Ironically, I have a singleton in the application I'm writing. I'm not opposed to the pattern itself, but it's definitely overused.


    Thanks for those insights!

    The reason I use a Singleton in this case is actually not as a worker, but as a persistent cache of settings.

    Yes, I know it's akin to the idea of Global variables in an Object-Oriented program, which is a WTF in its own right, but given that I have to store and load this information from save files, as well as make it persist across scenes/application states, I am finding it simplest and most maintainable to do it this way in this instance.

    My actual control is happening via other objects - the Maestro object I mention in my blog post monitors those Settings and knows how to interpret a PlaySoundEffect instruction, or tell the AudioSource (a Unity object) how to change background music. Settings GUI performs the value-set operations, obviously, as will Game Load system when that is implemented.

    As a rule in my own development, I only use a singleton when an object fulfills the requirement that under any circumstances, one and only one of it should exist at any given time. It's an easy pattern to abuse, as you (and others) have shown.
  • Dave Insurgent 2012-09-17 12:40
    The reason I use a Singleton in this case is actually not as a worker, but as a persistent cache of settings.


    You mean it's almost like it is a Repository of settings?

    How do you test your code that uses that object in a manner that isolates that object?

    I don't think adding eventing to your system is at all too big of a deal: unless you throw away your game engine every time, in which case there's plenty of other WTFs in what you're doing and you have yourself a big ol' WTF onion.

    If you don't have eventing and a clear separation of concerns, then you likely have a deep, or going to be deep, object hierarchy too... a Player might be a Character and a Monster is also a Character and yadda yadda yadda.

    Eventing and data-driven go well together and will let you have testable, resuable, separate and maintanable code.

    You can also use DI without the framework, in which case, yes, your AudioEventHandler should get an instance of the SoundManager provide to it - when you use Singleton, you're explicitly coupling that SoundManager implementation to it's use. You're doing the same thing, you just for some reason think it's more readable to have the dependency implicitly wired all over the place, rather that explicitly in the constructor.

    Also, by wanting "one instance" of something, you basically set yourself up for issues with multi-threading (not so bad) and multi-process (a complete fucking disaster). Maybe not relevant to a tiny game, but in a web application, youre almost guaranteed to be multi-process at any scale except prototyping.
  • Jeff Dege 2012-09-17 12:46
    Remy Porter:
    Further, in a situation like this, the GameEngine object should have the freedom to manage its child objects as it sees fit. If the GameEngine decides its a good idea to have one and only one SoundManager, then it can create only one. If it decides there should be more than one, it can make more than one.

    And who decides how many GameEngines there are?

    Or if you're using a DI engine, who decides how many DI engines there are?

    I'm working on a code-base that had a large number of singletons. In adapting it over the years, we've made significant changes to what goes on under the hood. A call to Logger.getInstance() no longer returns a static instance member, it forwards the call to a single instance of a SingletonManager class, which returns the appropriate instance of a Logger. In essence, we now only have a single singleton, the SingletonManager. Except that it doesn't actually construct itself, it relies on SingletonManagerStore - which is an abstract class with different implementations for different environments. One that stores the SingletonManager in memory, for normal executables, one that stores it and retrieves it from HttpContext.Current.Session, when used in ASP.NET, etc.

    Underneath, you could argue that this isn't actually singleton, anymore. But for the coder who's calling Logger.getInstance(), it looks like it's a singleton.
  • Garrison Fiord 2012-09-17 12:46
    Mathias:
    I like this new trend of Remy's comments in CodeSOD articles.

    Me too. As long as he doesn't write long articles that are gayed up by unicorns.
  • RandomGuy 2012-09-17 12:56
    Erik:
    TRWTF is the singleton pattern!

    Could not agree more.
    Global variables are bad and hiding them in a singleton does not make them better.
  • Dave Insurgent 2012-09-17 13:02
    Jeff Dege:
    And who decides how many GameEngines there are?


    The user? By virtue of how many instances of the application they start?

    it forwards the call to a single instance of a SingletonManager class, which returns the appropriate instance of a Logger.


    Why not LoggerFactory instead of a (presumably) multi-use SingletonManager?
  • Harrow 2012-09-17 13:03
    lmm:
    If you can't call getInstance() on it, is it still a singleton?
    It is a zeroton.

    -Harrow.
  • AGray 2012-09-17 13:04
    Dave Insurgent:
    The reason I use a Singleton in this case is actually not as a worker, but as a persistent cache of settings.


    You mean it's almost like it is a Repository of settings?


    That's a much more concise way of putting it...really, it is a Repository. Its only concern is the storage and persistence of data. Other things, such as the Settings GUI and Maestro, consume this data.

    How do you test your code that uses that object in a manner that isolates that object?

    I don't think adding eventing to your system is at all too big of a deal: unless you throw away your game engine every time, in which case there's plenty of other WTFs in what you're doing and you have yourself a big ol' WTF onion.


    I actually don't have to worry about throwing things away too much, given the way Unity is set up. Also, eventing is a core concern of Unity; it provides a rather nice event system that I take advantage of in a number of ways, including the application of the saved settings.

    If you don't have eventing and a clear separation of concerns, then you likely have a deep, or going to be deep, object hierarchy too... a Player might be a Character and a Monster is also a Character and yadda yadda yadda.

    Eventing and data-driven go well together and will let you have testable, resuable, separate and maintanable code.


    While I agree a deep hierarchy should be carefully considered, having a hierarchy is not necessarily a problem.

    The case you gave, while I use different terms in the code I've developed, is an apt one. An entity that can be engaged in a battle is...well, an Entity (has a health system and combat rolls, pretty much). In a standard JRPG there are two types of combative entities - players, and enemies.

    What is the difference between enemies and players? Players equip Equipment and Aethos (the customization system of the game...just nod and continue...), while Enemies drop various things, like Consumables and Equipment.

    On that note, Equipment, Aethos, and Consumables are all RPG Items. The abstract RPG Item defines a basic contract among items; the derived item classes state the applicable differences (e.g., you cannot Equip a Tonic, but you can drink it to restore 150 HP. Your Equipment confers stat bonuses, but Aethos also confer skills and stat bonuses, and further can level up to a certain point).

    You can also use DI without the framework, in which case, yes, your AudioEventHandler should get an instance of the SoundManager provide to it - when you use Singleton, you're explicitly coupling that SoundManager implementation to it's use. You're doing the same thing, you just for some reason think it's more readable to have the dependency implicitly wired all over the place, rather that explicitly in the constructor.

    Also, by wanting "one instance" of something, you basically set yourself up for issues with multi-threading (not so bad) and multi-process (a complete fucking disaster). Maybe not relevant to a tiny game, but in a web application, youre almost guaranteed to be multi-process at any scale except prototyping.


    First paragraph - I have no contest to this point. This is the downside of using a Singleton, and why I considered everything I know of before deciding to use a Singleton (which, is admittedly not much).

    Second paragraph - while this is not an issue for me at present (Unity handles multithreading behind the scenes for me), if I were to make my own custom game engine, you'd be right beyond a ghost of a doubt - I would almost certainly have to multithread or multiprocess to achieve playable performance.
  • Slapout 2012-09-17 13:17
    @Deprecated:
    snoofle:
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!


    this.





    PS., Although I do not like replies that contain a singleton 'this', I will make an exception in this case.


    Shouldn't that be throw an exception in this case? :-)
  • Dave Insurgent 2012-09-17 13:17
    That's a much more concise way of putting it...really, it is a Repository. Its only concern is the storage and persistence of data.


    Right, which is why it might serve you well not to use a Singleton for it. You may test with an InMemorySettingsRepository or a MockSettingsRepository (or Fake, or whatever) - you may use a ProprietarySettingsRepository that uses a file format you create, or an XmlSettingsRepository. Or a CloudSettingsRepository.. just saying, you may need one or more of those a any point. I don't see how Singleton helps you here.

    I actually don't have to worry about throwing things away too much, given the way Unity is set up. Also, eventing is a core concern of Unity; it provides a rather nice event system that I take advantage of in a number of ways, including the application of the saved settings.


    Sure, but if that's all you're using it for I think you're selling yourself short.

    While I agree a deep hierarchy should be carefully considered, having a hierarchy is not necessarily a problem.


    Well, I am not a professional game developer, but I have seen some interesting material about how absolutely insanely large commercial games can get when the use inheritance to model game logic/rules/concepts. Inheritance is an is-a approach, and while it's really easy to agree that Equipment is an Item, and so is a Consumable, it's not always going to be that simple. What about Equipment that can only be used once? Is that a Consumable? Or does Consumable imply that the player actually eats it? What then is the property that actually causes those objects to diminsh? Do you call them Finite? Is a Sword a piece of Equipment that implements Repairable, Damaging and so on? It seems appealing to try to categorize things that way, but I think it's a lot easier to manage when instead you think that a Sword is an Item with a Set<Attribute>. You can then have CompositeAttribute use the Specification pattern to define valid/invalid Attribute definitions (for example, Indestructable can not be Repairable) - this is harder to do with interface/multi-inheritance. You can also externalize those definitions which makes your game-engine really reusable because you don't have a compile-time definition of what is/isn't possible.
  • Coyne 2012-09-17 13:21
    The code used to get the instance isn't shown, but I assume it would typically be something like this:

        KpiService k = (new KpiService()).getInstance();


    After all, if one Lone Ranger is good, a legion of them would be better, right?
  • DB 2012-09-17 13:29
    HAHAHAHAHA! (More non-spam text?_
  • jay 2012-09-17 13:37
    Remy Porter:
    And you should avoid using the Singleton pattern. It's one of those patterns that's so easy to implement and seems so useful that you're tempted to create a bunch of them, but they should be used rarely.


    I think there's a missing step in this (no pun intended) logic.

    As stated, the best I can make of this argument is, "This thing is easy to use and is very useful, therefore you shouldn't use it."

    Like, "You shouldn't do addition in your programs. The plus operator is very easy to use and does addition easiy, so your'e tempted to use it to much."
  • jay 2012-09-17 13:47
    T:
    TheSHEEEP:

    (...)
    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?


    Obviously you should've used Dependency Injection framework.
    No matter that your game would be slower, larger and harder to maintain.

    But seriously, singletons are quite error prone in enterprise applications (clustering).
    Often they're used as lazy shortcuts.
    Environment can't manage them.
    Switching to another implementation can be painful.

    I see nothing wrong in using them in desktop applications or games.


    Well, if you have a clustered app, then you can't implement a singleton with the simple "private static MySingleton instance" approach. You need to store the singleton somewhere that will be "single" for the whole app, not just "single" to one node on the cluster. I suppose you could say that singletons don't scale, that is, if you have an app that was written for a single server, and that app uses singletons, you can't necessarily just drop it into a clustered-server environment. You'd have to look at how the singletons were implemented and probably redesign them. I'll agree that that's an argument against singletons as currently implemented given today's state of the art. But one could argue that that's a deficiency in current language implementations rather than in the concept of a singleton. The same objection would apply to all static data (shared data for you VB folks).
  • glopal 2012-09-17 13:53
    The getInstance method should instantiate the instance variable using the constructor, which should be private. No?
  • Remy Porter 2012-09-17 14:00
    GoF included things like Vistors and Decorators and Delegates specifically because is-a relationships can get very complex and multiple inheritance is a potential nightmare.

    As a general rule, any hierarchy deeper than three levels needs to be justified. This goes for inheritance trees, file organization schemes, and slide presentations.
  • Remy Porter 2012-09-17 14:01
    The operative word is that it seems useful, not that it is useful. Global variables also seem useful, and they are really easy to implement- at least until you have to support them. Then you discover how much spaghetti they actually create.
  • jay 2012-09-17 14:06
    Remy Porter:
    There are a lot of ways to provide sound functionality. One way that would be vastly superior to a Singleton is to use the Observer pattern. A single SoundManager instance registers interest in game objects. When sound-worthy events occur in those objects, the SoundManager matches them up with a sound.

    This approach lends itself to building a declarative system for linking sounds to events, making it easier to modify your game later. It also means that sound can be added to classes without changing the classes themselves. Why should a spaceship know how to make noises? It's a spaceship- noises are nothing more than a side-effect of what it does when it's doing far more interesting things.


    Hmm, interesting, but I don't see how this would be superior to implementing a SoundManager singleton.

    I don't know exactly how the original poster implemented this, but logically you could create a "playSound" function that takes an argument that describes what sound. Like "SoundManager.getInstance().playSound(FIRE_PHASORS)". The spaceship class then doesn't know anything about how to play sounds. It just knows to call the sound manager to do it. If you decide to change the sound the spaceship makes, you change the code in playSound, not in the spaceship class. Responsibility is properly delegated.

    To accomplish what you're describing, every class that is associated with a sound would have to provide a hook to declare a listener. Then it would have to call the listener when it was time to play the sound. So instead of a single playSound call, we now have to define a listener field, create an addSoundListener function, and then we still have to call soundListener.playSound. Then we have to write code in another class that registers the sound listener with the spaceship object. Instead of one line of code, we have 10 or 20 lines of code to accomplish the same thing, split across at least two classes and four or more functions.

    And what have we gained? It gives us the ability to have multiple, different sound managers, so we can register different sound managers with different "sound-producing" objects. Maybe there's value in that, but I doubt it. If different sounds are produced in different ways -- like some are produced by playing an AVI file while others are synthesized internally with code -- we could handle the distinction in the sound manager based on the sound identifier passed in. This would probably be easier than having multiple sound managers registering themselves.

    If you are creating a game engine API where the sound manager is not part of your API, an observer pattern might be a good idea as it lets the caller supply its own sound manager. That's about the only case I can think of where it would offer an advantage.


  • Dave Insurgent 2012-09-17 14:12
    The spaceship class then doesn't know anything about how to play sounds.


    Why does the spaceship know about sound at all?

    those sounds emit in response to events generated by the spaceship.

    i'd want my spaceship to fire a weapon, and that to produce a weapon_fire event, which the sound system observes and produces based on the type of the weapon. that way, if a turrent also has a weapon which can fire, it's all done. I don't have to worry about forgetting to make the turrent play a sound when it shoots.
  • This-tle and sham-rock 2012-09-17 14:18
    snoofle:
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!

    ^ This.
  • Remy Porter 2012-09-17 14:22
    The starship class needs to know that PHASORS make sounds. Why? What happens when we change our business rules and decide that PHASORS only make sounds in certain conditions. Do we put that logic in the SoundManager or the starship class?

    As I stated in a different comment, a better approach would be to let the GameEngine manage this. In each tick of the game, the GameEngine passes the relevant portions of the object graph off to the SoundManager and the Renderer objects. The observer pattern is, admittedly, overkill, but the Singleton pattern is hiding simplicity in global variables, which means lots of spaghetti code.

    The starship object should be focused on the business rules of starships. How to move. How to shoot. How to update stats as it takes damage and consumes inventory. Other objects should be responsible for deciding that sounds need to be played.

    I wrote a Singleton last week. I have a a set of Handler classes. I have a service that receives requests. Each request can be handled by a different type of Handler. I wrote a Singleton to use as a Factory; it uses reflection to find all concrete classes that implement the Handler interface and creates instances of them. When a request comes it, it invokes the CanHandle method on each Handler to find the Handler that's suitable for that request.

    I chose to make that factory a Singleton because a) I don't want to reflect the assemblies for each request, and b) it's possible that the instantiation of a Handler might be expensive.

    Now, only one class ever actually creates an instance of the Factory, and the beauty of that is that it allows me to restrict object creation. My Singleton is not, and could not be used, as a global variable. The internal/package/friend access modifier exists for a reason.

    As a side effect of this design, the individual Handlers are singleton by convention. I never create more than one instance, but I could if I ever wanted to. The Handlers are completely reusable. They're also stateless-by-convention, and there's a rule I wish I did have: "This class and all its children are stateless, and may not implement any data members."
  • Fakeyede Mobolaji 2012-09-17 14:26
    That code is not OK. Should have been somrthing like this

    public static KpiService getInstance(){
    return instance ?? instance = new KpiService();
    }

    private KpiService(){
    }

    Notice the constructor is private (This way, u can't just create a new KpiService. You have to go through getInstance() to get the singleton object
  • Fakeyede Mobolaji 2012-09-17 14:28
    That code is not OK. Should have been something like this

    public static KpiService getInstance(){
    return instance ?? (instance = new KpiService());
    }

    private KpiService(){
    }

    Notice the constructor is private (This way, u can't just create a new KpiService. You have to go through getInstance() to get the singleton object
  • jay 2012-09-17 14:28
    Remy Porter:
    The operative word is that it seems useful, not that it is useful. Global variables also seem useful, and they are really easy to implement- at least until you have to support them. Then you discover how much spaghetti they actually create.


    When global variables are used to pass parameters between functions, they are horriby evil. I absolutely agree with that. But that doesn't mean that global data is always evil. Some data is rightly global.

    I used to work at a company where the programmers were always looking for new ways to create global data. They would put EVERYTHING in session variables. Then we'd have all this dangling junk data sitting in the session. When someone came back to modify the program, it was almost impossible to figure out who put the data there or what it was for. The connections between modules were lost. When we beat that out of them, they started putting function parameters in static fields. After a few more beatings, they had to find other tricky places to hide it. For some reason people just couldn't get the idea to pass parameters between functions as, say, function parameters. I went crazy at that place.

    But that doesn't mean that global data is never appropriate. It makes good sense for data that legitimately applies all over the place and which there should be only one copy of. Like, what's the user's timezone? Many modules might use that. Rather than pass it in and out of 1000 places, it makes sense to keep it in a global somewhere. Or user preferences. Yes, we could load preferences in a start-up function and then pass them to everything, but it's a pain and it gains nothing.

    What's the difference? When data applies to the application as a whole, it's legimately global. When it applies to one particular thing we're doing right now, it is not legimately global. What is the current user's name? Fair game for global. What is the name of the customer on the transaction we are currently processing? Not fair game for global at all.
  • Remy Porter 2012-09-17 14:37
    I'm not saying "Never use singletons" or "never use globals". But they should be used sparingly. And for the most part, singletons should be implemented by convention, not by pattern.

    Globals for things that are truly global, singletons for things that are truly singular.
  • Dave Insurgent 2012-09-17 14:37
    Do we put that logic in the SoundManager or the starship class?


    I'd put that logic in the PhaserEventHandler which listens for PhaserFireEvent and then, based on the attributes of the Phaser that is being fired, play a sound. Just like, if instead it were a SniperRifle and it had a Silenced attribute, the handler would play a different sound.

    The idea that the Player knows that their SniperRifle makes a sound isn't very useful, and it means that they need to know about the assets involved (either directly as resources, or indirectly as method calls: playSilencedSniperRifleSound()).

    When I shoot a gun, the air around me makes the sound, not me.

    The starship object should be focused on the business rules of starships. How to move. How to shoot. How to update stats as it takes damage and consumes inventory


    Why? This is dogma. The starship is just a set of data: it is the union of a Thruster, a Hull, a Shield and a Weapon.

    When the player presses X, the Thruster emits an event indicating that it is active. The game engine delegates that event through handlers which then apply forces to the various entities that represent the physical model of the ship. The rendering subsystem updates the view of the ships physical model based on the observation of its physical state. Does the Ship need to know how it collides with other ships now, too? There's virtually no limit here.

    I wouldn't even have a Ship class. It's just a definition of a set of attributes that themselves may be composed of other primitive attributes in a very shallow hierarchy.

    it uses reflection to find all concrete classes that implement the Handler interface and creates instances of them. When a request comes it, it invokes the CanHandle method on each Handler to find the Handler that's suitable for that request.


    You used a Singleton to facilitate Chain of Responsibility. I don't see how the former is needed for the later.

    Now, only one class ever actually creates an instance of the Factory,


    Then your concerns about performance were a little unnecessary and your design decision seems tantamount to premature optimization.

    They're also stateless-by-convention, and there's a rule I wish I did have:


    You mean immutability?
  • dogmatic 2012-09-17 14:45
    Remy Porter:
    A single SoundManager instance registers interest in game objects. When sound-worthy events occur in those objects, the SoundManager matches them up with a sound.


    Hey, you know what would be a good pattern to enforce that there is only one instance of this SoundManager class? lol

    I too don't get the singleton hate. I agree that if you have more than what you can count on one hand in any given application you probably are over-using them, but I will use them any time there should only ever be one instance of a class. I have made a SoundManager singleton for some apps. And in a couple lightweight MVC frameworks I've developed, the controller class is a singleton.
  • Remy Porter 2012-09-17 14:47
    Dave Insurgent:

    The starship object should be focused on the business rules of starships. How to move. How to shoot. How to update stats as it takes damage and consumes inventory


    Why? This is dogma. The starship is just a set of data: it is the union of a Thruster, a Hull, a Shield and a Weapon.


    If a Thruster is a meaningful object in your context, fine. But we're getting too bogged down in the details of a hypothetical application and losing track of the core premise: objects should be focused on doing the job of being themselves. If we're building a Galaga clone, you've just terribly over-designed the application.

    Dave Insurgent:

    Now, only one class ever actually creates an instance of the Factory,


    Then your concerns about performance were a little unnecessary and your design decision seems tantamount to premature optimization.


    Only one class actually creates an instance of the Factory. I didn't mention how many instances of the client class there were. There will be a large number of clients accessing the Factory.

    Dave Insurgent:

    They're also stateless-by-convention, and there's a rule I wish I did have:


    You mean immutability?


    Immutability and statelessness are not the same, but I'd be fine with immutability as well. There are OO languages that have things like that, and there are plenty of functional languages that do.
  • Dave Insurgent 2012-09-17 14:48
    When I shoot a gun, the air around me makes the sound, not me.


    Just to expand on this:

    If the person firing the Weapon is the one who has to play the sound, they they must also become aware of all the rules that could alter how the sound is played. This is a clear violation of SRP.

    This means that if the Ship can fly in to the atmopshere as well as out in space, and you want to accurately model the fact that sound doesn't travel in space (nevermind why: that's irrelevant) - then suddenly your ship has to be able to tell if it is in space or not. I'm not talking about a HUD saying "You're now in space d00d!!!1elevent" - I'm talking about, at a code level, the spaceship now needs to be able to perform a test, or acquire information from somewhere ... and guess what? That leads to SomeSingleton.getInstance().isInSpace(this) or just a static method call. And if you want to add this feature, you have to go through and do it everywhere that plays sound. This is not DRY at all.

    Whereas, in the WeaponEventHandler (I wouldn't even have Phaser as a class/event handler), I could add that check, and it might not be very pretty, but at least it is in one place. It's no less pretty than putting it in the class 'using' the Weapon, but it's a lot more maintainable/testable.

    Then again, you could set up a Chain of Responsibility in the WeaponEventHandler at that point, refactor the old one to be a TerrestrialWeaponEventHandler and a SpaceWeaponEventHandler and so on and so forth.
  • Thuktun 2012-09-17 14:57
    The constructor is public, allowing anyone to create a new singleton whenever.

    Nowhere in the cited code is that construction initiated; if the "SNIP" section only contains functional bits of the singleton, the singleton is relying on something external to initialize it by constructing only one of these.

    One can easily find the Right Way to do this by doing a bit of Googling,or alternatively by using a framework like Spring that provides baked-in singleton initialization.
  • AGray 2012-09-17 15:02
    [quote user="Dave Insurgent"][quote]That's a much more concise way of putting it...really, it is a Repository. Its only concern is the storage and persistence of data.[/quote]

    Right, which is why it might serve you well not to use a Singleton for it. You may test with an InMemorySettingsRepository or a MockSettingsRepository (or Fake, or whatever) - you may use a ProprietarySettingsRepository that uses a file format you create, or an XmlSettingsRepository. Or a CloudSettingsRepository.. just saying, you may need one or more of those a any point. I don't see how Singleton helps you here.[/quote]

    You have a point here. Depending on one's opinions on the Singleton, a singleton just does the same thing with a different flavor.

    If you look at my webplayer, there are three actions the user can take on the title: Settings, New Game, and Load Game. Settings serves to set the settings up from the title screen, such that when a new game is started, the user already has their settings set up. The Load Game option is just going to overwrite that when it loads from the save file.

    The point I think I see from your argument is, instead of using a singleton, just create class(es) that live in the background (like the singleton currently does), and serve up information as it is needed (e.g. sound settings repo, graphics settings repo, etc.) It's easier to test by using unit tests, etc.

    I have to admit - I berate myself for not thinking of the problem in that way. Lack of experience on my part; I'll give that setup a shot!

    [quote]I actually don't have to worry about throwing things away too much, given the way Unity is set up. Also, eventing is a core concern of Unity; it provides a rather nice event system that I take advantage of in a number of ways, including the application of the saved settings.[/quote]

    Sure, but if that's all you're using it for I think you're selling yourself short.[/quote]

    I'm a bit confused by this statement. What other possibilities do you see?

    [quote]While I agree a deep hierarchy should be carefully considered, having a hierarchy is not necessarily a problem.[/quote]

    Well, I am not a professional game developer, but I have seen some interesting material about how absolutely insanely large commercial games can get when the use inheritance to model game logic/rules/concepts. Inheritance is an is-a approach, and while it's really easy to agree that Equipment is an Item, and so is a Consumable, it's not always going to be that simple. What about Equipment that can only be used once? Is that a Consumable? Or does Consumable imply that the player actually eats it? What then is the property that actually causes those objects to diminsh? Do you call them Finite? Is a Sword a piece of Equipment that implements Repairable, Damaging and so on? It seems appealing to try to categorize things that way, but I think it's a lot easier to manage when instead you think that a Sword is an Item with a Set<Attribute>. You can then have CompositeAttribute use the Specification pattern to define valid/invalid Attribute definitions (for example, Indestructable can not be Repairable) - this is harder to do with interface/multi-inheritance. You can also externalize those definitions which makes your game-engine really reusable because you don't have a compile-time definition of what is/isn't possible.[/quote]

    I love this bit here. I am going to write a blog article on it. I did some really good thinking over lunch on it.

    To answer some of the questions, I can invoke "A wizard did it." By that, I mean, in my specification of the system, I've already defined business rules like, "Equipment and Aethos cannot be Consumables." (Though that would be interesting; imagine whacking an enemy with a gummy sword, then eating part of it to restore HP! I'm thinking about that one too, btw!)

    That being said, I also see YOUR way of things - what if, in a future project, I want items to be degradable? What if I want to implement the Gummi Sword? The way you defined would be better.

    So, well-stated...I've got much more to consider going forward. And that's a good thing; player stats were what I was looking at working with next...
  • Dave Insurgent 2012-09-17 15:09
    But we're getting too bogged down in the details of a hypothetical application


    I disagree. I'm advocating for a data-driven model, which is philosophically different than your object-orieted model, and I'm trying to demonstrate why games specifically seem to be a good candidate for it.

    I too like rich domain objects, but what are those domain objects? I'm proposing that in a robust game engine that is easy to work with and appropriate for multiple purposes, the domain is a much smaller set of data than the higher order combinations you're operating with.

    Immutability and statelessness are not the same


    Oh, I agree - but they both exist in a way that produces no side effect on themselves. Can you think of a time when using one would be appropriate, but not the other? (Performance or memory aside, I mean, from a correctness perspective).
  • Lon 2012-09-17 15:11
    There's lots of things wrong with this.

    1. Using getInstance() in the normal way never created an object.

    KpiService.getInstance().foo() - never calls constructor.

    2. Even if you did artificially create an object and then use the singleton access getInstance() such as:

    KpiService junk = new KpiService();
    KpiService.getInstance().foo();

    It would be possible to change the instance variable by creating another KpiService object, thus violating the whole singleton concept.
  • Dave Insurgent 2012-09-17 15:14
    I love this bit here. I am going to write a blog article on it. I did some really good thinking over lunch on it.


    I'd like to read it - feel free to send an e-mail to doug dot moscrop at gmail dot com - if you're so inclined - when you post it.
  • Dave Insurgent 2012-09-17 15:17
    That being said, I also see YOUR way of things - what if, in a future project, I want items to be degradable? What if I want to implement the Gummi Sword? The way you defined would be better.


    Not to mention you can also write automated acceptance tests for them, so that your game, as an application of your game engine, is driven not by the "wizard specification" (nicely put), but by an actual spec - that maybe someone else a little more 'product-oriented' or 'business-oriented' (almost like an analyst... for business!) writes themselves.
  • xtremezone 2012-09-17 15:25
    TheSHEEEP:
    I don't get the bashing SIngletons get in general.

    The usual argument is: It is bad style.
    Uh-huh. Is that so?

    In a game I once made I had a singleton that allowed to play a fire-and-forget sound. It was very useful that I could do so from each place in the code, from where loading was finished to when a weapon was fired (both were obviously in very different parts of the code).

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?

    Of course, a singleton should actually be implemented correctly ;)
    And of course, some things should not be a singleton, as they really shouldn't be usable everywhere. But saying that the pattern itself sucks because you are not capable of using it correctly does really put me off.

    I guess, like with many things, a singleton can be used to do something wrong, and therefore, some seem to think that it is generally bad.
    Your game objects have no business knowing about higher-level objects and generally no business knowing about siblings either (e.g., player doesn't need to directly manipulate ememy; if he could why not just set enemy's health to 0 and call it a day?). You want to have higher-level manager classes that contain state for the game and manipulate all of the objects.

    For example, if your game objects themselves detect collisions, then which is correct? player.checkCollision(enemy) or enemy.checkCollision(player) or both? What about collisionManager.check(player, enemy)?

    Singletons can get the job done, but it's not ideal. They allow you to be lazy and not think things through. This results in terrible design decisions that don't actually make any sense at all, but are allowed to happen because of the global state. Most importantly, just because you only need one of something doesn't mean that you have to assert that only one of something can ever exist. That's just dumb. It prevents object-oriented programming from being used as intended.

    Watch this if you haven't yet and be enlightened:

    http://youtu.be/-FRm3VPhseI
  • AGray 2012-09-17 16:01
    Dave Insurgent:
    That being said, I also see YOUR way of things - what if, in a future project, I want items to be degradable? What if I want to implement the Gummi Sword? The way you defined would be better.


    Not to mention you can also write automated acceptance tests for them, so that your game, as an application of your game engine, is driven not by the "wizard specification" (nicely put), but by an actual spec - that maybe someone else a little more 'product-oriented' or 'business-oriented' (almost like an analyst... for business!) writes themselves.


    TDD in a nutshell...I've been exposed to it, and trying to integrate it into my very-indie dev ops at home.

    Well, we should thank the TDWTF staff - the WTF posted has helped one developer see another set of WTFs in his own work! Thanks Remy et al!
  • Zecc 2012-09-17 16:04
    jay:
    I don't know exactly how the original poster implemented this, but logically you could create a "playSound" function that takes an argument that describes what sound. Like "SoundManager.getInstance().playSound(FIRE_PHASORS)". The spaceship class then doesn't know anything about how to play sounds. It just knows to call the sound manager to do it. If you decide to change the sound the spaceship makes, you change the code in playSound, not in the spaceship class. Responsibility is properly delegated.

    To accomplish what you're describing, every class that is associated with a sound would have to provide a hook to declare a listener. Then it would have to call the listener when it was time to play the sound. [...]

    And what have we gained? [...]

    If you are creating a game engine API where the sound manager is not part of your API, an observer pattern might be a good idea as it lets the caller supply its own sound manager. That's about the only case I can think of where it would offer an advantage.


    I was going to say pretty much the same thing, but there's another advantage. The spaceship only needs to fire the event once, and both the SoundManager and the ParticleEmitter systems will do their thing if they've registered their interest.

    In any case... what does this have to do with the whether to use singletons or not?
    Even in the case where Spaceship calls SoundManager:getInstance(), if what is returned is a singleton or not should be of no concern to Spaceship. It's purely an implementation detail of SoundManager.
  • AGray 2012-09-17 16:13
    @Remy - Question.

    Reading the replies in between my exchange with Anonymous Dave, I notice he's not the only one who considers the Singleton as something that, while it appears helpful, is only half as helpful as the next best solution.

    You mentioned you're using a singleton in an app you're using. I would assume you're using it the "right" way (after all, you're an editor for TDWTF, not a regular submitter)...what/when is the right way/time to use a Singleton class?
  • wgeek 2012-09-17 16:41
    One problem is that getInstance() will be called before the constructor unless the programmer explicitly "news" the object first. And why is the constructor public. Yeah, this is a WTF.
  • Jazz 2012-09-17 16:51
    snoofle:
    It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.


    You sound like the love child of Nagesh and Immanuel Kant.
  • Leak 2012-09-17 16:51
    AGray:

    public class Singleton
    
    {
    private static Singleton _Instance;
    public static Singleton Instance
    {
    get
    {
    return _Instance
    ?? (_Instance = new Singleton());
    }
    }
    // ...Snip...


    And that still leaves a miniscule window of misopportunity for a race condition to occur between checking the value of _Instance and assigning the newly created Singleton object to it...

    If you want to get rid of that too, assign a newly created Singleton object to _Instance in it's declaration - such static variable initializers are guaranteed to run once and only once when the class is first loaded.
  • SilentRunner 2012-09-17 16:57
    Is that the is that Clinton referred to? Is that the is that is? Or aren't?

    What the hell is a singleton? Something to do with a card game the last I knew. Why are we talking about a card game on TDWTF?
  • DarrenC 2012-09-17 17:25
    Mcoder:
    Why don't languages already come with a singleton modifier? It would remove an entire class of WTFs from the face of Earth (and create another entire and more entretaining class, so we can laugh more).


    We've had them since the dawn of time - they're called globals.
  • Meschelle 2012-09-17 18:11
    jay:
    Remy Porter:
    The operative word is that it seems useful, not that it is useful. Global variables also seem useful, and they are really easy to implement- at least until you have to support them. Then you discover how much spaghetti they actually create.


    When global variables are used to pass parameters between functions, they are horriby evil. I absolutely agree with that. But that doesn't mean that global data is always evil. Some data is rightly global.

    I used to work at a company where the programmers were always looking for new ways to create global data. They would put EVERYTHING in session variables. Then we'd have all this dangling junk data sitting in the session. When someone came back to modify the program, it was almost impossible to figure out who put the data there or what it was for. The connections between modules were lost. When we beat that out of them, they started putting function parameters in static fields. After a few more beatings, they had to find other tricky places to hide it. For some reason people just couldn't get the idea to pass parameters between functions as, say, function parameters. I went crazy at that place.

    But that doesn't mean that global data is never appropriate. It makes good sense for data that legitimately applies all over the place and which there should be only one copy of. Like, what's the user's timezone? Many modules might use that. Rather than pass it in and out of 1000 places, it makes sense to keep it in a global somewhere. Or user preferences. Yes, we could load preferences in a start-up function and then pass them to everything, but it's a pain and it gains nothing.

    What's the difference? When data applies to the application as a whole, it's legimately global. When it applies to one particular thing we're doing right now, it is not legimately global. What is the current user's name? Fair game for global. What is the name of the customer on the transaction we are currently processing? Not fair game for global at all.
    So global constants are ok, not global variables?

    FWIW - I think configuration and some type of loggigns are two good examples where global stuff makes some sense - But in both of these instance is because the data is basically constant - and certainly pertinants to almosts all of the app. Loggings is a little more difficults because we may want to separate out multiple types of loggings....but assuming their is one globals error log it would makes much sense to alway be loggings through the same instance.

    But maybe not too...
  • I fix for you 2012-09-17 18:19
    Dave Insurgent:
    That's a much more concise way of putting it...really, it is a Repository. Its only concern is the storage and persistence of data.


    Right, which is why it might serve you well not to use a Singleton for it. You may test with an InMemorySettingsRepository or a MockSettingsRepository (or Fake, or whatever) - you may use a ProprietarySettingsRepository that uses a file format you create, or an XmlSettingsRepository. Or a CloudSettingsRepository.. just saying, you may need one or more of those a any point. I don't see how Singleton helps you here.


    You have a point here. Depending on one's opinions on the Singleton, a singleton just does the same thing with a different flavor.

    If you look at my webplayer, there are three actions the user can take on the title: Settings, New Game, and Load Game. Settings serves to set the settings up from the title screen, such that when a new game is started, the user already has their settings set up. The Load Game option is just going to overwrite that when it loads from the save file.

    The point I think I see from your argument is, instead of using a singleton, just create class(es) that live in the background (like the singleton currently does), and serve up information as it is needed (e.g. sound settings repo, graphics settings repo, etc.) It's easier to test by using unit tests, etc.

    I have to admit - I berate myself for not thinking of the problem in that way. Lack of experience on my part; I'll give that setup a shot!

    Dave Insurgent (I guess):
    I actually don't have to worry about throwing things away too much, given the way Unity is set up. Also, eventing is a core concern of Unity; it provides a rather nice event system that I take advantage of in a number of ways, including the application of the saved settings.


    Sure, but if that's all you're using it for I think you're selling yourself short.


    I'm a bit confused by this statement. What other possibilities do you see?

    Dave Insurgent (I guess):
    While I agree a deep hierarchy should be carefully considered, having a hierarchy is not necessarily a problem.


    Well, I am not a professional game developer, but I have seen some interesting material about how absolutely insanely large commercial games can get when the use inheritance to model game logic/rules/concepts. Inheritance is an is-a approach, and while it's really easy to agree that Equipment is an Item, and so is a Consumable, it's not always going to be that simple. What about Equipment that can only be used once? Is that a Consumable? Or does Consumable imply that the player actually eats it? What then is the property that actually causes those objects to diminsh? Do you call them Finite? Is a Sword a piece of Equipment that implements Repairable, Damaging and so on? It seems appealing to try to categorize things that way, but I think it's a lot easier to manage when instead you think that a Sword is an Item with a Set<Attribute>. You can then have CompositeAttribute use the Specification pattern to define valid/invalid Attribute definitions (for example, Indestructable can not be Repairable) - this is harder to do with interface/multi-inheritance. You can also externalize those definitions which makes your game-engine really reusable because you don't have a compile-time definition of what is/isn't possible.


    I love this bit here. I am going to write a blog article on it. I did some really good thinking over lunch on it.

    To answer some of the questions, I can invoke "A wizard did it." By that, I mean, in my specification of the system, I've already defined business rules like, "Equipment and Aethos cannot be Consumables." (Though that would be interesting; imagine whacking an enemy with a gummy sword, then eating part of it to restore HP! I'm thinking about that one too, btw!)

    That being said, I also see YOUR way of things - what if, in a future project, I want items to be degradable? What if I want to implement the Gummi Sword? The way you defined would be better.

    So, well-stated...I've got much more to consider going forward. And that's a good thing; player stats were what I was looking at working with next...
  • likeclockwork 2012-09-17 18:30
    TheSHEEEP:

    I don't get the bashing SIngletons get in general.

    The usual argument is: It is bad style.
    Uh-huh. Is that so?

    In a game I once made I had a singleton that allowed to play a fire-and-forget sound. It was very useful that I could do so from each place in the code, from where loading was finished to when a weapon was fired (both were obviously in very different parts of the code).

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?

    Of course, a singleton should actually be implemented correctly ;)
    And of course, some things should not be a singleton, as they really shouldn't be usable everywhere. But saying that the pattern itself sucks because you are not capable of using it correctly does really put me off.

    I guess, like with many things, a singleton can be used to do something wrong, and therefore, some seem to think that it is generally bad.

    It's not necessary to be a singleton. No, you don't have to pass it everywhere. But it's just a global, or part of some other system which could be referenced and constructed as such. (game.soundmanager)

    Singleton method is usually just a way for people to have globals without having globals. Singleton is the diet soda of design patterns.

    Singleton makes a little sense if it's life-threateningly mission critical that there only be one instance. Otherwise you're just using your class itself as a container for a global variable, rather than storing the instance somewhere that actually makes sense.
  • Remy Porter 2012-09-17 19:10
    I won't claim that I made a perfect implementation of singletons, but I can at least promise that I thought about it before I did it.

    My pattern, as mentioned, was a Factory sitting atop a Chain of Responsibility. Creating the chain is potentially expensive- in this case, some objects might need to talk to the filesystem, a database, or some other resource-heavy store. Once the chain is created, however, each object doesn't expect it to change. Each object in the chain is stateless, or at least should be. Essentially I have a set of incoming requests, each request should be handled by an instance of the appropriate handler for that particular request.

    My reason for using a Singleton wasn't quite my own personal dogma: a singleton should wrap objects that are truly singular.

    Instead, I decided that an object should be singular because a) the objects it managed were stateless, b) creating those objects could be expensive, and c) I could control who accessed the singleton instantiator.

    And before you get too excited about TDWTF editors being magical fonts of programming wisdom, I have personally written up a few of my own WTFs as articles. In fact, I will give out some spoilers: I was the mentor in my next WTF, and was at least partially the source of the WTF. There are reasons I didn't route around it, but mostly it boiled down to a "Not my problem" situation.
  • Dave Insurgent 2012-09-17 19:48
    Each object in the chain is stateless, or at least should be


    Sure, I'm not sure why that matters. Arguably, if you're using Singleton, I'd really expect them to be stateful, because that's what you wanted to do: capture universal state. Otherwise it would just be static method calls.

    I guess the alternative is that they're not stateful, but they do have some creation-time configuration.

    In the former case, using Singleton still presents the ususal problems of testability and you could still just use dependency injection to pass one instance around right from main.

    In the later, I don't see any reason to enforce the one-instance rule because either your performance concern is premature/inappropriate (if you follow the same dependency injection adherence as above) or a case for a Flyweight - which is to say, I'd like to have one instance, but is it incorrect to have more than one? How can that be, if they're stateless?

    I guess you could argue that they might end up configured differently..
  • Gaui 2012-09-17 20:02
    Actually, it should be:

    public static KpiService getInstance() {
    if(instance == null)
    instance = new KpiService();

    return instance;
    }
  • Beta 2012-09-17 20:18
    A new design pattern: Null Class, when only zero (at most) instances of the class may exist at a time.
  • Jeff Dege 2012-09-17 20:24
    Dave Insurgent:
    Jeff Dege:
    it forwards the call to a single instance of a SingletonManager class, which returns the appropriate instance of a Logger.


    Why not LoggerFactory instead of a (presumably) multi-use SingletonManager?

    The user of the Logger class never sees SingletonManager, or any of its implementing functionality. He just calls Logger.getInstance().

    So whether the functionality is provided by a single multi-purpose class or by a bunch of separate Factory classes is, fundamentally, irrelevant.
  • Dave Insurgent 2012-09-17 21:20
    I appreciate that it is irrelevant to the caller, but we're talking about implementation.

    Logger.getInstance() can make use of a static LoggerFactory which, at least, only concerns itself with the construction of loggers - SingletonManager appears, to me, a violation of both SRP because it is going to contain construction logic for all sorts of things it manages, as well as OCP (okay I may be stretching here) because it requires modification to the SingletonManager whenever you introduce something completely unrelated to the other singletons.

    Either that or it's a useless layer of indirection.
  • wgeek 2012-09-17 22:42
    Dude, you're over-thinking it to your detriment.

    You ask this question:

    The idea that the Player knows that their SniperRifle makes a sound isn't very useful, and it means that they need to know about the assets involved (either directly as resources, or indirectly as method calls: playSilencedSniperRifleSound()).

    And I answer it thusly: if my design goals, which BTW you BLITHELY IGNORE, are such that I want to model that physical aspect of the "world", then YES, I DO IN FACT want the Player to "know" or "have knowledge" of the sound.

    In particular its duration.

    Yes, inter-object communication is a PITA. Decouple it then, as you have stated, with events or some other mechanism that provides a "mediator" as it were in between the Player and the sound.

    Think outside the box man, and always know what the design goals are before trying to apply a so-called solution.


  • wgeek 2012-09-17 22:57
    I would argue that the player SHOULD in fact "know" or better stated, HEAR, the sound of the weapons. This makes stealth missions possible.

    An easy way with no jive indirection, which MAY meet the design goals (as yet, UNSTATED): create a base class "Weapon" with both abstract and concrete implementations of methods.

    Use the "observer" pattern to couple that weapon with the subject, in this case the player.

    Now, use an "Environment" object that will set the parameters such as amplitude and duration of the sound appropriately for the given environment. In fact, some form of class factory would be very useful here.

    Ain't rocket science dude and I honestly think you're way off base with this. ALL WEAPONS make a sound. Think about it. If in a vacuum, the weapon's attributes such as amplitude and duration can simply be zeroed out, without need for a tri-state situation involving nulls, nor funky edge cases.

    Later.
  • Remy Porter 2012-09-17 23:10
    The only Singleton-by-rule is the Factory. The Factory maintains a list of instances of Responsible objects. The Responsible objects themselves are Singleton-by-convention- they're basically useless except for one specific task: handling requests. They're stateless because those requests arrive via a web-service. Yes, they maintain configuration information, but configuration is only checked on instantiation.

    The reason to enforce Singleton-by-rule on the Factory is that, again, it's a web service. I don't want to regenerate the Factory's list on every request, since we expect a fairly high number of requests and a high cost for creating at least some of the Handlers. In this case, it's far less developer effort to make the Factory a Singleton versus doing the actually efficient thing and caching it.

    Now, if I actually wanted to dynamically load Handlers by scanning a directory for assemblies, then yes- caching would be superior (it would load new Handlers on every cache expiration). But that would be really premature- the number of handlers is as-yet-unimplemented, but known.

    And as for testability, the configuration is managed by a class itself- something I can inject inside of a unit test.
  • Remy Porter 2012-09-17 23:13
    Also: this thread is essentially a textbook on why you can't design an object model without carefully defining your problem domain.
  • Dave Insurgent 2012-09-17 23:23
    Dude, you're over-thinking it to your detriment.


    Aside from the fact that I don't believe I'm over thinking it, because I'm specifically advocating design decisions that are reasonably well established, I don't understand at what point I've suffered harm or loss even if I was over-thinking it.

    You ask this question:


    The following quoted block of text was not a question.

    I want to model that physical aspect of the "world", then YES, I DO IN FACT want the Player to "know" or "have knowledge" of the sound.


    I think you're possibly confused with my use of "know" or "have knowledge" - it's not about being able to percieve the sound as a matter of game logic. It's about the player class, the actual code, being the one responsible for making the call to the sound system, or service, or whatever, to play the sound. The coupling between whatever it is you want a Player object/class to represent, and the fact that sound can be generated, is unwise based on numerous software engineering princples.

    In particular its duration.


    Why did you call this out specifically? You really want the Player instance to store the duration of a particular sound? Each and every sound that the Player may make? For what purpose? If I was right earlier in your confusion, you may mean to keep track of how long a "noise" has been heard, for some game rule calculation. I don't have anything to say about that, because it's all about what you want to do with it.

    I really don't see how bringing up data-driven vs. inheritance to model abstract concepts in a system is over-thinking it, not to mention...

    Decouple it then, as you have stated, with events or some other mechanism that provides a "mediator" as it were in between the Player and the sound.


    After being seemingly critical of what I'm suggesting, you go on to agree with me?

    Think outside the box man, and always know what the design goals are before trying to apply a so-called solution.


    Well, duh - we're talking about abstract concepts and their general application; I don't understand how you object to this. All sorts of discussion happens around the various merits of certain design patterns and other techniques such as dependency injection, without fixing to a specific "design goal" (except that the "design goal" is generally assumed to start with "design software that doesn't suck, which [...]" and there are a number of things that help with that first part.

    Try to avoid global state. Wait, I don't know what your desing goals are.

    I would argue that the player SHOULD in fact "know" or better stated, HEAR, the sound of the weapons. This makes stealth missions possible.


    This is not what we're talking about. We're not talking about the ability for sound that exists in the environment to be detected by other things in that environment. We're talking about the player [i]causing the sound to play directly by a code call as opposed to indirectly by event-driven architecture[i].

    ALL WEAPONS make a sound. Think about it. If in a vacuum, the weapon's attributes such as amplitude and duration can simply be zeroed out, without need for a tri-state situation involving nulls, nor funky edge cases.


    Where is the tri-state null edge case? The weapon shooting emits an event indicating as such. The game engine has various subsystems that observe the event and alter the game accordingly: the rendering subsystem displays a muzzle flash. the audio subsystem plays a "bang". the environmental subsystem alerts nearby enemies. the physics subsystem produces a projectile that causes damage to a crate.
  • Dave Insurgent 2012-09-17 23:33
    In retrospect, perhaps the confusion, if I'm right about it, was because I talked about "sound" which is possibly ambiguous (although I thought not because we were talking about code) between the sound the player of the game hears in the real world and the sound the Player could hear in the game world. When I say the game object should not "have knowledge of sound" I meant "have knowledge of the audio subsystem used to produce sound in the real world".
  • George 2012-09-17 23:41
    This is a pattern known as 'the Schizophrenic Singleton'.
  • Hal 2012-09-17 23:45
    True enough. Where ever you go, there you are.
  • scooby509 2012-09-18 01:04
    Mcoder:
    Why don't languages already come with a singleton modifier? It would remove an entire class of WTFs from the face of Earth (and create another entire and more entretaining class, so we can laugh more).


    Singleton is a class of WTFs, it should be called the Bottleneck pattern. You can easily waste far more cycles waiting for the damned singleton than you would creating a new instance each time.

    Just use thread locals:

    public class Service {
    
    private final static Service instance = new ThreadLocal<Service>();
    public final static getService() {
    Service inst = instance.get();
    if (inst == null) instance.set(inst = new Service());
    return inst;
    }
    }

    And unless you're an asshole and pass it to another thread, you never have to worry about locking.
  • inhibeo 2012-09-18 02:12
    Remy Porter:
    And you should avoid using the Singleton pattern. It's one of those patterns that's so easy to implement and seems so useful that you're tempted to create a bunch of them, but they should be used rarely.


    I have now resolved to call it the Simpleton pattern.
  • McFly 2012-09-18 02:13
    snoofle:
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!


    Obviously that appears to be the case: a) You have to explicitly call the default constructor (or else getInstance will return null) and b) what is even worse you might choose to call the default constructor anywhere in your code changing the instance in your wake (not so unique is it), leading to inconsistent behaviour or worse.
  • delenit 2012-09-18 02:18
    Slapout:
    @Deprecated:
    PS., Although I do not like replies that contain a singleton 'this', I will make an exception in this case.


    Shouldn't that be throw an exception in this case? :-)


    I have made an exception for you. Pray that I do not throw it.
  • Errorsatz 2012-09-18 03:42
    I've actually seen this type of pattern before, and it does have a legitimate use - although technically it may not qualify as a singleton. Let's say you have an object which there is only one of at a given time, but the object can vary over the course of the program. For example, the player avatar in a game, which could vary from a person to ship to whatever as you go through different levels. Now technically yes, there are different ways you could accomplish this that conform to patterns better.

    However, that brings me to my second point. Games, especially small games, have different requirements from "enterprise" projects. When you're making a game, you're not trying to create every possible design, you're trying to create this design. Trying to make your side-scroller also have the capability to be a FPS, puzzle game, and sandbox - when those aren't part of the design - is more of a liability than a benefit. You're also not (generally) trying to create an engine that will last decades.

    Personally, I've found that trying to anticipate every possible feature leads to a mess of excess code that's harder to maintain, and you still end up missing something. Make the code work for the actual design, and refactor as necessary. And yes, that means that when something is limited to a single instance in the design, it can be a single instance in the code as well.
  • QJo 2012-09-18 03:53
    Garrison Fiord:
    Garrison Fiord:
    Over the years I've read from this site, I've read countless comments criticizing the editors. I've read offensive and possibly-illegal posts by Zunesis. I don't know why I'm suddenly being singled out for harassment.

    Fixed. Yes, now I don't contribute, since my comment is going to be deleted anyway. I'm not going to register so you can spam my email address either.

    For the record, I have never been spammed as a result of having registered on this site.
  • QJo 2012-09-18 03:57
    RandomGuy:
    Erik:
    TRWTF is the singleton pattern!

    Could not agree more.
    Global variables are bad and hiding them in a singleton does not make them better.

    Reminds me of the sheep in animal farm: "Local variables good! Global variables bad!"
  • wgeek 2012-09-18 04:19
    What I "object to" as you say, is your blind zeal for all things event driven, presumably your cure-all for all things that may resemble direct coupling.

    And to make your point, you then post a contrived, even ridiculous example that doesn't show coupling per se, it shows INLINING of the sound code in the relevant object.

    You don't bother to even CONSIDER the usage of a base class with virtual methods?

    Events are nothing more than the Observer design pattern, and they come with many shortcomings as opposed to using a more "direct coupled" approach (by way of polymorphism, NOT your naive inlining of code into ridiculously named class methods).

    Blindly advocating an event-driven architecture without stating the features and benefits strikes me as naivete; not good engineering.

    Carry on.
  • wgeek 2012-09-18 04:27
    Errorsatz:
    I've actually seen this type of pattern before, and it does have a legitimate use - although technically it may not qualify as a singleton. Let's say you have an object which there is only one of at a given time, but the object can vary over the course of the program. For example, the player avatar in a game, which could vary from a person to ship to whatever as you go through different levels. Now technically yes, there are different ways you could accomplish this that conform to patterns better.

    However, that brings me to my second point. Games, especially small games, have different requirements from "enterprise" projects. When you're making a game, you're not trying to create every possible design, you're trying to create this design. Trying to make your side-scroller also have the capability to be a FPS, puzzle game, and sandbox - when those aren't part of the design - is more of a liability than a benefit. You're also not (generally) trying to create an engine that will last decades.

    Personally, I've found that trying to anticipate every possible feature leads to a mess of excess code that's harder to maintain, and you still end up missing something. Make the code work for the actual design, and refactor as necessary. And yes, that means that when something is limited to a single instance in the design, it can be a single instance in the code as well.

    I concur.

    Event-driven architectures are a pain in the ass to debug.

    The concept of a publisher/subscriber is not readily expressed outside of meta-attributes, and can lead to a whole mess of other issues and very difficult to follow runtime behavior.

    They are great if the goal is to, as you say, allow for future expansion OR dynamic publish/subscribe types of things.

    But if the constraints of the game are in place, pure inheritance type of - GASP - coupling is often the better choice in terms of modelling the intent and ease of debugging.

    Singleton's are legitimate and simple ways to model one instance of an object: such as a printer, a screen, or an ini file. Only one such object can ever exist, and its state is shared. Model it with a Singleton then.

  • Jack Mathews 2012-09-18 04:29
    Took me a minute to get it too.

    Since the coor is public and there is no verification on setting the instance, any number of these objects can get created and silently become the new global instance.
  • wgeek 2012-09-18 04:41
    QJo:
    RandomGuy:
    Erik:
    TRWTF is the singleton pattern!

    Could not agree more.
    Global variables are bad and hiding them in a singleton does not make them better.

    Reminds me of the sheep in animal farm: "Local variables good! Global variables bad!"

    Or "Coupling bad"....

    Hey, global scope is global scope if the entire program needs access to it! This happens! Use namespaces for crying out loud: that's why they exist.

    And coupling is not in and of itself bad. It is INFLEXIBLE. But that does not equate to BAD. Even in the enterprise. IF that aspect of the design will not and cannot change, then couple it!

    It will be easier to maintain and debug down the road.
  • Leak 2012-09-18 04:43
    scooby509:
    Just use thread locals:
    [...]
    And unless you're an asshole and pass it to another thread, you never have to worry about locking.

    Good luck doing that with anything that accesses a non-shared resource, like a log file.

    It's called a singleton because there is only one instance of it across the whole application...
  • Randy Snicker 2012-09-18 04:53
    Dave Insurgent:
    What about Equipment that can only be used once? Is that a Consumable? Or does Consumable imply that the player actually eats it?


    Consumable is a bad name, because you get this sort of problems when people don't agree whether it means something that gets used up, something edible or something that a person stuff down their throats (but possibly shouldn't). You should know whether you're playing with data structures or with game objects.

    Dave Insurgent:
    What then is the property that actually causes those objects to diminsh? Do you call them Finite? Is a Sword a piece of Equipment that implements Repairable, Damaging and so on? It seems appealing to try to categorize things that way, but I think it's a lot easier to manage when instead you think that a Sword is an Item with a Set<Attribute>.


    Well, no one told you to be an idiot about it.
    Inheritance shouldn't be used as a labeling tool for abstract properties. Inheritance should be used to define the methods (and implementations) of use. Programming isn't philosophy, programming is engineering.
    There shouldn't be a class called Damaging. Instead, you could have a Weapon class defining an interface functions (and possibly subclasses) for different ways of using something as a weapon.
  • linepro 2012-09-18 05:04
    Aah the simpleton pattern. I remember it well...
  • David 2012-09-18 05:11
    jay:
    But that doesn't mean that global data is never appropriate. It makes good sense for data that legitimately applies all over the place and which there should be only one copy of. Like, what's the user's timezone? Many modules might use that. Rather than pass it in and out of 1000 places, it makes sense to keep it in a global somewhere. Or user preferences. Yes, we could load preferences in a start-up function and then pass them to everything, but it's a pain and it gains nothing.


    This is the first comment that Ive fully agreed with.

    Programming is a form of modelling, and the whole point of modelling is to extract and use those properties that are relevant while ignoring those that are not.

    Ignoring relevant properties is as dangerous to the sanity of the code and the programmer as is larding the app with unneccessary cruft purely in order to conform to a particular dogma.

    If your methodology completely forbids a particular structure - GOTOs, global variables, doesnt matter - then the case *will* one day arise where it is simply *wrong*.

    And how you deal with that case makes all the difference. Do you accept that every methodology has its limits, and switch codes? Or do you mash the problem until the square edges are ground off and it will, with some hammering, fit through that round hole?

    I suspect (without proof) that more WTFs arise form the second case than the first.
  • Maurizio 2012-09-18 05:16
    And why you need a single SoundManager instance ?
    Wouldn't make more sense to have SoundManager instances that deals with different kind of events/sounds, depending on the complexity of the application ?

    And once there, wouldn't make sense to do not call it SoundManager, since sound manager is a low level task, while what you want to handle are application events, not sounds as such.
  • Watson 2012-09-18 05:19
    jay:
    Remy Porter:
    And you should avoid using the Singleton pattern. It's one of those patterns that's so easy to implement and seems so useful that you're tempted to create a bunch of them, but they should be used rarely.


    I think there's a missing step in this (no pun intended) logic.

    As stated, the best I can make of this argument is, "This thing is easy to use and is very useful, therefore you shouldn't use it."


    I think there is a missing step in this logic: the one where you get from "seems useful" to "is useful".
  • Ahmad Alhaj Hussein 2012-09-18 06:10
    Using the Enum Singleton are better in Java than double checking Singleton because creation of Enum instance are easy to write and thread-safe
    ///////Easy Singleton
    public enum EasySingleton{
    INSTANCE;
    }
    ///////Complex Singleton
    public class DoubleCheckedSingleton{
    private DoubleCheckedSingleton INSTANCE;
    private DoubleCheckedSingleton(){}

    public DoubleCheckedSingleton getInstance(){
    if(INSTANCE == null){
    synchronized(DoubleCheckedSingleton.class){
    //double checking Singleton instance
    if(INSTANCE == null){
    INSTANCE = new DoubleCheckedSingleton();
    }
    }
    }
    return INSTANCE;
    }
    }
  • brian banana 2012-09-18 07:07
    thx for the info!
    actually most of us already knew that.
    we do not come here for solutions,
    but for wtfs. you just outed yourself a noob.
    but it's ok. we're all noobs!
  • TheCPUWizard. 2012-09-18 07:12
    For those discussing the Singleton pattern.... here is a great question (I love to give it on interviews...)...

    How do you determine if a Singleton pattern or a Static class is the appropriate choice for a given situation?
  • CustardCat 2012-09-18 07:17
    Isn't the getInstance() supposed to have some kind of:

    if (instance == null) instance = new KpiService();

    though?
  • CustardCat 2012-09-18 07:20
    Oop!

    Caught out by the Featured Comments thing again..

  • Jack 2012-09-18 07:40
    What's wrong is that the singleton implementation depends on some code somewhere calling the object constructor first to establish the static singleton. And if any code anywhere else calls the constructor the global singleton value is changed. So, the value of the singleton is unpredictable and possible not present. While this may work and you may get away with it, it will eventually bite you in the butt.
  • Severity One 2012-09-18 08:23
    glopal:
    The getInstance method should instantiate the instance variable using the constructor, which should be private. No?
    No.

    If you create the instance when the getInstance() method is invoked, you need to make sure that it wasn't initialised before. And because of thread-safety, the check must be put in a 'synchronized' block, which synchronises against the singleton's class object. That's an expensive operation in terms of CPU time.

    In other words, the instance should be created statically, which in Java means it still gets constructed only when accessed for the first time.
  • Severity One 2012-09-18 08:36
    TheSHEEEP:
    The question is not why I would want to allow something, the question is why would I want to limit something that does no harm?

    [...]

    I am absolutely convinced that a simple SoundManager singleton would be far easier to understand than the complex architecture you suggest. And it would also be much faster, an event system would simply introduce clutter.

    [...]

    I get the idea, but that would be an overly complex solution to a very easy problem.
    You know, we have a nice saying in our office: "You can solve any problem by adding another layer of indirection. Except too many levels of indirection."
    Some people should really take that to heart. ;)
    The problem is that you're confusing ease and simplicity. They're not the same thing. The solution you've chosen is the easiest, but not necessarily the simplest - simplest in terms of transparency, maintainability, and all the other things that are a factor in software development.

    A singleton is a global variable - no more, no less. There are certain things that can't really be solved in a different way than using a static/global/singleton, but if you use it as some sort of magic bullet, you're probably going to, at some point or other, run into the same problems that global variables have caused over the past decades.

    TheSHEEEP:
    Also, people seem to be stuck on that sound example.
    Let's introduce another one: Logging. In the same game (and in all other applications I wrote and most I encountered) I had a Logging singleton class.
    Now Logging is really something you want to do from possibly everywhere without having to rewrite your class hierarchy/structure.

    And the possibility to do so from everywhere also doesn't break or harm anything. Except threaded stuff, but you can always make that Singleton thread-safe.
    I find myself passing instances of (log4j) Logger objects around, because of the way that support can dynamically set the location of configuration files, including the log4j.properties file. There's no neat way around that.

    But the point that people are trying to make is that your design may suck, pardon my French. Maybe it's set up in such a way that you don't have an application context, or some other way to access the sound system.
  • Zecc 2012-09-18 09:02
    Beta:
    A new design pattern: Null Class, when only zero (at most) instances of the class may exist at a time.
    I use this pattern all the time. The best thing about it is, I don't even need to be write code to implement it.
  • no laughing matter 2012-09-18 09:15
    And you provided a fine proof why singletons are such a WTF: You will use them in situations where they are not appropriate.
    wgeek:

    Singleton's are legitimate and simple ways to model one instance of an object: such as a printer, a screen, or an ini file. Only one such object can ever exist, and its state is shared. Model it with a Singleton then.

    At my workplace two displays are connected to my PC. Even the simple notepad accessory allows me to select the printer to which i want to print (and lists several printers including a fax-printer and a "virtual printer" which creates a PDF-file).

    And why should there by only one ini file per application?

    At least there should be a separation between global settings and user settings.

    In exactly NONE of your examples a singleton was a good design choice.
  • xtremezone 2012-09-18 09:31
    jay:
    But that doesn't mean that global data is never appropriate. It makes good sense for data that legitimately applies all over the place and which there should be only one copy of. Like, what's the user's timezone? Many modules might use that. Rather than pass it in and out of 1000 places, it makes sense to keep it in a global somewhere. Or user preferences. Yes, we could load preferences in a start-up function and then pass them to everything, but it's a pain and it gains nothing.

    What's the difference? When data applies to the application as a whole, it's legimately global. When it applies to one particular thing we're doing right now, it is not legimately global. What is the current user's name? Fair game for global. What is the name of the customer on the transaction we are currently processing? Not fair game for global at all.
    The point we're trying to make is that you shouldn't be needing the timezone or user preferences in 1000 places. ;) You don't want worker modules knowing directly about where that data is coming from. You want to pass it in so that, for example, if you need the identical functionality with custom inputs you can simply change what you pass to them. If you do decide to cheat and use globals in this fashion then I would prefer you to wrap it up in a factory to hide it from the code (i.e., pass global parameters into the object's constructor with a factory).
  • Bernie The Bernie 2012-09-18 10:06
    Oh, that type of Singleton is actually a RefreshableSingleton (TM).
    Whenever your Singleton grew old and bad, just call the constructor, and get a fresh one!
    Only too bad that the Gang of Four forgot to describe it among their "design patterns". But now you know it, and you'll love to use it.
  • Dave Insurgent 2012-09-18 10:42
    Games, especially small games, have different requirements from "enterprise" projects.


    Not disputing that. My original comment specifically called out professional games, and I also started out with declaring that I'm not a professional game developer, meaning I was trying to say "Hey, so I heard of this, and it sounds really neat because it solves this problem that experts in the field say comes up very frequently and I hadn't even thought of it that way before until I read it. Maybe you would enjoy it too" - and the poster I replied to, in fact, did. So there's that.

    and refactor as necessary


    Do you agree or disagree that certain up-front decisions may impact that ability to do that? The experience of the leading minds in our field seem to be, for the most part, a collective "yes" - I base this on the scientific aspect of the field as well as the publications of many authors.

    is your blind zeal for all things event driven


    "blind zeal" is a bit much, don't you think? You're basing that on a few comments I made that are all specific to game engine design, in which I led them with a statement indicating that I was not a professional, but had seen a few professional discussions about some problems in game engine design and an interesting alternative approach. I could just as easily call you a "blind zealot" of object-oriented design. What good is that to the discussion?

    it shows INLINING of the sound code in the relevant object.


    That was how it was brought up. The discussion, if you cared to follow it before bringing your loaded gun to the comment box, was:

    - Singletons should be generally avoided [except where experience/need proves otherwise: which is a caveat in every freaking guideline that you seem to want spelled out each time otherwise it is 'zeal']

    - I used a Singleton here, because I needed to call it

    - Do you think that your class should call that Singleton directly? Since there will be many classes that would call that Singleton, there may be a way to break that specific part down in to some other reusable component. With the - seemingly reasonable - assmumption that most of us here are aleady object-oriented developers, I'd like to also highlight some ways that the object-oriented paradigm can fall short, I learned about this not by experience, which I'm basically sorry to say, but by reading about the experience of others who work in the field.

    - How dare you have such zeal as to advocate for an alternate programming paradigm? There is no data-driven, there is no responsibility-driven, there is only Zuul, I mean, object-oriented!

    Events are nothing more than the Observer design pattern, and they come with many shortcomings as opposed to using a more "direct coupled" approach


    They have inverse ownership of the observation, meanign that the item being observed doesn't have to maintain a list. They're obviously different, although I agree that share semantic qualities. What the hell is your point? You gave me shit for not listing pros/cons, but you just said "many shortcomings" without elaborating. I at least pointed out that if the weapon/whatever is responsible for playing sounds, then you have to ensure that behaviour everywhere as a separate act of maintenence, and re-using it via inheritance will likely lead to a bloated, confusing, if not impossible object hierarchy.

    The eventing was an auxillary point, the main point was that having the Weapon know about the SoundManager is an unwise coupling. That wasn't "all coupling is bad, rah!" - and the jump wasn't from "it is coupling, therefore it is bad", it was "it is coupling, and it seems unnecessary, therefore it may be bad".

    (by way of polymorphism, NOT your naive inlining of code into ridiculously named class methods)


    I think you meant to type "separation of responsibilty and data in to meaningful elements by approaching the problem using a different paradigm for the sake of discussion" - it's alright, the keys are like right next to each other.

    Programming isn't philosophy, programming is engineering.


    Well, engineering is a branch of philosophy, just like science. I'd argue the difference is that engineering has little to no tolerance for experimentation, which belongs in science and then gets merged in to engineering after sufficient testing/examination. What's the point? You still use philosophy to practice engineering. Just a refined subset of it.

    There shouldn't be a class called Damaging


    Why not? You don't know my problem domain any better than I know yours. I'm postulating ways to define a robust language for expressing many different configurations that could happen within a game engine. Not a specific application (a game), but the engine - which I'd like to reuse if possible, because I don't like rewriting the same crap every day.

    I like the idea of their being a Damaging attribute (it might not be an actual class definition either: it could be a name of a composite definition of other attributes, but that's not important right now). A Weapon is Damaging. Is a Thruster? I can apply the Damaging attribute to both, so that when the Thruster engages, anybody behind them gets burnt.

    We're not arguing about class naming conventions or object hierarchy, because I get what you're trying to do and it works when it works and it doesn't when it doesn't. I'm illustating that there is a different paradigm for how you model the problem - and that, based on the experience of some professionals, solves some issues that crop up in a neat way.

    Instead, you could have a Weapon class defining an interface functions (and possibly subclasses) for different ways of using something as a weapon.


    I already addressed that point: It works for simple games, but my point was that as games increase in complexity then seem to run in to an awful mess of an inheritance tree, and sometimes end up not being able to model certain things at all.

    You can read more about it here which has aggregated a number of sources on the subject.
  • AGray 2012-09-18 11:34
    Easy folks! Dave there was just saying that instead of building RPG Items around inheritance, there's an alternative. After sleeping on it, I may or may not agree with *all* of it, but, hey. I was always told,

    [quote="Generic Programming Professor"]"If you find just one way to solve a programming problem, it's proof you didn't think hard enough."[/quote]

    As far as game settings are concerned, I took a long careful look at my code last night. Using a singleton and using a Settings class instance that starts up immediately when the game does and hangs around in the background offer absolutely no difference; given that I only want one settings repository, though, it's a case where a singleton does wind up making sense. I don't need a plethora of Settings repositories hanging around all over the place.

    The reason that having a repository remains desirable to my project, as opposed to having the game objects themselves host the values directly, is that one scene's Maestro and PlayerControl scripts are discarded across scene boundaries by necessity; while these can be preserved across scene boundaries, it's a prohibitive level of work to do that effectively. The effort just outweighs the rewards. It means I have coupling going on, but honestly? I'm not put off by that. As you'll see below, I've devised some 'unit tests' that take advantage of that.

    How the repository is being defined in this case does not matter, and will honestly not make the code any easier or harder to test; a Unit Test will have a hard time determining whether sound for a GUI element click or background music really is softer or louder...after all, internally, those volume levels are all floats; the unit test at best can only see that the Maestro is correctly conveying the settings to the AudioSource that the Maestro is attached to, but other than that, a user has to manually listen while manipulating settings and mentally ask themselves, "When I slid the Master Volume slider 'downwards', did all sounds become quieter?"

    If I absolutely need to perform a Unit Test in the background, the Unity game engine provides me more difficulty than the Singleton actually does. I'll have to make a test scene that runs my unit tests, for starters - no NUnit for me! :( However, my unit tests will flow somewhat like this:

    [code][Setup]
    Create new SettingsGUI instance.

    [Test]
    Set Settings.MasterVolume = (value)
    Perform synchronization action from instance of SettingsGUI.
    Does AudioListener.volume = Settings.MasterVolume? (P/F)

    [Test]
    Set Perform graphics quality action from instance of Settings GUI.
    Does QualitySettings.level = Settings.graphicsLevel? (P/F)[/quote]

    ...and so on. Actually, in some ways, given Unity's quirkiness...a Unit Test with this singleton isn't too hard to pull off, given that it's acting as a repository.

    So, I agree that testability matters - I have means to create a form of unit testing. I can tie [action] to [business rule].

    I agree that portability matters. I can take my code, apply the Settings GUI to another project, bring over the Settings repository, Maestro, Player Control scripts, and/or even other GUIs, and this stuff will still work with minimal modifications.

    On the other hand, I still agree about multiprocessing and multithreading. I stand behind what was said yesterday: singletons are not threadsafe, and have a small window for a race condition to occur. That being said, that's also not how I'm using this Settings construct. YAGNI still applies!

    (Rest assured though, I do not defend using a Singleton in such an environment; it's the wrong tool for the job.)

    I like this discussion, since it's forcing me not just to justify my design decisions to other people (and yes, I too carefully considered/designed the use of singletons in my project before implementing,) but also to myself. I am a younger programmer (probably obvious), but one thing that is clear to me, is at the end of an implementation, I have to ask myself, "am I comfortable with this? Is this obviously inferior to the alternatives that I have been made aware of?"

    In the case of the Settings? The answer turns out to be 'no,' in the context of my current techniques and the limitations of the engine I'm using, and the bounds of my problem domain (create a JRPG, not a game engine sitting atop a game engine.)
  • Sten 2012-09-18 12:12
    snoofle:
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!


    Wrong is that getInstance() should create the singleton if it does not exist and that the singleton should not be created by anything else which it obviously is
  • danielkzu 2012-09-18 12:21
    That's pretty much how lots of "ambient singletons" work (i.e. Transaction.Current, HttpContext.Current, etc.).

    There's nothing wrong with the approach.
  • Guardia 2012-09-18 13:18
    szeryf:
    It's wrong because the return statement isn't properly commented. There, I fixed it:


    public static KpiService getInstance() {
    // That's this.
    return instance;
    }


    Your comment needed some work.
  • jay 2012-09-18 17:30
    xtremezone:
    jay:
    But that doesn't mean that global data is never appropriate. It makes good sense for data that legitimately applies all over the place and which there should be only one copy of. Like, what's the user's timezone? Many modules might use that. Rather than pass it in and out of 1000 places, it makes sense to keep it in a global somewhere. Or user preferences. Yes, we could load preferences in a start-up function and then pass them to everything, but it's a pain and it gains nothing.

    What's the difference? When data applies to the application as a whole, it's legimately global. When it applies to one particular thing we're doing right now, it is not legimately global. What is the current user's name? Fair game for global. What is the name of the customer on the transaction we are currently processing? Not fair game for global at all.
    The point we're trying to make is that you shouldn't be needing the timezone or user preferences in 1000 places. ;) You don't want worker modules knowing directly about where that data is coming from. You want to pass it in so that, for example, if you need the identical functionality with custom inputs you can simply change what you pass to them. If you do decide to cheat and use globals in this fashion then I would prefer you to wrap it up in a factory to hide it from the code (i.e., pass global parameters into the object's constructor with a factory).


    Wow, I never thought I'd be defending global data. Usually I have to explain to people why it's bad.

    But for the case where it's good: Let's take a concrete example. Suppose we allow the user to select whether he wants large icons or small icons in his menu bars. We have a preferences screen where he can select this, and we save it from run to run.

    Yes, we COULD say that at start-up time we call a "load preferences" function that reads the preferences and returns, presumably among many other things, the large icon vs small icon indicator. Then we could pass this indicator to every function that paints a screen. Of course we'd also have to pass it to every function that calls a function that paints a screen. I think you can easily see that we would be passing this indicator around everywhere.

    Why? What is gained by doing this? It just clutters up our parameter lists. The whole idea of a preference like this is that it applies everywhere: we're not going to draw screen A with large icons and screen B with small icons. We WANT the icon preference to have only one value everywhere in the system.

    What if function A does not paint a screen today, so I don't pass in the parameter. Then someone comes along and modifies it so now it does paint a screen. Now he has to not only add this parameter to function A, but to every function that calls function A, and to every function that calls one of those functions, etc.

    If this was the only "applies all over the place" data in the system, it might not be a big deal: one extra parameter on every call. But in real life we could have many such things. Are we going to add twenty parameters to every function call? I suppose we could create an "applies all over the place" object and put all these things in there, and then pass this object around. But then all we've accomplished is to do a bunch of extra work to simulate global data.

    Let me make clear that there is a very narrow set of things that are rightly global data, namely, data that applies all over the system, and if it is changed, we want that new value to be used all over the system.

    Of course MOST data that a function uses is only applicable in this specific place. I've written elsewhere about the horrors of global data.
  • Dave Insurgent 2012-09-18 20:32
    Are we going to add twenty parameters to every function call?


    Well, this is why separation of concerns gets so much attention: What function are you imaginging that has so many responsibilities that it needs twenty parameters of various things?

    Your WindowManager might have an IconFactory that can be used to create an Icon. There are two implementations of the IconFactory: LargeIconFactory and SmallIconFactory.

    Now, odds are the WindowManager wouldn't directly be responsible for the creation if Icons, but rather some kind of Pane or View or Control, the reason being is that Icons aren't necessarily a guaranteed part of the UI. Maybe you want just a flat list - but in creating the [Foo] that places Icons on the screen, you would supply the IconFactory to it.

    I think one important thing to consider is that classes are no different than closures in that they both provide calling context to a function, which means that context around a function call beyond the parameters list isn't "wrng" and so the issue isn't that global state surrounds a function call with extraneous context. That context, if mutable/stateful, provides it's own problems, but there are ways of dealing with that. The anti-singleton crew gets most of its juice from the kind of examples of narrow-sightedness that people like wgeek pose: Hey, there's only one display. Is there? Not on my machine.

    we could create an "applies all over the place" object and put all these things in there,


    Right, which means things go the route of some kind of ApplicationContext. That's not so bad, but the key thing is that you can have more than one. With the Singleton, the very definition of what an ApplicationContext is becomes bound to the notion that there is only one.

    Now, the pragmatic side of the issue is that you shouldn't go putting a bunch of stuff in to satisfy requirements that are undefined. The experienced side knows that this is true, but those same requirements are prone to change and there are reasonable ways of anticipating that change.

    Global, there-can-only-be-one, kind of state is one of those things that hardline pragmatical people say, "fuck it, I'll fix it when I have to" and the leaders of the field say "that's more likely than not, so why not do it differently if there are no other consequences?"

    This may upset a few people but I would frame the issue this way:


    Foo.doSomething() {
    Bar.getInstance().doSomethingElse()
    }


    is both a dependency on the use of Bar as well as directly knowledge of how a Bar is constructed, not really different than new Bar().doSomethingElse() except that the convention implies that there's only one Bar, ever.

    The fact that a Bar is constructed somehow is implicit:


    Foo.doSomething(bar) {
    bar.doSomethingElse()
    }


    or


    Foo.doSomething() {
    this.bar.doSomethingElse()
    }


    both use a bar that exists - but don't need to know how it came in to existence. Bar would possibly be provided as part of the Foo constructor (or via a setter), and we Know This Is Good because of the DIP part of SOLID.

    Obviously there are simpler components that don't need to be lifecycle managed, no one objects to new String() or whatever, and I don't think it's just primitives. The point is that anything you're trying to define as unique, there can only be one, probably has a little more to it than the time. In fact, Calendar isn't a singleton - it's a static factory. I don't see why TimeZone needs to be either.
  • djd 2012-09-19 01:55
    The problem here (I think, I haven't been doing C++ very long) is that getInstance is static meaning it can be invoked BEFORE any instance is actually constructed, at which point it will return null.

    Is the intent to allow access of the singleton without needing to use its name? Even if that didn't create a bug, why would it be useful?
  • Flo 2012-09-19 04:49
    The problem is that the object is instanciated in the constructor. But the classic way to get a singleton is to invoke the static method getInstance(). You never invoke the constructor (because you never directly create new objects. It is one aim of the singleton pattern...).
    So at the first invocation of getInstance(), instance is null.
    And it remains null, unless somebody invokes new()...
    That is obvious, but it took me a certain amount of time to see where the problem was, because it is so crazy! This is a real WTF!
  • Neil 2012-09-19 06:24
    Fakeyede Mobolaji:
    return instance ?? instance = new KpiService();
    I've always wondered whether there should be an ??= (or ||= in other languages) operator:
    return instance ??= new KpiService();
  • Randy Snicker 2012-09-19 10:18
    Dave Insurgent:
    Programming isn't philosophy, programming is engineering.


    Well, engineering is a branch of philosophy, just like science. I'd argue the difference is that engineering has little to no tolerance for experimentation, which belongs in science and then gets merged in to engineering after sufficient testing/examination. What's the point? You still use philosophy to practice engineering. Just a refined subset of it.


    Thank you for not attributing my quotes to me so that I get to play detective to find out whether you've answered to me or not.

    Sure, if you like to mince words and act like an idiot, engineering is philosophy. Although, anyone with half a brain should have seen that I clearly contrasted philosophy with engineering, which should tell a rational person that I used the terms in a context in which I specifically redefined them to be mutually exclusive. In addition, the word philosophy is probably most often used to describe the particular branch of science that doesn't fall under the common definitions of engineering or the other branches of science.

    I'd argue the difference is that engineering is about problem solving and making things work, whereas philosophy is a bunch of hipsters drinking red wine talking about something so meta that it has effectively no relevance to anything practical whatsoever.

    There shouldn't be a class called Damaging


    Why not? You don't know my problem domain any better than I know yours. I'm postulating ways to define a robust language for expressing many different configurations that could happen within a game engine. Not a specific application (a game), but the engine - which I'd like to reuse if possible, because I don't like rewriting the same crap every day.

    I like the idea of their being a Damaging attribute (it might not be an actual class definition either: it could be a name of a composite definition of other attributes, but that's not important right now). A Weapon is Damaging. Is a Thruster? I can apply the Damaging attribute to both, so that when the Thruster engages, anybody behind them gets burnt.

    We're not arguing about class naming conventions or object hierarchy, because I get what you're trying to do and it works when it works and it doesn't when it doesn't. I'm illustating that there is a different paradigm for how you model the problem - and that, based on the experience of some professionals, solves some issues that crop up in a neat way.

    Instead, you could have a Weapon class defining an interface functions (and possibly subclasses) for different ways of using something as a weapon.


    I already addressed that point: It works for simple games, but my point was that as games increase in complexity then seem to run in to an awful mess of an inheritance tree, and sometimes end up not being able to model certain things at all.


    Well, you "addressed" that point by coming up with a strawman idea of using classes as an overly general attribute definition and then proceeding to create rules based on those overbroad categories. Unsurprisingly, that idea turned out to be rather stupid.

    If your attribute definitions fall in the category of idle philosophy rather than that of methods of use, you're going to have the problem of overly complicated rules for interactions, which is exactly the problem you seemed to be arguing against earlier. Even if your goal is to not design a game but rather model a world with fully realistic and rationally emergent behaviour, you're going to have extremely tough time doing it if you don't define the objects in terms of their use - regardless of the specificity.

    Don't believe me? Ok, you want to know what is damaging? Everything in some way or another. It's a useless property to be used as a class definition, because it doesn't restrict in any way how something is damaging. Then again, if it does, it's badly named because the name doesn't reflect those specifics. Weapon, however, defines clear use cases on which more specific logic can be built on.
    Even game engines should focus on some specifics and useful naming conventions so that whatever the framework they provide includes some default logic and structures for basic interaction. Otherwise it's just massive waste of everyone's time.
  • Dave Insurgent 2012-09-19 11:21
    Randy Snicker:
    Sure, if you like to mince words and act like an idiot, engineering is philosophy


    Here I was thinking I said:

    Dave Insurgent:
    engineering is a branch of philosophy


    Not that they were equal.

    Randy Snicker:
    I clearly contrasted philosophy with engineering ... I specifically redefined them to be mutually exclusive.

    In addition, the word philosophy is probably most often used to describe the particular branch of science that doesn't fall under the common definitions of engineering or the other branches of science.

    I'd argue the difference is that engineering is about problem solving and making things work, whereas philosophy is a bunch of hipsters drinking red wine talking about something so meta that it has effectively no relevance to anything practical whatsoever.


    Philosophy is the study of general and fundamental problems. Philosophy is distinguished from other ways of addressing such problems by its critical, generally systematic approach and its reliance on rational argument.


    So you wanted to contrast "critical", "generically systematic" and "rational" with engineering? You should play more carefully with "idiot" and "half a brain". You have your relationship between science and philosophy backwards.

    Well, you "addressed" that point by coming up with a strawman idea of using classes as an overly general attribute definition and then proceeding to create rules based on those overbroad categories. Unsurprisingly, that idea turned out to be rather stupid.


    I also provided a reference to a number of sources from people in the industry related to the problem domain in question, stating that it is, in fact, a real problem. Not a straw man. But I know it makes you feel superior (perhaps while drinking white wine since red is for those hipster philosophers) to try to bring up the term as often as you can.

    If your attribute definitions fall in the category of idle philosophy rather than that of methods of use, you're going to have the problem of overly complicated rules for interactions, which is exactly the problem you seemed to be arguing against earlier.


    It's not idle philosophy. It's still scoped to the mechanics that the game engine wishes to model. Equipment, Damage, Health, and so on. Those "overly complicated rules for interactions" are created through composition and those definitions can be tested and maintained better than an object hierarchy.

    Even if your goal is to not design a game but rather model a world with fully realistic and rationally emergent behaviour, you're going to have extremely tough time doing it if you don't define the objects in terms of their use - regardless of the specificity.


    Good point! It shows you didn't bother to read anything I linked and continue on your "object oriented is the only way to think about any given problem" way.

    Don't believe me? Ok, you want to know what is damaging? Everything in some way or another. It's a useless property to be used as a class definition, because it doesn't restrict in any way how something is damaging. Then again, if it does, it's badly named because the name doesn't reflect those specifics.


    Except that you could restrict it by virtue of the class definition of Damaging. The physics subsystem might emit collision events which have the Damaging attribute with a type - physical. It could possibly inherit the type from whatever it collided with. A Weapon could have the Damaging attribute with a type of magic. It could be fire, or poison, or whatever. That's where, you're not wrong that you have to have some definition of what problem you're trying to solve. But it's very, very difficult to model "A weapon that causes fire damage and can only be held with two hands" using inheritance.

    Weapon, however, defines clear use cases on which more specific logic can be built on.


    Yep! And that specific logic is sometimes mutually exclusive in an is-a approach, which is why inheritance becomes very messy. I'm not saying you can't make Weapon. It may make sense. But where do you stop? Is Bow a Weapon? Is Crossbow a Bow? Does Bow implement Pluckable? Does the Crossbow implement some properties of a Gun interface such as .reload()? If you do make Weapon, you can give it the Damaging attribute as an explicit definition of what is a weapon. You've still used composition and a has-a approach to giving the weapon the property of causing damage as opposed to trying to figure out how much of a weapon is a damage causing thing. Those decisions are both philosophical and engineering based.

    It's appears you don't understand some portions of software engineering / computer science that I'm talking about. That's OK, I mean, I don't think any of us understand all of it, but you're not very nice about it. You seem like a fairly intelligent person, though prone to outbursts which probably impacts your social life. It would benefit your career to take a look beyond object-oriented programming as the only way you approach problems. That doesn't mean stop using it. Just put another tool on your tool belt. And be a little nicer to people.

    The best part is that it's still actually object-oriented programming, so you should have figured it out. The "data oriented" part is a design decision.

    The first thing you can do towards bettering yourself as a professional (the being nice part is up to you) is read some good sources about what I'm advocating. I'm still well within the realm of not "composing everything" - data-oriented design specifically pushes the limit of what constitutes composing everything, but I'm not acting radically here at all, as I am supported by the link (and subsequent links it contains) I provided earlier.

    Good luck with your career!
  • Dave Insurgent 2012-09-19 11:32
    I'd also like to add that a difference between you and I is that I brought up an idea - I proposed a problem, supported the existence of that problem with evidence, and showed how people who have had to deal with that problem "for real" have used that idea to solve the problem. All that does is give you another problem-solving tool to choose from. You're effectively attempting to suppress the existence and use of that tool absolutely based on rigid adherence to the use of another tool. Not supported by evidence. In fact, in direct opposition to evidence supplied, which is unreasonable.

    That's not engineering or philosophy, even with your asinine redefinition.
  • qbolec 2012-09-19 13:40
    I have read all comments and found many of them interesting but I really need to add my two cents.

    1. It is easy to fix all problems submitter had with the offending code by simply adding new KpiSingleton call in the same file in which the class is defined. Depending on the language this may require special care to make sure there is only one call resultimg fromthis single line but I am pretty sure that any of you can figure out the needed combination of static, final, etc keywords.

    2. The submitter's attempt to fix a problem by calling constructor on something which has Singleton in its name is stupid.

    3. The idea of putting Singleton in the name of a class means that all pkaces where you use it is forced to know that it is a singleton. Same goes for naming the function getSingleton instead of more general getInstance.

    4. I see no difficulties in unit testing code which uses KpiSingleton. Simply inherit from it, add mocking functionality to derived class nd call its constructor, which as we can see will replace the instance with mock.

    5. I believe that if you need to resort to making something private to make sure there is only one instance of something, then you put too much trust in code and too litle in coworkers. Where I work it is common to change private to public if there is a good reason for it, and there are some things we do not do even though they are possible.

    6. Singleton is not equal to global state. You can have singletons which are not globally accessible. You can have global state comprised of multiple objects of the same class. Global state is bad for me because it makes understaning of a peace of code more difficult. Same goes for goto. Same goes for multiple assignments to same variable. But singleton does not make it harder. Virtual functions sometimes make it harder. Events often make it not only harder to comprehend but also to debug.
  • Thomas 2012-09-19 15:28
    The constructor is public, and every time you create a new object via the constructor, getInstance() returns a different object. That's the problem
  • AN AMAZING CODER 2012-09-19 15:37
    Though it was probably a pain in the neck to track down, that's a very simple bug to fix. Have our WTFs come down to small and obvious bugs?

    change public to private then be done with it and move on.
  • impulse 2012-09-19 22:51
    I think the issue was that the static instance is never initialized.
  • Alexander Gee 2012-09-20 00:14
    I don't think this was ever intended to be a singleton. I think that someone didn't want to pass a reference down a chain of objects (to lazy to write the setters) so they just chucked it statically in there statically assuming that no one else would want to instantiate it.
  • JimmyNeutron 2012-09-20 16:52
    snoofle:
    What's wrong with that? It's easily discerned that the this in this singleton refers to this this from the singleton's point of view. From all other points of view, you should not be accessing that this, this this or the this, but the singleton via its getInstance, or pseudo-this.

    See? This is not so hard to understand!

    But the constructor is *public*. And the static initializer should be a pointer.
  • narrowtux 2012-09-21 05:30
    the point is, that the singleton doesn't care about actually constructing the singleton if it doesn't exists.

    The author should've put something like

    if (instance == null) {
    new KpiService();
    }

    at the top of the getInstance() method
  • justforgetme 2012-09-21 05:33
    I don't get it, are you funny?
  • TheLogster 2012-09-21 06:00
    snoofle:
    What's wrong with that?

    The singleton is never initialised, nothing calls new KpiService().

    If you call KpiService.getInstance(), you'll get null.

    The code should be something like :


    public class KpiService {
    private static KpiService instance = null;

    public static KpiService getInstance()
    {
    if (instance == null)
    new KpiSerice();

    return instance;
    }

    protected KpiService() {
    instance = this;
    }
    }


    The constructor should also be protected, so nothing outside the class can create a KpiService, and thus change where the instance points to.


  • StewartMoss 2012-09-21 07:17
    The method which populates the static property with the current instance is not static (eg the constructor is populating the property). This is a good WTF
  • gumcio 2012-09-24 08:18
    This code always throws exception.
  • Mmx 2012-09-24 10:19
    I guess the complaint is because the traditional singleton pattern would have it be allocated just-in-time at the first getInstance call.

    However that is a very broken pattern which behaves badly under multithreading scenarios AND has issues if order of construction should be maintained in any minor way.

    On the other hand requiring a manual allocation requires you to slap a new KpiService(); somewhere in the code.

    At the very least I would:
    - wrap the allocation a create method
    - assert if create is called twice (ignore the request in release)

    I think the mistake is calling this a singleton.



  • sigs 2012-09-24 12:30
    Sad thing is, Unity basically forces you to do that; there are lots of cases where you need a "manager class", essentially a singleton, but it still needs to inherit a framework class to integrate with the game loop.

    So yes, you basically get instance = this; a lot.
  • DFPercush 2012-09-24 13:02
    The problem is that the class has never been instantiated, and you try to call a static function. Static functions don't trigger any constructor because they do not reference any particular instance. So the assignment of 'this' never happens.
  • KnobbyHills 2012-09-25 12:17
    From the code snippet, i cannot see that the class is ever instantiated. That is, while the singleton accessor is legitimate, shouldnt getInstance() check for a null state and initialize the var?
  • Chris 2012-09-30 02:15
    Just because all of you guys expect a certain pattern for a Singleton doesn't make your opinions right. What about multi threading scenarios where two threads both try to access the Singleton for the first time simultaneously? That requires a bunch more synchronization in the getter which slows down the whole getInstance() function.

    The solution is then to ensure that only one thread can instantiate the instance on app creation, which is no different than having that one same thread just call the constructor.

    I have written code exactly like this for android for exactly this reason. If you know your Singleton is only ever constructed once, then it's better to have a null pointer exception when you violate the state design than sometimes have strange state errors caused by synchronization problems between threads. If you don't understand what I'm talking about, work 10 more years and reread this comment.

    /big fan of asserts that crash the program the second bad things happen as opposed to debugging strange behaviors.
  • Justin 2012-10-04 14:58
    I think the issue here is that calling getInstance() should return you an instance even if it hasn't been initialized and also constructing the object would reset the singleton instance.

  • Mikey 2012-10-15 22:21
    Sure, it compiles fine, but it's certainly not a singleton. You can create as many as you want, and instance just points at the most recently created one.

    For it to be a singleton, it should be:

    public class KpiService {
    private static KpiService instance = null;

    public static KpiService getInstance() {
    if (instance == null) { instance = new KpiService(); }
    return instance;
    }

    private KpiService() {
    // NOT NEEDED: instance = this;
    }
    /* SNIP */
    }

    Also, singletons are ridiculous anyway -- just use a static class and be done with the bullshit.
  • Oi 2012-10-17 05:34
    The constructor should be private and the getInstance should do something like:

    public static KpiService getInstance() {
    if (self::instance == null)
    {
    self::instance = new self();
    }
    return instance;
    }
  • Grayson C 2012-10-26 12:17
    But it's never initialized, so THIS doesn't matter.
  • Powerslave 2012-10-29 10:39
    I'm not sure if you're actually trolling or not?
  • TigerShark 2012-11-17 13:37
    letatio:
    QJo:
    For the record ...


    public class KpiService {

    private static KpiService instance = null;

    public static KpiService getInstance() {
    if (instance == null) {
    instance = new KpiService();
    }
    return instance;
    }

    private KpiService() {
    /* SNIP */
    }

    /* SNIP */
    }



    But don't you think about performance? While the program runs every single access to the singleton will execute that unnecessary "if", except the first. Do you think processor cycles grow on trees?
    Are you serious?
    Modern CPUs have been using branch prediction units for ages now, and that pattern will cause a pipeline flush only the first time that is executed, optimistically speaking.
  • TigerShark 2012-11-17 13:42
    TheSHEEEP:
    Severity One:
    TheSHEEEP:

    What would be better? Passing an instance of the "SoundManager" to each an every class that may want to play a sound? Limiting what classes should be able to play a sound? What good would come of that?
    Well, why would you want to play a sound from your disc I/O classes?


    The question is not why I would want to allow something, the question is why would I want to limit something that does no harm?

    Remy Porter:
    Yes, you should limit what classes should be able to play a sound. I think OOP does a lot of harm in the way it forces us to live in a world of nouns, but one of the principal benefits of OOP is that it allows for clear separation of concerns. Part of that is making the interactions between modules explicit. This allows the code to be understood from a structural perspective.


    I am absolutely convinced that a simple SoundManager singleton would be far easier to understand than the complex architecture you suggest. And it would also be much faster, an event system would simply introduce clutter.

    Mind you, I'm not talking about more complex stuff like 3D sound, that would be a pretty bad idea to have as a Singleton as it depends on a whole new layer of stuff, would have to watch objects, etc. And if you already built that 3D sound class, there's no point in building an extra singleton class for 2d sounds. Even if it means you won't be able to play a sound from anywhere.

    Remy Porter:
    There are a lot of ways to provide sound functionality. One way that would be vastly superior to a Singleton is to use the Observer pattern. A single SoundManager instance registers interest in game objects. When sound-worthy events occur in those objects, the SoundManager matches them up with a sound.


    I get the idea, but that would be an overly complex solution to a very easy problem.
    You know, we have a nice saying in our office: "You can solve any problem by adding another layer of indirection. Except too many levels of indirection."
    Some people should really take that to heart. ;)

    Also, people seem to be stuck on that sound example.
    Let's introduce another one: Logging. In the same game (and in all other applications I wrote and most I encountered) I had a Logging singleton class.
    Now Logging is really something you want to do from possibly everywhere without having to rewrite your class hierarchy/structure.

    And the possibility to do so from everywhere also doesn't break or harm anything. Except threaded stuff, but you can always make that Singleton thread-safe.
    AOP is the solution for logging, as for every crosscut responsibility, not singleton.
  • TigerShark 2012-11-18 05:27
    AGray:
    Dave Insurgent:
    That being said, I also see YOUR way of things - what if, in a future project, I want items to be degradable? What if I want to implement the Gummi Sword? The way you defined would be better.


    Not to mention you can also write automated acceptance tests for them, so that your game, as an application of your game engine, is driven not by the "wizard specification" (nicely put), but by an actual spec - that maybe someone else a little more 'product-oriented' or 'business-oriented' (almost like an analyst... for business!) writes themselves.


    TDD in a nutshell...I've been exposed to it, and trying to integrate it into my very-indie dev ops at home.

    Well, we should thank the TDWTF staff - the WTF posted has helped one developer see another set of WTFs in his own work! Thanks Remy et al!
    nope, this is BDD. You formalise the requirements in acceptance test and then when your code is green for all of these you know that you are satisfying the requirements.
    TDD is at unit test level, you write a test that defines some method behaviour and then you write the simplest code that makes it pass, then you write another test and so on until the method behaviour is formalised exactly by the unit tests.

    (Why there are Latin words in the captchas? WTF???)
  • TigerShark 2012-11-18 06:43
    Randy Snicker:


    Thank you for not attributing my quotes to me so that I get to play detective to find out whether you've answered to me or not.

    Sure, if you like to mince words and act like an idiot, engineering is philosophy. Although, anyone with half a brain should have seen that I clearly contrasted philosophy with engineering, which should tell a rational person that I used the terms in a context in which I specifically redefined them to be mutually exclusive. In addition, the word philosophy is probably most often used to describe the particular branch of science that doesn't fall under the common definitions of engineering or the other branches of science.

    I'd argue the difference is that engineering is about problem solving and making things work, whereas philosophy is a bunch of hipsters drinking red wine talking about something so meta that it has effectively no relevance to anything practical whatsoever.
    philo and Sophia are two Greek words that mean love for the knowledge.
    I think that an engineer that is not in love for the knowledge is not an engineer by any means.
  • TigerShark 2012-11-18 06:52
    Chris:
    Just because all of you guys expect a certain pattern for a Singleton doesn't make your opinions right. What about multi threading scenarios where two threads both try to access the Singleton for the first time simultaneously? That requires a bunch more synchronization in the getter which slows down the whole getInstance() function.

    The solution is then to ensure that only one thread can instantiate the instance on app creation, which is no different than having that one same thread just call the constructor.

    I have written code exactly like this for android for exactly this reason. If you know your Singleton is only ever constructed once, then it's better to have a null pointer exception when you violate the state design than sometimes have strange state errors caused by synchronization problems between threads. If you don't understand what I'm talking about, work 10 more years and reread this comment.

    /big fan of asserts that crash the program the second bad things happen as opposed to debugging strange behaviors.
    just instantiating it in the member variable or in a static initialiser as someone already suggested fix this problem with absolutely no need for synchronisation.
    that code is really awful and is a TWTF.
  • cppdragon 2012-11-21 08:33
    The point is if you create a singleton this way the constructor has to be private in order to not contruct the object from the outside.
    This thing is in fact no singleton because there is no implementation in the code to ensure it. Of course it will always return null because it is never constructed, not even by the getinstance!