SIngleton object initialization and dependency injection

Question

I usually create singleton classes using singleton holder pattern like this:

public class Foo {

    private static final class SingletonHolder {
        private static final Foo INSTANCE = new Foo();
    }  

    private Foo(){}

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

}

Everything ok. But, what about if I need inject one dependency to initialize the singleton object?

In that case, I add one method initialize that receives the dependency and must be called just one time:

public class Foo {

    private static final class SingletonHolder {
        private static final Foo INSTANCE = new Foo();
    }  

    private Foo(){}

    public static final void initialize(Dependency d) {
        SingletonHolder.INSTANCE.setDependency(d);
    }

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

}

Am I in the right way? Is there another solution? I know that it depends on the program, my logic and so on... but how generally should be solved this problem?

Answer 1

The problem with this approach is that the initialize method can be invoked more than once, and there is no indication in your code that setting the dependency is being handled with thread safety.

If you're fine with that, then your lazy initialization idiom is fine.

Otherwise, you could throw an IllegalStateException or otherwise silently do nothing when the dependency is being set more than once within your singleton instance.

Edit

As Andy Thomas says, you also aren't checking whether the dependency is set before getInstance is invoked, or at least your code doesn't display it.

Answer 2

I think you are overcomplicating it. In a few places I worked (including my current one) we don't try to enforce singletons. All the wiring for the actual application is done in one place, so if you searched for usages of constructor you should find one in src and probably multiple in test. See the code below.

A few disadvantages of your approach:

• you lose immutability, Foo's dependency can change
• both setDependency and initialize methods are test code in production
• your constructor doesn't create a valid object, a second half of constructor is in initialize method which you have to remember to call after calling constructor
• SingletonHolder is a boiler plate code, I am not sure why don't you just declare public static final Foo instance field instead?
• it is still possible to create multiple instances of Foo using reflection API and object serialisation mechanisms

public class Foo {

    private final Dependency dependency;

    public Foo(Dependency dependency) {
        this.dependency = dependency;
    }

    // ...
}

public class Dependency {
    // ...
}