Java Singleton.getInstance() returns null?

Question

I have this singleton I'm trying to use, but getInstance can apparently return null:

class Singleton {
    public static final String K_LEVEL = "level";
    static Singleton instance = new Singleton();
    private int level;

    static Singleton getInstance() {
        return instance;
    }

    int getLevel() {
        return level;
    }

    void incrementLevel() {
        System.out.println("LEVEL INCREASED TO " + ++level);
    }

    void addToLevel(int x) {
        for(int i=0;i<x;i++)
            incrementLevel();
    }

}

class A {
    public static void main(String[] args) {
        Singleton s = Singleton.getInstance();
        Integer i = Integer.getInteger(Singleton.K_LEVEL);
        s.addToLevel(i);
    }
}

I heard implementing singletons in Java is very hard and prone to race conditions. Is my singleton pattern implemented wrong? I recently changed my code to look like this, and now getInstance returns null sometimes. Why?

$ java A -Dlevel=1
Exception in thread "main" java.lang.NullPointerException
    at A.main(A.java:29)

Answer 1

There is nothing wrong with your Singleton. There are no concurrency problems because this is not multithreaded code.

You were thinking s was null, but it is really i that was null.

Since addToLevel takes an int as a parameter, the Integer i was autounboxed (implicitly converted from Integer to int), but since i was null, NullPointerException was thrown. Autounboxing throws NullPointerException when the value being coverted is null.

The reason Integer.getInteger(Singleton.K_LEVEL) returned null is because you did java A -Dlevel=1 as opposed to java -Dlevel=1 A. The latter is the correct syntax.

Answer 2

This is not about your singleton pattern which looks fine to me. It is the Integer.getInteger(Singleton.K_LEVEL); method that is returning null. I bet the "level" system property has not been set and is null.

java A -Dlevel=1

You need to put the -Dlevel=1 before the A class on the command-line. If you debug your code or print out the system property you will see that it is null.

java -Dlevel=1 A

You get a NPE when you try to pass the null into addToLevel(int x) and it tries to auto-unbox the null to be int x.

As an aside, if this class is used by multiple threads, you should consider using an AtomicInteger inside of your Singleton class which is reentrant.

Answer 3

java -Dlevel=1 A should suit your needs.

From the doc, the syntax is java [ options ] class [ argument ... ], and -Dlevel=1 is considered as an option (see the options section).

Answer 4

static Singleton instance = new Singleton(); should be final to prevent race condition.