HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Request for Comments: Singleton pattern implemented in Java

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
requestimplementedjavasingletonforcommentspattern

Problem

I have several utility classes who should only ever have one instance in memory, such as LogHelper, CacheHelper, etc. One instance to rule them all.

I've never implemented Singleton in Java before, so I wanted to post my first pass and see if anybody had major issues with this, or just helpful critiques. I want to make sure that my design will have the intended affect when I go to use these singleton classes throughout my codebase.

Singleton:

public class Singleton
{
    private static Singleton instance;

    protected Singleton()
    {
        super();
    }

    public static synchronized Singleton getInstance()
    {
        if(instance == null)
            instance = instantiate();

        return instance;
    }

    // Wanted to make this abstract and let subclasses @Override it,
    // but combining 'abstract' and 'static' is illegal; makes sense!
    protected static Singleton instantiate()
    {
        return new Singleton();
    }
}


LogHelper (a singleton since all my classes will use it to write messages to the same destinations):

public class LogHelper extends Singleton
{
    protected static LogHelper instantiate()
    {
        return new LogHelper(...blah...);
    }

    public LogHelper(...blah...)
    {
        // Initialize the singleton LogHelper
    }

    // More methods for logging, etc.
}


Here's how I'd like to use it throughout my codebase:

LogHelper logHelper = LogHelper.getInstance();
logHelper.logInfo("...");


What I'm worried about is that I may have inadvertently designed this wrong and heading down the wrong avenue. Can anybody weigh-in on my design?

Thanks!

Solution

The best pattern for singleton that is guaranteed to be thread safe and minimizes lock overhead (because it has none) is this:

public class Singleton {
    private Singleton() {}

    private static class InstanceHolder {
        static Singleton INSTANCE = new Singleton();
    }

    public static Singleton getInstance() {
        return InstanceHolder.INSTANCE;
    }
}


This is lazy initialized but carries no locking overhead. It is guaranteed safe by the Java Memory Model: The inner static class is not loaded until the first call to getIinstance() and all static initializtion is guaranteed to be completed before the class is used, therefore INSTANCE is fully constructed when first accessed.

Note: The double checked locking pattern is BROKEN - see this article that explains exactly why.

Code Snippets

public class Singleton {
    private Singleton() {}

    private static class InstanceHolder {
        static Singleton INSTANCE = new Singleton();
    }

    public static Singleton getInstance() {
        return InstanceHolder.INSTANCE;
    }
}

Context

StackExchange Code Review Q#3920, answer score: 8

Revisions (0)

No revisions yet.