Google Singleton Detector | Hacker News Reader

Google Singleton Detector

code.google.com

135 pointsjamesjyu81 comments

In case you are wondering why singleton's are bad practice in Java, here are two reasons off the top of my head.

1) Dependency hiding.

Singletons are rarely based around as parameters. Rather, methods simply access and modify them as necessary. This can be obviously convenient in a lot of places, but it makes looking at method signatures less reliable a way of determining dependencies.

2) Global state.

Singletons are global state, more or less, and are thus difficult to maintain. Bugs involving global state can be hard to detect, and global state isn't even amenable to testing, because the order of your tests start to matter. Side-effects bring a lot of troubles.

Edit: I guess this is redundant with the information on this page. http://code.google.com/p/google-singleton-detector/wiki/WhyS...

thanks for your explanation. But, what is a singleton?

A quite good answer, from Wikipedia:

"a design pattern [that restricts] the instantiation of a class to one object"

    public class Singleton {
        private static Singleton self = null;
        private Singleton() {
            //...
        }
        public static Singleton getInstance() {
            if(self == null) {
                self = new Singleton();
            }
            return self;
        }
        ...
    }

So at any point in your code you can call Singleton.getInstance() to have access to the same Singleton object. So it's effectively the same thing as a global variable.

Edit: Added `static' to be more correct.

private Singleton self = null; is wrong.

It has to be:

private static Singleton self = null;

and that isn't thread safe lazy initialization either ;-)

it's surprisingly tricky to get lazy init right.

What are the advantages of instantiating the Singleton object in multiple different locations rather than, for example, using the Singleton class itself as an object with various static attributes and methods?

Not thread safe.

Thread safety is the same whether you use a static instance or plain static methods. In fact with all static members you don't have to worry about synchronizing initialization of the singleton object which is often done wrong.

The only reason to use a singleton over static members is that it feels like OO. I suppose it also makes it easier to replace the singleton with a 'multiton' at some point in the future which somewhat justifies it.

There isn't a clean way to initialize all of your static properties doing it that way. If you're using a language that has accessors you can call a private init method, but you have to do it for all of the properties. Or you can just use methods, but again you have to remember to call init at the beginning of them all. Or you could have a public init() and just remember to call it when your application first launches. Using a singleton is a natural way to have an entry point towards initialization. You can also have the best of both worlds by using static properties/methods that point to the singleton version.

In Java/C# you can use a nested class to ensure thread safety and cheat towards having your static methods. For example:

private int Foo() { }

public static int Foo() { return _instance.Foo(); }

private static FooBar _instance { return Nested.instance; }

FooBar() { // Initialize here! }

class Nested {

static Nested() { }

internal static readonly FooBar instance = new FooBar();

}

In addition to what johnyzee says, in Java at least, a class with just static methods can't _implement_ any Interfaces.

One benefit is that you can replace the singleton instance with a mock or subclass for testing if you need to.

Nothing. Both are the same bug.

Enum version:

    public enum Singleton {
        INSTANCE;
    }

Guaranteed against multiple instantiation.

Edit: the class version of the singleton example should also be made final, or I could just extend your class and therefore bypass your singleton restriction.

Not with a private constructor.

My gripe with global state (i.e. singletons) is not testability, maintainability nor correctness. It's nothing more than bad object-oriented design, or lack thereof.

I've seen this time and again - even this week, as I am doing some major refactoring in old code - singletons are used exactly where someone was too lazy (or too busy) in order to do a proper design and review. I cannot count the number of proverbial SomethingManager singleton classes I've seen.

At most, singletons should be used in a single point of entry to a module. From there on, even if other classes should be used once, their singularity will be derived from that single point of entry that contains them.

Rule of thumb - stop using singletons. In 99% of the cases you will see a much better design exists.

Isn't it bad design because it hinders testability, maintainability, and correctness?

In general, whether we are talking about procedural, OO, functional, or whatever kind of programming, code is easier to reason about and test and use if the effects of its individual units (functions, classes, modules, &c.) depend largely on the inputs to them. In functional languages this is called "referential transparency," but its value applies to OOP as well.

Using things like singletons creates hidden dependencies on things that can change over the lifetime of the program without those changes being made obvious to clients of that unit of code. Good design nails things down to explicit parameters.

Calling something bad design without discussing the actual problems it causes is mere name-calling or unsupported opinion. I submit that if you can't make a claim that something is hard to maintain, unreliable, hard to understand, incorrect, fragile, etc... then it is not bad design.

The claim that it is "bad" because it doesn't fit in to a certain paradigm is at best a valid claim that it is inconsistent with the rest of the codebase and therefore hard to work with. At worst, it's cargo-cult engineering; doing something a certain way because it's said to be "good" without understanding why.

"I submit that if you can't make a claim that something is hard to maintain, unreliable, hard to understand, incorrect, fragile, etc... then it is not bad design."

That's not really true either - it just means that the claimant probably lacks the experience to recognize whether or not it's bad design. Even people who are right for the wrong reasons can still be right.

That's a valid point, but in that case the claim is no more reliable than random chance.

Since you handwaved over "a much better design exists," I thought I'd give an example.

So, you have an app deployed to seven customers. The typical pattern is you develop an app for a first customer. It is then sold to subsequent customers, doing the least possible to make it work. In this case, as I have seen in the past, you have a properties file, saying which classes to use and which configuration options the customer has available.

These are then loaded up, as global state, so that your code can ask, "Does this application have Widget Processing capabilities?" and gets the answer for its code path. Consider:

  if (AppProperties.getInstance().hasWidgetProcessing() {
    process(widget); 
  }

Now, the problems here are pretty obvious. Since you're depending on a single file, your testing is a bear. Getting to the various codepaths is nigh impossible. Ideologically, it's bad design because your software has a bunch of if or switch statements in the code to decide what it should really do.

Now, what would make this code better? Well, you could start with taking all your Widget Processing capabilities and putting them in their own object or package. Then, instead of the above, your code looks like this:

  public class WidgetImpl implements Widget {

    private WidgetProcessor widgetProcessor;

    public void process() {
      widgetProcessor.process(this)
    } 

    public void setWidgetProcessor(WidgetProcessor wp) {
      this.widgetProcessor = wp;
    }
  }

In your main code:

  widget.process();

WidgetProcessor has two (or more) implementations, in this case: RealWidgetProcessor and NullWidgetProcessor. NullWidgetProcessor just returns without doing anything. Now, how does your Widget get the proper widget processor? Dependency injection. http://code.google.com/p/google-guice/wiki/Motivation?tm=6 has a pretty good explanation.

Improvements on the design are welcomed, as always. :) Though for simplicity's sake I was trying to keep code to a minimum. Now, speaking of code, my main issue with dependency injection is that the code baloons, especially in java. DI as a pattern makes me feel like there's something missing in current language implementations that would make this more elegant. Aspect-oriented programming was one try at it, but it seems that the cure is worse than the disease in that case.

I should mention that another alternative is to write a domain specific language, which is what a Lisper or Rubyist would advocate. That way, you put together each instance like play-doh. (a la http://weblog.jamisbuck.org/2008/11/9/legos-play-doh-and-pro... )

Also most people forget that the typical approach to singletons can yield multiple instances if the application employs multiple classloaders.

As a new Objective-C programmer, I recently ran into the global state problem with an iPhone game I'm working on. I needed some settings that could be accessed/changed from many different classes. I was told the AppDelegate isn't the right place to put this.

A singleton SettingsManager was suggested, but singletons always get a bad wrap.

So is this a good use of a singleton? What would be the correct way to handle this?

YMMV, but the way I prefer doing it is by telling the settings explicitly to the different classes, instead of having all those classes rely on getting it by themselves from the singleton (this is where the problem lies).

You can pass the settings in the constructors, with setter methods or have a "state object" that contains the configuration.

For "updating" the states on every class in real time I'd rather rely on explicit state change using the Observer pattern.

EDIT: I'd also like to know what other developers do, or if there's anything wrong with my methods, so I second your question.

I have been wondering the same, about other developers.

I am working on an application that periodically pulls information from a server about a physical object (sorry, I can't be more specific than this, so please don't ask). This information needs to be accessed by several different classes, and displayed in view controllers on different tabs. A view controller will pull the information and make it available to all the other view controllers.

A singleton provided an effective solution to this because only one data model of my physical object's state can exist at any given time, with the added benefit that I can access it from any view controller without much effort.

I may be wrong, but I thought the purpose of a singleton was for models where there can be one and only one instance of your class? Singletons (like other design patterns) were designed to solve a specific problem. But I concede that they are probably overused.

Letting individual classes get settings from NSUserDefaults (which itself effectively presents a singleton) directly seems to work pretty well for me.

One advantage is that the classes are then reusable in other projects because they aren't coupled to an application specific settings provider.

The correct way to deal with it is to realise that if you need some settings that can be accessed/changed from many different classes then the design of your app is severely lacking. However, this doesn't necessarily that you need a more complex/sophisticated 'design'. ie. if you stick to TDD this situation is highly unlikely.

Hopefully this will stem the tide of questions on Stack Overflow about different Objective-c singleton implementations.

The correct way to deal with this is to realize you have a problem? I think the above comment is willing to admit a problem and is asking for a solution. What does TDD have to do with it?

I'm also interested in hearing what is considered the best practice for things like passing around config options, an eventloop, or message bus.

The solution is essentially to take a hard look at your design and see why you have to pass around config options, event loops, or message buses to begin with. The need to access global state is a code smell.

When you DO need global state, then a singleton object is a reasonable way to achieve it. But chances are fairly high that you don't.

You can look for days, and if you don't know any of the alternatives, none will pop out.

As I'm not in the mobile sphere, I wouldn't know how to avoid a singleton for an app that is constrained by speed and space requirements. I've already shown an example above, what's your solution in this environment, with the constraints of the questioner?

> if you need some settings that can be accessed/changed from many different classes then the design of your app is severely lacking.

If he's making a game, this is completely acceptable design. Games programming is wildly different from application or server programming. Also, strict TDD is much, much less useful in games programming than anywhere else. Writing unit tests for central data structures and algorithms is useful, of course, but the majority of modules/classes/objects are so small, uncritical, and change completely so often that it's not worth bothering.

hey Jaber. Singletons are fine for that purpose, actually they are probably the best solution.

iOS uses singletons everywhere, so feel free to use them.

Don't fall the trap of the cargo culture thinking. Google has a completely different problem than you do. Their code spans hundreds of thousands (or millions), of code, in distributed systems, etc..

your iOS app probably has different constrains.

Where does IOS use Singletons?

any method that has 'shared' or 'current' in front.

http://developer.apple.com/library/ios/#documentation/uikit/...

It is almost everwhere.

[MyClass sharedMyclass] is the Objective-C equivalent of getInstance()

Also, singletons make good sense in mobile apps, as it is a very different type of a beast than a server side app.

See: http://gigaom.com/apple/iphone-dev-sessions-using-singletons...

Depends on your definition of singleton.

In java, singletons are enforced by the type system - i.e. There's no public way to make more of the object. Usually that's what people mean by 'singleton' in java. If you don't have the source and you outgrow the single instance, there's nothing you can do except stop using the class.

In Objective-C, people do occasionally simulate that by mungng init and alloc, but that is definitely a bad idea. More often a singleton class will have a conventional init and alloc, but have a class method like +sharedInstance which lazily instantiates and dispenses a single instance of the object during the lifetime of the application. That's what people most often mean by 'singleton' in objective-c. It's basically a global variable, but when you outgrow it, it's often not hard to switch to allocating more than one.

Also, partly due to what many would see as a deficiency - the fact that it's much harder to produce architecture independent libraries in objective-c than java, the source is available more of the time, so evolving past a singleton is more often straightforward.

Don't fall the trap of the cargo culture thinking. Google has a completely different problem than you do. Their code spans hundreds of thousands (or millions), of code, in distributed systems, etc..

This is probably the single most useful quote in this entire discussion. Singletons are not inherently bad -- they were created for a reason, but because people abuse them, they have a bad rep.

If you aren't writing an actual hardware device driver for a specific peripheral port, you are abusing Singleton.

There are many instances where you need just one instance of a class, not just a hardware device driver (for instance, when you need GPS location coordinates on a mobile device across several different classes).

No, Settings/Configuration is a classic example of where using a Singleton horribly breaks testability, because you can't run multiple "application runs" in the same "test run" without completely exiting and rebuilding the app runtime environment.

Testing frameworks exit and rebuild the app runtime environment all the time, to isolate the effects of one test case from the next. When your singleton instantiation is paramatrized, for instance with a configuration file, than there is no testability problem whatsoever.

I would go as far as to say that if you have to do multiple "application runs" in the same "test run", where these "application runs" require different configurations to have the test cover some specific scenario, then your code has larger problems than having the configuration in a singleton.

If you're using NSUserDefaults, why bother with a SettingsManager? If the SettingsManager is an NSUserDefaults replacement for whatever reason, then it's probably fine for it to be a singleton.

Google also use singleton, it really depends on the context and how it's used.

e.g. http://code.google.com/mobile/analytics/docs/iphone/

GSD doesn't only detect singletons; it detects four different types of global state, including singletons, hingletons, mingletons and fingletons

uhh, what are hingletons, mingletons and fingletons?

From the usage page:

    * Hingleton - Derived from “helper singleton,” a class which turns another class into a singleton by enforcing that class's singularity.
    * Mingleton - Derived from “method singleton” a class which has any static method that returns some state without taking any parameters.
    * Fingleton - Derived from “field singleton,” a class which contains a public static field.

This is what happens when you let Professor Frink publish papers.

Or Dr. Seuss (PhD?).

They are defined in the usage wiki: http://code.google.com/p/google-singleton-detector/wiki/Usag...

  Singleton	 A class for which there should only be one instance in the entire system at any given time. This program detects singletons which enforce their own singularity, which means they keep one static instance of themselves and pass it around through a static getter method.

  Hingleton	 Derived from “helper singleton,” a class which turns another class into a singleton by enforcing that class's singularity.

  Mingleton	 Derived from “method singleton” a class which has any static method that returns some state without taking any parameters.

  Fingleton	 Derived from “field singleton,” a class which contains a public static field.

So, GSD is just "grep -r static . | grep -v class" then?

It operates on JVM bytecode, not on java source code.

No.

Misko Hevery - one of the developers of the GSD - has a blog were he talks about testing, dependency injection, and global state. He hasn't posted a lot recently, but there is a lot of interesting stuff if you dig through his archive.

http://misko.hevery.com/

Misko's blog is brilliant and it is truly a shame he isn't posting as much as he used to

Some posts relevant to this discussion

http://misko.hevery.com/2008/08/17/singletons-are-pathologic...

http://misko.hevery.com/code-reviewers-guide/flaw-brittle-gl...

I learn how bad they were and why from this article :

https://sites.google.com/site/steveyegge2/singleton-consider...

That's a good read

I have seen this article referenced rather often, and I really have a problem with it. Despite being called "Singleton Considered Stupid," much of the article is not about singletons at all, and he barely addresses why singletons are bad. The actual arguments, buried 3/4 of the way into the article, boil down to a) resource/memory management problems, and b) that in the future, you might possibly need more than one object of anything you decide to make a singleton. (He mentions multithreading too, but that is just silly; e.g. even if I had multiple Log objects, there would need to be synchronization at some point if I'm outputting to the same stream). He writes numerous times that it's bad OOP, or "OOP made easy," because it is too much like procedural programming, but only mentions the factory-method pattern as an alternative at the very end of the article, and doesn't actually say WHY it's better. The necessity for, or at least temptation of global state is not even mentioned.

Googe Guice's documentation has a nice explanation of how to scope objects (from http://code.google.com/p/google-guice/wiki/Scopes). How they word it is specific to Guice but the underlying points are valid for any Java application

"Choosing a scope

If the object is stateful, the scoping should be obvious. Per-application is @Singleton, per-request is @RequestScoped, etc. If the object is stateless and inexpensive to create, scoping is unnecessary. Leave the binding unscoped and Guice will create new instances as they're required.

Singletons are popular in Java applications but they don't provide much value, especially when dependency injection is involved. Although singletons save object creation (and later garbage collection), getting a handle to the single instance requires synchronization. Singletons are most useful for:

* stateful objects, such as configuration or counters

* objects that are expensive to construct or lookup

* objects that tie up resources, such as a database connection pool."

I have to admit that when I first started coding Java/Guice I had 'singleton addiction'. Luckily with Guice, as someone else mentioned, it's just matter of deleting the annotation on a class to change your mind about that.

Anyway, if you use Guice and dependency inject your singleton classes, then the issue of being hard to test and disguising dependencies goes away, doesn't it?

If you use Guice you aren't using a singleton anymore. Guice is creating an instance and passing it to multiple classes as a dependency. This is different from any class just referencing a Singleton by importing it statically.

It's a slightly subtle distinction, and you still have the risks associated with non-local mutable objects, but at least you can track (using the Annotation API) which classes are using the shared state; it's better than letting any line of code anywhere in the middle of any method sneakily and suddenly grab access to shared state.

I was genuinely expecting a hack that parsed google+ profiles to figure out who's single... doh!

If the point is to find hard-to-test code, wouldn't one find it automatically while writing tests?

Maybe the point is to find hard-to-test code in other people's code.

shrug. I've found singletons to be good for modelling session data in a web server.

Calling GetInstance returns the object stored in the session, creating a new empty one and stashing it there if required.

That's actually, pretty much the only thing I use them for, though.

It's not really a singleton if it returns an instance per session instead of a global one.

Yeah, its not really at true singleton in the dangerous sense in the context of a webserver, when you have a separate copy of the environment for each HTTP connection. It wouldn't be a singleton unless the same state could be accessed by different HTTP sessions.

I haven't come across a web framework that enables this, as you might expect; there's no good reason to do so.

Why don't you pass the session data as an argument to methods that need it?

Example of stuff that was modeled erroneously as a Singleton?

A personal example.

Just when I started college, was trying to write a 3D game in C++ (I was young, so yeah, I thought it was just C with classes, LOL).

Since I had no idea about design patterns, so I made a singleton called "Rendering Manager" that took care of "everything-OpenGL". I would have numerous classes, like CHuman, CHero, CParticle, CBuilding (it was more abstract but whatver) and stuff like that that inherited from something like a CDrawable, which had a CDrawable::Render() method.

The problem was that the CDrawable::Render() would call the singleton by itself, so it had a direct dependency with the Rendering Manager. If I had to change something on the singleton, I'd have to change a lot of other classes.

When I needed to render on different "virtual screens", for reflections, shadows, cameras (yes, it was funky lol), I had to put global state on the singleton in order to make it render things in different ways.

When I needed LOD (level of detail), I had to pass the camera position via a global, only adding more globals to the mess.

Of course, I was just a student back then, but failing spectacularly taught me a lot of new things.

What I would have done today: CDrawable objects would only worry about their own geometry and transformations and would be ignorant about the glorified-OpenGL-wrapper.

Instead, we could have a "Scene Manager" (which doesn't even have to be a singleton anymore) that "visits" each object to "ask" them just enough to get them rendered.

Now all we need is a over-engineered and inconvenient dependency injection framework detector, and we'll be all set.

Isn't a "mingleton" the same as a named constructor, or am I missing something?

If you mean this: http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Named_Const... , which is called a Factory, in Java, then no.

A Named Constructor / Factory creates a new instance on demand. A mingleton hands out references to the same instance each time it is called.

Yes, I was refering to that idiom. Thanks for the clarification.

Read Google Singularity detector. Was disappointed.

Aren't singletons the whole goal of Spring?

What is Spring?

Second hit on Google: http://www.springsource.org/

Not at all. IIRC, default instantiation mode is one instance per injection.

Actually I think singleton is the default: http://static.springsource.org/spring/docs/3.0.x/spring-fram...

I didn't know about the other scopes though

Dependency injection frameworks/IOC containers (pick your stupid name of choice) avoid at least one of the problems of singletons: if you decide that you don't want it to be a singleton any more, you can trivially change the lifecycle to 'instance'.

By contrast, using singletons in plain old Java, you'd have to go through and find every usage of SingletonClass.instance().everyMethodCallOnThisClass() and push it up to the constructor. Since you now have a new dependency you have to pass all the way down your callstack, it can get painful quickly.

You still have the issue of global state, but I think that's altogether more complicated. Sometimes state really is global to your program and exposing it everything makes a certain amount of sense. It may well be ok for managing system resources (thread pools, network connections, IO in general). I think often you can get away with a boolean flag or two shared across a whole app. If it's read-only it's not a problem at all.

You need to be aware that every bit of shared state you make available or continue to use is a potential source of bugs and maintenance. Every bit of this that you expose needs serious thought. Don't let it snowball. Even if it means you have to make a few extra types of object or pass a stupid number of arguments all the way down the callstack, that's usually preferable because at least then you can reason about what's going on.

And with structured programming, it's only one extra argument per function call, StuffThisFunctionNeeds (plus a structure definition for each different flavor of StuffThisFunctionNeeds)

Singletons are great for some things, and not bad design at all. Font managers, sound managers, upload managers, focus managers, anything that starts and stops threads are all good candidates for them. Singletons are an obvious solution for any program that needs to round-robin users to one of many nodes upon login and then leave them there for all classes to communicate with. Especially if you might need all your classes to suddenly start talking to a different server node instead. There's no reason -- and it's possibly dangerous -- to create multiple instances of any of the above...and the ability to (for instance) globally alter the volume of all the sounds of a given type in your game, or call a static function to play any sound, far outweighs the supposed design beauty of OO in those cases. In fact, you could argue that OO code only really works because it relies on a single instance of the system it's running on to manage one or more threads, one or more sound channels through a single set of speakers, etc.

If common sense would dictate that there should never be more than one instance of something, then enforcing that becomes good design, the equivalent of writing good docs and clean virtual classes / interfaces.

In my experience I have found that explicitly passing your dependencies around through constructors or method arguments leads to more robust abstractions. Defining an object as a Singleton from the get-go can lead to its overuse.

If you have to pay the cost of adding a parameter to a method or a constructor you really evaluate whether you need that object in that method in the first place. This doesn't stop you from enforcing a single instance.

Reply on news.ycombinator.com