Wrestling with Singleton - Round 1

How many times you were dealing with Singletons in your codebase. To be frank, it has been always a problem to properly understand the nature of Singleton, its usage and refactoring methods. Singleton, as such is not an embodied evil. It is rather the usage that developers think they "design". 

In order to fully understand the problem, let's have a quick look on Gang of Four (GoF) Singleton definition:
"Ensure a class only has one instance, and provide a global point of access to it.". 

The big hoo-ha is focusing on second part of above definition: "... providing a global point of access to it (to a single object).". In GoF's implementation, they provided a global point of access, by taking the advantage of static getInstance() method. While it perfectly well fulfills assumptions and leading concept of Singleton definition, it also introduces "an extra, unwanted feature" i.e. a global state visible to every class. 

Well, some pesky guy, with devil-may-care attitude may say, so what! Apparently nothing, however I can bet that such a smart alec has never written a single line of unit test, especially in legacy code. The thing is people invented Singleton pattern to maintain a single instance of the object among the entire set of object graphs aka an application. Providing a global access point in a correct way is slightly more tricky to materialize, than just using static getInstance() method. However, it is still feasible to do it in a right way.

Have you ever thought, why nobody finds faults with Spring or with any other Dependency Injection (DI) framework? People do not moan about DIs libraries, even though there is a way to make an object a singleton. Now, it sounds odd! In fact, the answer is hidden in lowercase singleton. Spring is able to create, maintain and inject a singleton object, without exposing it as a global state. It is worth to notice that Spring deals with Singleton problem, correctly. It not only meets GoFs definition without adding any unnecessary burden i.e. no static getInstance() method, but also provides desired inversion of control. It is an xml configuration, which is enabling us to mark a bean as a singleton and that is it. If you want to use it, you will have to inject it, as any other bean via constructor, setter or field. DI framework, in its construction, promotes testability by enforcing the concept of seam.

If you are a bit nosy person, you should ask this sort of question: is it the only, correct way I can use singletons? Obviously, the answer is: no, it is not. The reason why DI framework makes better use of singletons is the fact that it combines single instance of some class with dependency injection.
If you do not want to use Spring for some reason or it is simply an overkill for your solution, then there are at least two ways you can choose. You can either use the 'wrap the Singleton' or 'inherit from singleton' approach. In this article, I will focus on the former one. In a nutshell, it is a dependency injection for poor man going along with Singleton. Incidentally, it is quite powerful technique, when it comes to legacy code refactoring. Let's have a look on a model GoF's implementation of Singleton pattern and its usage in sample legacy code: 

 
   public class PropertiesCache {

	private static PropertiesCache instance = new PropertiesCache();

	private PropertiesCache() {

	}

	public static PropertiesCache getInstance() {
		return instance;
	}

	public void overrideWith(File fileProperties) {
		// some logic comes here
	}
 }

 
public class SamplePropertiesCacheUsage {

	public void overrideExistingCachePropertiesWith(File fileProperties){
		PropertiesCache cachedProperties = PropertiesCache.getInstance();
		cachedProperties.overrideWith(fileProperties);
	}
}

It is a very simple and extremely common scenario, which shows the tight coupling between SamplePropertiesCacheUsage and Singleton classes. Bear in mind that Singleton might be quite substantial in size, as it is a properties cache, all in all. Moreover, some cunning developer before you, might have armed Singleton with quite a few "handy methods" for loading properties from file, merging them from different sources applying precedence policies on a top of that etc. Generally speaking, nothing pleasant and it is you, who have to wrestle with that code, now.

Let's assume that our goal is to get rid off that tight dependency to Singleton. Second, more implicit assumption is that our IDE will slightly change Singleton call in our production code.

Okay, let's get started. First thing we should do is to look for test for SamplePropertiesCacheUsage. Wait a second, but we are going to start our digging in legacy code, so do not even bother to look for any test. It might have been quite difficult to write such test for developer, anyway. As a matter of fact, we quickly found that we have to refactor using IDE's built in methods.

In my InteliJ IDE it will be a few steps process. First of all, let's extract a private method called getInstance(), encapsulating Singleton static call. This method is not static, any more.

 
public class SamplePropertiesCacheUsage {

	public void overrideExistingCachePropertiesWith(File fileProperties){
		PropertiesCache cachedProperties = getInstance();
		cachedProperties.overrideWith(fileProperties);
	}

	private PropertiesCache getInstance() {
		return PropertiesCache.getInstance();
	}
}

Our next step will be to extract a PropertiesCacheWrapper class with public getInstance() method, from SamplePropertiesCacheUsage Singleton client.

 
public class SamplePropertiesCacheUsage {

	private PropertiesCacheWrapper propertiesCacheWrapper = new PropertiesCacheWrapper();

	public void overrideExistingCachePropertiesWith(File fileProperties){
		PropertiesCache cachedProperties = propertiesCacheWrapper.getInstance();
		cachedProperties.overrideWith(fileProperties);
	}

	private PropertiesCache getInstance() {
		return propertiesCacheWrapper.getInstance();
	}
}

 
public class PropertiesCacheWrapper {
	public PropertiesCacheWrapper() {
	}

	public PropertiesCache getInstance() {
		return PropertiesCache.getInstance();
	}
}

Now, it is time for initializing propertiesCacheWrapper field in the constructor. You may also need to manually delete inlined initialization of propertiesCacheWrapper field. 
This is actually the moment, when the injection of PropertiesCacheWrapper happens.

public class SamplePropertiesCacheUsage {

	private PropertiesCacheWrapper propertiesCacheWrapper;

	public SamplePropertiesCacheUsage(PropertiesCacheWrapper aPropertiesCacheWrapper) {
		propertiesCacheWrapper = aPropertiesCacheWrapper;
	}

	public void overrideExistingCachePropertiesWith(File fileProperties){
		PropertiesCache cachedProperties = propertiesCacheWrapper.getInstance();
		cachedProperties.overrideWith(fileProperties);
	}

	private PropertiesCache getInstance() {
		return propertiesCacheWrapper.getInstance();
	}
}

As a last step, we may delete getInstance() method from SamplePropertiesCacheUsage, as it is no longer used.

public class SamplePropertiesCacheUsage {

	private PropertiesCacheWrapper propertiesCacheWrapper;

	public SamplePropertiesCacheUsage(PropertiesCacheWrapper aPropertiesCacheWrapper) {
		propertiesCacheWrapper = aPropertiesCacheWrapper;
	}

	public void overrideExistingCachePropertiesWith(File fileProperties){
		PropertiesCache cachedProperties = propertiesCacheWrapper.getInstance();
		cachedProperties.overrideWith(fileProperties);
	}
}

Let's have a look on what happened. Now, we have Singleton invocation wrapped in a separate class. What is more, SamplePropertiesCacheUsage class does have a constructor type seam, which is used to inject  PropertiesCacheWrapper. The code is now at least testable, so we are able to write a test for SamplePropertiesCacheUsage class.

@RunWith(MockitoJUnitRunner.class)
public class SamplePropertiesCacheUsageTest {

	@Mock private PropertiesCache cachedProperties;
	@Mock private PropertiesCacheWrapper propertiesCacheWrapper;
	@Mock private File file;

	@BeforeMethod
	public void initializeMocks() {
		initMocks(this);
		given(propertiesCacheWrapper.getInstance()).willReturn(cachedProperties);
	}

	@Test
	public void shouldOverrideExistingPropertiesWithFileProperties() {
		SamplePropertiesCacheUsage samplePropertiesCacheUsage = new SamplePropertiesCacheUsage(propertiesCacheWrapper);

		samplePropertiesCacheUsage.overrideExistingCachePropertiesWith(file);

		verify(cachedProperties).overrideWith(file);
	}
}

Everything looks good, now. We have a unit test describing SamplePropertiesCacheUsage class, which was previously using static call to Singleton class. We also got rid off tight dependency to Singleton. 

You can find entire refactoring exercise on my GitHub account.

Round one is finished.