Is this singleton design pattern correct?

Question

Is this singleton design pattern correct ? I mean what's the need of checking the object is null or not when it's static and the method is synchronized .

public class MySingleton {

    int val = 10;
    private static final MySingleton singleton = new MySingleton();

    private MySingleton() { }

    public static synchronized MySingleton getSingleton() {
        return singleton;
    }
}

Answer 1

You don't need to make your method synchronized. The fact that the variable is initialized in a static initializer is enough. Also, your val variable should almost certainly be private...

The double-checked locking pattern (with the nullity checking) is usually used when you don't want a synchronized method and you don't want a static initializer. (To my mind it's unnecessarily complex and brittle in almost all cases.)

Another option would be to use an enum:

public enum MySingleton {
    INSTANCE;

    private int val = 10;

    // Presumably something to use val
}

Using an enum enforces the singleton-ness and even gets it right in the face of serialization. It's also a pretty simple way of doing it with no actual code :) On the other hand, it's never felt entirely right to me...

Answer 2

You can do it like that, but in many cases you can use "lazy evaluation" - you create the instance the first time it is requested:

public class MySingleton {

    private static MySingleton singleton = null

    private MySingleton() { }

    public static synchronized MySingleton getSingleton() {
        if (singleton == null) {
            singleton = new MySingleton();
        }
        return singleton;
    }
}