• Gandor (unregistered)

    CodeSOD instead?

  • (cs)

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

  • (cs) in reply to Gandor

    Apparently the idiot that wrote this article picked the wrong category tag, Gandor.

  • charriu (unregistered) in reply to Mcoder

    Scala does that with the "companion object" as it's called, IIRC.

  • My name is unimportant (unregistered)

    Well, that's what David gets for trying to use the singleton before creating an instance of the class.

  • (cs)

    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 (unregistered)

    Well, it's a multigleton... an evolution of the poor old singleton.

  • Erik (unregistered)

    TRWTF is the singleton pattern!

  • (cs) in reply to snoofle
    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 (unregistered)

    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 */
    }
  • (cs)

    TRWTF is languages which require everything to be a class. Like Math.

  • Andrew (unregistered)

    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 (unregistered)

    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 (unregistered)

    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 (unregistered)

    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 (unregistered)

    Can anyone explain this for laymen?

  • (cs) in reply to Röb

    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 (unregistered) in reply to Mcoder

    Scala does.

  • (cs) in reply to maybe
    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 (unregistered) in reply to Remy Porter

    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 (unregistered)

    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 (unregistered) in reply to maybe
    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 (unregistered)

    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.

  • (cs)

    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!)

  • (cs) in reply to TheSHEEEP
    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.

  • (cs)

    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 */
    }
    
    
  • (cs) in reply to TheSHEEEP

    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 (unregistered) in reply to snoofle

    This is a great comment. And by this I mean that this, not this this.

  • Confused (unregistered) in reply to snoofle

    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 (unregistered) in reply to Remy Porter
    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 (unregistered) in reply to @Deprecated
    @Deprecated:
    Sooo... which one do you call?

    Ghostbusters! Oh, wait...

  • T (unregistered) in reply to TheSHEEEP
    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 (unregistered) in reply to Jeff Dege
    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.

  • (cs) in reply to Jeff Dege

    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 (unregistered)

    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 (unregistered) in reply to QJo
    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 (unregistered) in reply to VinDuv

    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 (unregistered) in reply to Severity One
    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 (unregistered) in reply to TheSHEEEP
    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 (unregistered)

    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 (unregistered)

    [code] public class KpiService {

    private static KpiService instance = new KpiService();;
    
    
    
    public static KpiService getInstance() {
        return instance;
    }
    
    
    
    private KpiService() {
    
        /* SNIP */
    
    }
    
    
    
    /* SNIP */
    

    }

  • Garrison Fiord (unregistered)

    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 (unregistered) in reply to snoofle

    The non-static constructor for the singleton should be private, not public.

  • Garrison Fiord (unregistered) in reply to 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.
  • (cs) in reply to TheSHEEEP

    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 (unregistered) in reply to Garrison Fiord
    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 (unregistered) in reply to Nagesh
    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 (unregistered) in reply to Remy Porter
    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 (unregistered) in reply to Röb
    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.

  • (cs) in reply to 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 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.

Leave a comment on “The Lone Rangers”

Log In or post as a guest

Replying to comment #:

« Return to Article