Is my code a 'safe' singleton?

Question

I was wondering if my code will produce a true singleton. I am creating an Android app, and all activities should access my API through one instance of the SyncApi class.

public class Api {

    private static SyncApi api = null;

    static {
        synchronized (api) {
            if (api == null) {
                api = new ApiFActory().getSyncApi();
            }
        }
    }

    public static SyncApi getInstance() {
        return api;
    }
}

Use of the class would look like:

SyncApi api = Api.getInstance();
api.logOn("jjnguy", "pa55w0rd");

Answer 1

In Java there’s an established idiom for creating a thread-safe singleton, due to Bill Pugh:

public class Api {
    private Api() { }

    private static class SingletonHolder { 
        public static final SyncApi INSTANCE = new ApiFactory().getSyncApi();
    }

    public static SyncApi getInstance() {
        return SingletonHolder.INSTANCE;
    }
}

This implementation is both lazy and completely thread-safe, without the need to explicit synchronization and checking. This has the advantage that it eliminates the possibility of subtle race conditions and redundant locking.

Answer 2

The best and simple method was gived by Joshua Bloch in 《Effective Java 2th edition》

public enum Api {
    INSTANCE;
}

you can use it like this

Api api = Api.INSTANCE;

Answer 3

If you want thread-safe and lazy, I completely agree with the answer provided by Konrad Rudolph.

If you want just thread-safe without laziness (which is also very common), then you can initialize INSTANCE in the upper class, not from inner one.

Here is a good article with many tests and examples:
http://www.javaworld.com/javaworld/jw-04-2003/jw-0425-designpatterns.html

But both approaches don't give you an absolute singleton, since different ClassLoaders will create different instances. It might look like a nearly impossible case, but in my experience I faced a bug which was cause by such a situation.

Here is an article about absolute singletons:
http://surguy.net/articles/communication-across-classloaders.xml

Answer 4

I would have done this:

public class Api {

    private static SyncApi api = new ApiFActory().getSyncApi();

    public static SyncApi getInstance() {
        return api;
    }
}

Answer 5

Well, this is definitely not a Singleton, because this code is legal: Api.api=new Api();

You should at least define api variable as final and at least private constructor to avoid instantiation.

See @Konrad Rudolph's for proposed solution.

Answer 6

I did like this:

public class Api{

    private static SyncApi api=new ApiFActory().getSyncApi();

    private Api(){
    }

    public static SyncApi getInstance() {
        if(api!=null)
            return api;
    }
}

Answer 7

I agree with all the other answers but your problem, as you state it, is not to have a single API instance but to have a single SyncApi instance.

You will have no way to prevent a user from doing:

SyncApi api = new ApiFActory().getSyncApi();

You have to make SyncApi a Singleton or encapsulate it's usage in your Api class and not give access to it outside your Api...