Creating Stubs for Legacy Code - Testing Techniques 6
Any one who reads this blog will probably have realised that at present
I’m working on a project that contains a whole bunch of legacy code
that’s large, expansive, and written without any tests what so ever. In
working with this legacy code, there’s been one very badly behaved class
that’s all pervasive, which the whole team have tripped over time and
time again. In order to protect the guilty, I’ll call it Mr. X although its real name is SitePropertiesManager as it manages the web site’s properties. It’s badly behaved because it:
- breaks the single responsibility principal
- has uses singleton pattern summarised by a getInstance() factory method,
- has an init() method that must be called before any other method,
- loads its data using direct access to the database rather than using a DAO,
- uses an complex map of maps to store its data,
- accesses the file system to cache data returned by the database
- has a timer to decide when up update its cache.
- was written before generics and has a multitude of superfluous findXXXX() methods.
- doesn’t implement an interface,
- employs lots of copy and paste programming.
SitePropertiesManager propman = SitePropertiesManager.getInstance();
This blog takes a look at ways of dealing with awkward characters and demonstrates how to create stubs for them, whilst negating the effect of the Singleton pattern. As with my previous Testing Techniques blogs, I’m basing my demo code on my Address web app sample1.

In the other blogs in this series, I’ve been demonstrating how to test the AddressService and this blog is no different. In this scenario, however, the AddressService has to load a site property and decide whether or not to return an address, but before we take a look at that I first of all needed the badly written SitePropertiesManager to play with. However, I don’t own that code, so I’ve written a stunt-double version, which breaks as many rules as I can think of. I’m not going to bore you with the details here as all the source code for SitePropertiesManager is available at: git://github.com/roghughe/captaindebug.git
As I said above, in this scenario, the AddressService is using a site property to determine if it’s enabled. If it is, then it’ll send back an address object. I’m also going to pretend that the AddressService is some legacy code that uses the site properties static factory method as shown below:
public Address findAddress(int id) {
logger.info("In Address Service with id: " + id);
Address address = Address.INVALID_ADDRESS;
if (isAddressServiceEnabled()) {
address = addressDao.findAddress(id);
address = businessMethod(address);
}
logger.info("Leaving Address Service with id: " + id);
return address;
}
private boolean isAddressServiceEnabled() {
SitePropertiesManager propManager = SitePropertiesManager.getInstance();
return new Boolean(propManager.findProperty("address.enabled"));
}In taming this type of class, he first thing to do is to stop using getInstance() to get hold and an object, removing it from the above method and to start using dependency injection. There has to be at least one call to getInstance(), but that can go somewhere in the program’s start-up code. In the Spring world, the solution is to wrap a badly behaved class in a Spring FactoryBean implementation, which becomes the sole location of getInstance() in your application - at least for new / enhancement code.
public class SitePropertiesManagerFactoryBean implements
FactoryBean<SitePropertiesManager> {
private static SitePropertiesManager propsManager = SitePropertiesManager
.getInstance();
@Override
public SitePropertiesManager getObject() throws Exception {
return propsManager;
}
@Override
public Class<SitePropertiesManager> getObjectType() {
return SitePropertiesManager.class;
}
@Override
public boolean isSingleton() {
return true;
}
}This can now be autowired into the AddressService class:
@Autowired
void setPropertiesManager(SitePropertiesManager propManager) {
this.propManager = propManager;
}These changes, however, don’t mean that we can write some proper unit tests for AddressService, they just prepare the ground. The next step is to extract an interface for SitePropertiesManager, which is easily achieved using eclipse.
public interface PropertiesManager {
public abstract String findProperty(String propertyName);
public abstract String findProperty(String propertyName, String locale);
public abstract List<String> findListProperty(String propertyName);
public abstract List<String> findListProperty(String propertyName, String locale);
public abstract int findIntProperty(String propertyName);
public abstract int findIntProperty(String propertyName, String locale);
}In moving to interfaces, we also need to manually configure an instance of SitePropertiesManager in the Spring config file so that Spring knows which class to connect to what interface:
<beans:bean id="propman" class="com.captaindebug.siteproperties.SitePropertiesManager" />
We also need to update the AddressService’s @Autowired annotation with a qualifier:
@Autowired
@Qualifier("propman")
void setPropertiesManager(PropertiesManager propManager) {
this.propManager = propManager;
}With an interface, we can now easily write a simple SitePropertiesManager stub:
public class StubPropertiesManager implements PropertiesManager {
private final Map<String, String> propMap = new HashMap<String, String>();
public void setProperty(String key, String value) {
propMap.put(key, value);
}
@Override
public String findProperty(String propertyName) {
return propMap.get(propertyName);
}
@Override
public String findProperty(String propertyName, String locale) {
throw new UnsupportedOperationException();
}
@Override
public List<String> findListProperty(String propertyName) {
throw new UnsupportedOperationException();
}
@Override
public List<String> findListProperty(String propertyName, String locale) {
throw new UnsupportedOperationException();
}
@Override
public int findIntProperty(String propertyName) {
throw new UnsupportedOperationException();
}
@Override
public int findIntProperty(String propertyName, String locale) {
throw new UnsupportedOperationException();
}
}From having stub, it’s pretty easy to write a unit test for the AddressService that uses the stub and is isolated from the database and the file system
public class AddressServiceUnitTest {
private StubAddressDao addressDao;
private StubPropertiesManager stubProperties;
private AddressService instance;
@Before
public void setUp() {
instance = new AddressService();
stubProperties = new StubPropertiesManager();
instance.setPropertiesManager(stubProperties);
}
@Test
public void testAddressSiteProperties_AddressServiceDisabled() {
/* Set up the AddressDAO Stubb for this test */
Address address = new Address(1, "15 My Street", "My Town", "POSTCODE",
"My Country");
addressDao = new StubAddressDao(address);
instance.setAddressDao(addressDao);
stubProperties.setProperty("address.enabled", "false");
Address expected = Address.INVALID_ADDRESS;
Address result = instance.findAddress(1);
assertEquals(expected, result);
}
@Test
public void testAddressSiteProperties_AddressServiceEnabled() {
/* Set up the AddressDAO Stubb for this test */
Address address = new Address(1, "15 My Street", "My Town", "POSTCODE",
"My Country");
addressDao = new StubAddressDao(address);
instance.setAddressDao(addressDao);
stubProperties.setProperty("address.enabled", "true");
Address result = instance.findAddress(1);
assertEquals(address, result);
}
}All of which is pretty hunky-dory, but what happens if you can’t extract an interface? I’m saving that for another day...
1 The source code is available from GitHub at:
git://github.com/roghughe/captaindebug.git
From http://www.captaindebug.com/2011/11/creating-stubs-for-legacy-code-testing.html
(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)






Comments
Andrew Spencer replied on Thu, 2011/12/01 - 4:16am
I take issue with the approach of extracting an interface and writing the stub by hand. With tools like Mockito, you can stub concrete classes. This generally involves less boilerplate than writing stubs by hand. And, it means that we no longer need to program to interfaces for reasons of testability alone.
Programming to interfaces has been promoted as a good design practice, independent of its effect on testability. I held that belief myself not so long ago. With the benefit of greater experience, I now think that it's justified only when there is a genuine need -- in the production code -- for the abstraction that the interface represents. A genuine need means either that we are going to write multiple implementations ourselves, or that we are giving client code the possibility to provide an implementation. If there's only ever going to be one real implementation, I'd stick with a concrete class.
Spring Batch is a good example of a case where programming to interfaces is appropriate: a flexible library, that client code can extend and customise almost without limitation. A service class in a business app, on the other hand, is never going to be exported outside the application, and it's very unlikely that there'll ever be multiple implementations of its API.
Props btw for going to the trouble of writing a stunt double. I've often seen dodgy classes in customers' projects that I've wished I could put in a blog post, if only for the purpose of letting off steam, but I'm too lazy to write stunt doubles. (Or maybe it is just that I can't bring myself to add to the quantity of bad code in the world, even for example purposes. Yes, that must be it.)
Roger Hughes replied on Thu, 2011/12/01 - 10:22am
in response to:
Andrew Spencer
Thanks for the comments. Yes, you're right, there are tools available for writing stubs more easily, but I really wanted to demonstrate the technique of using stubs, which some people are not aware of, and to show what functionality they should contain. In my blog on Mocks I demonstrated this by writing a homemade mock and then doing the same thing using EasyMock.
I agree that there's no point in programming to interfaces just for the sake of it; however, I'm also of the opinion that test code should be treated as a genuine client of the production code and that a test implementation should be treated as a real implementation. If extracting an interface from a third party class for test purposes means that my code is tested then that's not a bad trade off, but it shouldn't be done unnecessarily. In terms of writing homemade stubs I think that I prefer the idea of using inheritance, which was published earlier today. This is just smaller, simpler and neater than extracting an interface.
...and also I have to say that I found trying to write code badly for the stunt double really hard.