Proper usage of Singleton getInstance method

Question

What is the proper usage of a singleton that uses a static synchronized getInstance method and why?

For example:

MySingleton mySingleton = MySingleton.getInstance();
.....
mySingleton.doSomething();

where mySingleton would be a field and used throughout the class.

versus

MySingleton.getInstance().doSomething();

EDIT: By proper, I mean coding style and thread safety.

Answer 1

While I agree with Andy Turner's answer to avoid singletons if possible, I'll add this for completeness: the static synchronized getInstance method is a relic from the past. There are at least 3 singleton pattern implementations that don't require that.

a) eager singleton

public class EagerSingleton{
  private EagerSingleton(){}
  private static final EagerSingleton INSTANCE = new EagerSingleton();
  public static EagerSingleton getInstance(){ return INSTANCE; }
}

This is initialized at class-load time, whenever you reference the class EagerSingleton in any way (e.g. by declaring a variable of that type).

b) lazy singleton

public class LazySingleton{
    private LazySingleton(){}
    private static class Holder{
        private static final LazySingleton INSTANCE = new LazySingleton();
    }
    public static LazySingleton getInstance(){ return Holder.INSTANCE; }

}

This is initialized at call time, the first time you call the method getInstance(). The class loader now loads the holder class and initializes the INSTANCE field, and it's guaranteed to do that in a synchronized way. Use this version if your singleton has expensive setup, and is not always necessary.

c) enum Singleton

public enum EnumSingleton{
    INSTANCE;

    // and if you want, you can add the method, too, but it's
    // unnecessary:
    public static EnumSingleton getInstance(){ return INSTANCE; }
}

Enum items are compile-time constants, i.e. their uniqueness is guaranteed at compile-time, not only at runtime (at runtime, singletons are unique per class loader, and that applies to all versions). This is the best way to go, unless your requirements are to extend from an existing class.

The enum version gives you many other features for free:

• equals / hashCode / toString implementations out of the box
• defense against de-serialization attacks
• multiton support (just add another enum item)

So the next time you see somebody write the double-checked locking mechanism or similar, tell them they are being way too complicated and technical. Let the class loader do the work for you.

With all 3 versions: make sure the functionality you use is backed by an interface, and code against that interface, not the implementation type. That will make your code testable.

Answer 2

A better solution than using a singleton is to use dependency injection:

class MyClass {
  private final MySingleton mySingleton;

  MyClass(MySingleton mySingleton) {
    this.mySingleton = mySingleton;
  }

  // Use mySingleton in instance methods as required.
}

The advantage here is that the fact it's a singleton isn't relevant - it can be a singleton, but it doesn't need to be.

I called the field mySingleton because that's what you've called it in your question, but there is no longer a requirement for it to be really singleton: the only thing is that MyClass doesn't need to care about the life cycle of the instance.

This breaks the static coupling between MyClass and MySingleton that would exist if you did it by either of the ways described in the question. This makes it easier to test MyClass, amongst other advantages.

Answer 3

Somebody must instantiate a singleton. This has to be done only once. Here comes thread safty into play. If there is a chance that different threads call

SingletonClass.getInstance();

nearly at the same time, the instantiation of the singleton has to be protected like this:

private static volatile SingletonClass myInstance;

public static SingletonClass getInstance()
{
    if ( myInstance == null )
    {
        synchronized( SingletonClass.class)
        {
            if ( myInstance == null )
            {
                myInstance = new SingletonClass();
            }
        }
    }

    return myInstance;
}

This is called "double checked locking". The variable must be volatile.

If the methods of the SingletonClass share state between threads (e.g. you have a member which is written by one thread and read by another thread) additional care has to be taken on synchronization. But this is common and has nothing to do with the Singleton pattern.

Your question what is the difference between "XY.getInstance().doSomething()" vs. keeping a reference in a own member for accessing it later: It is only a (minor) performance penalty (callstack one more, if statement, return). But IMO this is ignorable because on long run the java optimizer would inline this anyway. On the other hand: keeping a reference keeps the code shorter and more readable, escpecially if you access the singleton often. At the end it is a matter of taste.

Answer 4

I think it would be better to define a field for every singleton in the class. So you can easily find out dependencies of a class to singletons in the system by just looking at its first n lines. It helps to understand coupling of a class.

Also, defining fields for singletons makes testing easier. If by any chance you can move methods of the singleton to an interface, you can define a stub class for the singleton and you can inject to stub singleton in your test code.

For thread safety part, if your singleton is thread-safe, there shouldn't be any problem.