The Singleton Killer

We all hate singletons so here's another useful refactoring pattern: The Singleton Killer.

Here's a common snippet of code we all love to hate:

class Transfer
 def funds(from_number, to_number, amount)
   from = AccountRepository.instance.get(from_number)
   to = AccountRepository.instance.get(to_number)
   from.balance -= amount
   to.balance += amount
 end
end

So how do we kill this beast? Simply follow these idiot proof steps:

Step One: Introduce Field

Stop asking the singleton for it's reference and store your own by creating a field:

class Transfer
 @repository = AccountRepository.instance
 def funds(from_number, to_number, amount)
   from = repository.get(from_number)
   to = repository.get(to_number)
   ...
 end
end

Wow: it's starting to look like normal code!

Step Two: Initialize in constructor

Move the initialization to the constructor:

def initialize()
 @repository = AccountRepository.instance
end

This is good but wouldn't it be better if the dependancy could be injected?

Step Three: Chain constructor

In languages where dependency injection is useful you will need to chain the constructors:

public Transfer() : this(AccountRepository.instance)
public Transfer(AccountRepository repository)
{
 this.repository = repository
}

Now the AccountRepository can be injected which of course means we can mock it out!

Step Four: Extract interface (for statically typed languages)

Except statically languages: they'll need to extract an interface:

public interface IAccountRepository
{
 Account Get(string number);
}

public Transfer(IAccountRepository repository) ...

Good: now you can start mocking and testing.

Step Five: Introduce container

If you haven't already got a container then get one (else ensure you a wiring up your dependencies in one place). Then get that to wire up the Transfer object for you:

container.Add(AccountRepository)
container.Add(Transfer)
transfer = container.Get(Transfer)
transfer.funds("YOUACCNUM", "MYACCNUM", 50000)

Step Six: Remove singleton

Once a container is doing all your injection for you that singleton is a waste of space: get rid of him by removing that horrible instance accessor.

Choose your strategies with this refactoring wisely: if your objective is to get a class tested then first repeat steps one-four within the same class until it is clean of all singletons. This is a good exercise in it's own right - even if you're not going to move on to test or remove the singleton completely - as you start to move all the dependancies to the constructor you gain a better picture of the classes collaborators (and the quality of design). On the other hand if your objective is to remove the singleton repeat steps one-three on every usage you can find then finish up with steps four-six. Another technique is to keep pushing the useage of the singleton up through the code so each client then takes on responsibility for passing the singleton down when it creates an instance of the class. Like so:

  Transfer.new(AccountRepository.Instance)

Then repeat steps one-three on the class you've just pushed into, then push the singleton up again up and so on until you can't push it anymore. This can be an interesting exercise in itself as it starts to push out the paths through the code base as you move through the layers and may also give you a clear point where your container needs to be. Be warned though: it can get messy (though strangely satisfying)!

On larger, messier codebases none of these approaches may be realistic in which case opt for the guerilla approach and pick them off as you find them. Though I would encourage trying to at least clean out a whole class using steps one-three even if you won't end up testing it right away. With all your dependancies in one place you'll still have a better picture of the design.