Here comes the time for round 2 of our fight with Singleton. This time, we will try to get rid off unwanted, hard-wired dependency, by applying Michael Feathers's trick.
During first round, we used a simple, wrapping mechanism, enabling developer to disjoin Singleton from class under test. Although it is not complicated, it does amend production code in few places. What Michael Feathers has suggested in his book "Working Effectively with Legacy Code", is a slightly different approach. It also does changes in production code, but just in Singleton class itself. One may say it is a less invasive way of dealing with that sort of problem.
Anyway, let's get started in the same place, we started last time:
public class PropertiesCache { private static PropertiesCache instance = new PropertiesCache(); private PropertiesCache() { } public static PropertiesCache getInstance() { return instance; } public boolean overrideWith(File fileProperties) { return someWeirdComplicatedFilePropertiesLogic(fileProperties); } private boolean someWeirdComplicatedFilePropertiesLogic(File fileProperties) { if (fileProperties.length() % 2 == 0) { return true; } return false; } }
public class SamplePropertiesCacheUsage { public boolean overrideExistingCachePropertiesWith(File fileProperties){ PropertiesCache cachedProperties = PropertiesCache.getInstance(); return cachedProperties.overrideWith(fileProperties); } }I added a static setter to PropertiesCache class using InteliJ tool called: code - generate setter. Second move is a manual change of constructor's modifier: from private to protected.
public class PropertiesCache { private static PropertiesCache instance = new PropertiesCache(); protected PropertiesCache() { } public static PropertiesCache getInstance() { return instance; } public static void setInstance(PropertiesCache instance) { PropertiesCache.instance = instance; } public boolean overrideWith(File fileProperties) { return someWeirdComplicatedFilePropertiesLogic(fileProperties); } private boolean someWeirdComplicatedFilePropertiesLogic(File fileProperties) { if (fileProperties.length() % 2 == 0) { return true; } return false; } }Now, I created two classes inheriting from the Singleton. They stub the overrideWith method. As you can see, there is also a simple, but valuable test created.
public class StubbedForTruePropertiesCache extends PropertiesCache { @Override public boolean overrideWith(File fileProperties) { return true; } }
public class StubbedForFalsePropertiesCache extends PropertiesCache { @Override public boolean overrideWith(File fileProperties) { return false; } }
public class SamplePropertiesCacheUsageTest { private File dummyFileProperties; private SamplePropertiesCacheUsage propertiesCache; @BeforeMethod public void setUp() { dummyFileProperties = new File(""); propertiesCache = new SamplePropertiesCacheUsage(); } @Test public void shouldReturnTrueDueToWeirdInternalSingletonLogic() { PropertiesCache.setInstance(new StubbedForTruePropertiesCache()); boolean result = propertiesCache.overrideExistingCachePropertiesWith(dummyFileProperties); assertThat(result, is(equalTo(TRUE))); } @Test public void shouldReturnFalseDueToWeirdInternalSingletonLogic() { PropertiesCache.setInstance(new StubbedForFalsePropertiesCache()); boolean result = propertiesCache.overrideExistingCachePropertiesWith(dummyFileProperties); assertThat(result, is(equalTo(FALSE))); } }That's all. We have relaxed the coupling between Singleton and system under test. We have tests. Design is also improved a bit. We reached our goal.
As previously, you can find entire refactoring exercise on my GitHub account.
Round two is finished.