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)

Lanellelanette

What does System.out.println(i); right before s.addToLevel(i); print? – Ceasefire

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.

Nitty

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.

Goggin

No, I set it by doing this: java A -Dlevel=1 – Lanellelanette

The -D has to be before the A class @Dog. I'd debug your code or print out the system property to verify it. – Goggin

I agree. @Lanellelanette why don't you print i just before calling addToLevel to verify. – Refrain

It sounds like you are going on from a speculation. Even if the getInteger was returning null, the cast should make it 0. I don't create other threads, but I don't know.. I thought perhaps the VM instantiates my singleton on another thread so that could by why I'm getting null for s? – Lanellelanette

Nope. That's not how auto-boxing works. You will and are getting a NPE because your system property is not set right @Dog. – Goggin

see the below answer by sp00m. Looks like that's the reason. Its weird though. – Refrain

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

Swipe

Answer 4

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

Stapes