Wednesday, 1 August 2007

Refactor singleton out of your code

There are a lot of posts about that the Singleton design pattern [GoF] is not pattern but anti-pattern (eg. SINGLETON - the anti-pattern!). Some posts also proposes an alternatives for Singleton design pattern or solutions for already existing Singleton design pattern problem (eg. Patterns I Hate #1: Singleton). But I didn't found any post which describes solution for big project with bunch of singleton class usage without big effort of rewriting many places by hand.

Because I was in situation that I should refactor a big project containing bunch of Singletons, I was thinking about some automatic way how to remove them. I found a solution using several refactorings. After bellow described sequence of steps you can replace Singleton design pattern by Registry design pattern [P of EAA] which solves all crucial problems of a Singleton, like testing possibility, singleton subclassing, ....

Lets have simple singleton class which we want to refactor to access it's instance not as a singleton but using some registry.

public class SingletonClass {
private static final SingletonClass INSTANCE = new SingletonClass();

public static SingletonClass getInstance() {
return INSTANCE;
}

private SingletonClass() {
super();
}

public void voidMethod(String param) {
System.out.println("Doing something in method returning void");
}

public Object objectMethod(String param) {
System.out.println("Doing something in method returning Object");
return new String("This is an String");
}
}

and also lets have very simple client class which access the singleton

public class Client {
public void clientMethod() {
SingletonClass.getInstance().voidMethod("param");
Object object = SingletonClass.getInstance().objectMethod("param");
}
}
  • At the first I create simple interface for Registry with two simple methods - getter and setter for instance of class which is currently implemented as singleton (SingletonClass in my example). There are also other variants of Registry implementation. For example you can have one generic methods getService and setService with some selector as a parameter (String ID, service class, ...). But I choosed the simplest one.

    public interface ISingletonRegistry {
    SingletonClass getSingletonClass();
    void setSingletonClass(SingletonClass singleton);
    }

  • Make SingletonClass's constructor public

  • Then implement the interface as a simplest Registry design pattern implementation - SingletonRegistry.

    public class SingletonRegistry implements ISingletonRegistry {

    private static final SingletonRegistry INSTANCE = new SingletonRegistry();

    private SingletonClass singleton;

    public static ISingletonRegistry getInstance() {
    return INSTANCE;
    }

    private SingletonRegistry() {
    //this is the reason why the constructor should be public
    singleton = new SingletonClass();
    }

    public SingletonClass getSingletonClass() {
    return singleton;
    }

    public void setSingletonClass(SingletonClass singleton) {
    this.singleton = singleton;
    }
    }

    You can see that I implemented it as singleton (of course using practise described in one of my previous posts - Singleton pattern implementation in 4 steps :-). I know that a lot of readers will have animadversions that I am creating new singleton when I want to remove other singletons. But realize that singleton is not evil every time. Also Service Locator design pattern uses singleton to access ServiceLocator instance.
    You can also see that constructor of the class initializes reference to SingletonClass instance. Of course this is just possibility. You can use some mechanism of configuration of the singleton instances. But note that it should be performed before you use SingletonRegistry by some client for the first time by. Anyway this implicit setup can stay here because you are able to set different instance of SingletonClass using setSingletonClass method.

  • Change getInstance method of SingletonClass to get instance from SingletonRegistry.

    public static SingletonClass getInstance() {
    return SingletonRegistry.getInstance().getSingletonClass();
    }

  • Over getInstance method of SingletonClass perform Inline... refactoring (Alt+Shift+I), mark 'Delete method declaration' chekbox and click OK. The method getInstance from SingletonClass disappears and all it's client classes uses SingletonRegistry to access SingletonClass.

    public class Client {
    public void clientMethod() {
    SingletonRegistry.getInstance() .getSingletonClass().voidMethod("param");
    Object object = SingletonRegistry.getInstance() .getSingletonClass().objectMethod("param");
    }
    }

  • Delete INSTANCE constant from SingletonClass class.

  • As a cherry on top of a cake you can Extract Interface (Alt+Shift+T, E) from SingletonClass. Write just new interface name, select all methods from SingletonClass which you can extract into new interface and press OK. All references to SingletonClass will be refactored to references to your newly created interface.
And that is all. Now you have SingletonClass with totally same functionality but you are able to mock it, extend or replace by different implementation setting up your instance of SingletonClass to SingletonRegistry.

References:

AddThis Feed Button 14 comments:

WarpedJavaGuy said...

Good motive :) but instead of just refactoring singletons, it would be better to factor them out completely. This can be achieved by registering the singleton class in an IoC container (like Spring, HiveMind, PicoContainer, etc..) that can inject the instance into the code. This would give you the effect of both a registry and a singleton without the associated code smell.

Anonymous said...

idiotic abstract OOP patterns, again

every week some philosopher of code (those who never had a real world commercial application released in ther lives) comes up with a new one, or blogs how anti- a used pattern really is...

gimme some global variables and functions and go write some smart algo, if you can

Prashant Jalasutram said...

Hi,

But making SingletonDemo class constructor public is obvously breaking singleton purpose ? what will happen if user tries to create SingletonClass directly?

please do correct me if i am wrong. But really appreciate your thought reqarding this.

Thanks
Prashant

WarpedJavaGuy said...

@anonymous,
We have these discussions here because we want to encourage better programming practices and care about OO and its future.

Anonymous said...

I don't think this helps the UNIT testability of your code. The client method internally calls the Singleton thus making this an implicit dependency rather than an explicit one (passed via parameters). That's what makes singletons so devious. They assume a lot about their environment.

Roman Bosák said...

@Prashan Jalasutram
Yes, you are right, it can happen. Developer who don't know that the SingletonClass should be single instance can create it's own instance. Hmm, I should revalue the solution.

Roman Bosák said...

@anonymous
You are right that the client uses singleton, but it uses singleton of the Registry, not of the SingletonClass, so you can mock it.

chubbard said...

You've just moved the problem. Global registries have all the same bad properties as Singletons. The problem with singleton is that lower level classes are making architectural decisions about dependencies, and that does not make for an malleable program. Why not just pass what he needs in to the constructor, and let something higher up make the decision of what that is, and how many instances you should have kicking around?

Kit said...

Also remember the singleton holder pattern, to enable thread-safe singleton creation:
public class MySingleton
{
private MySingleton()
{
//...
}

public static MySingleton getInstance()
{
return Holder.instance;
}

private static class Holder
{
static final MySingleton instance = new MySingleton();
}

}

(how do I format code?!)

H said...

I don't think this solution is of any use if:

1) It replaces 1 singleton with another singleton (yes we can call it registry but it still has all the problems of a singleton). Even if this means we replace N singletons with just 1, the same problems still apply.
2) You're forced to have a public constructor on a class you wanted to control instantiation. "The developer should know" is not the right answer in my opinion; the developer should know not to use global variables but we still use private to make sure the compiler enforces it, don't we ?

interesting post on singleton:
http://tech.puredanger.com/2007/07/03/pattern-hate-singleton/

Anonymous said...

The Registry has a fatal flaw, a setter method when it should be immutable, otherwise you can get so many side effects that will scream with shock! A properly designed Singleton is far better than any registry and can easily hide all the complications found in multi-threaded code e.g. by selective use use of the ThreadLocal class in Java.

Java has plenty of necessary Singletons e.g. Class, Thread, Runtime etc., which it cannot do without, so stop spreading FUD, kill the dogma and learn how to use Singletons properly!

Design for MySpace said...

typically the purpose of singletondemo constructor is nullified since it has been declared public.

It has an arbitary effect when user creates a singleton class

Anonymous said...

It's better to use "abtracton" in this case. It makes it more dip-able.

public abstract class Abstracton
{
private static Abstractor s_instance;


public static Abstracton getInstance()
{
if (s_instance == null)
{
throw new IllegalStateException("there's no Abstracton instance");
}
return s_instance;
}


protected Abstracton()
{
if (s_instance != null)
{
throw new IllegalStateException("an instance of is already created");
}
s_instance = this;
}


public abstract void theMethod(final Object arg);


protected final void doDispose()
{
if (s_instance != this)
{
throw new IllegalStateException("i dont know how you did it, but you did!");
}
s_instance = null;
}
}


Subclass Abstracton, implement theMethod(s) that you want. Make a public method that calls dispose for clean up which the creator only know about. The client will only have access to the public methods of Abstracton.

In general I think this is a better aproach, since the abstracton itself can be a mock as well and the instance control is hidden for the client.

A big backdraw with this is of course if using to many of these will cause a lot of dry vialotations.

// Drutt

Peter Kofler said...

Thank you for this piece of information. I used it in my presentation about Practical Unit Testing given at the local Java Student User Group last year.