This post is part of the “Legacy Code to Testable Code” series. In the series we’ll talk about making refactoring steps before writing unit tests for legacy code, and how they make our life easier. Other posts included:
The Legacy Code To Testable Code Series
| General patterns | Accessibility | Dealing with dependencies | Advanced patterns |
|---|---|---|---|
| Introduction | Add setters | Extract method | Static constructors (initializers) |
| Renaming | More accessors | Extract class | More static constructors |
| Add overload | Introduce parameter | Instance constructors | |
| Testable object | Conditionals to guard blocks |
As with renaming, extracting a method helps us understand legacy code better. If you find it easy to name the method, it makes sense. Otherwise, youv’e just enclosed code that does a lot of things. It can be useful sometimes, although not as extracting small methods that make sense from the legacy code.
Extracting a method also introduces a seam. This method can now be mocked, and can now be used to control the code in the unit tests. One of the tricks when not using power-tools is wrapping a static method with an instance method.
In our Person class, we have the GetZipCode method:
public class Person {
String street;
public String getZipCode() {
Directory directory = Directory.getInstance();
return directory.getZipCodeFromStreet(street);
}
}
The Directory.getInstance() method is static. If we extract it to a getDirectory method (in the Person class) and make this method accessible, we now can mock it.
public class Person {
String street;
public String getZipCode() {
Directory directory = getDirectory();
return directory.getZipCodeFromStreet(street);
}
protected Directory getDirectory() {
return Directory.getInstance();
}
}
While it’s now very easy to mock the getDirectory method using Mockito, it was also easy to mock the Directory.getInstance if we used PowerMockito. Is there an additional reason to introduce a new method?
If it’s just for the sake of writing unit tests – there’s no need to do the extraction. Sometimes, however mocking things with power-tools is not easy. Problems appearing in static constructors/initializers may require more handling on the unit test side. It may be easier to wrap in a separate method.
There are times when extracting methods helps us regardless of the mocking tool. We can use method extraction to simplify the unit test, even before we’ve written it. It’s simpler and safer to mock one method, rather than 3 calls.
If our getZipCode method looked like this:
public String getZipCode() {
Address address = new Address();
address.setStreet(street);
address.setCountry(country);
address.setState(state);
address.setCity(city);
Directory directory = Directory.getInstance(address);
return directory.GetZipCode();
}
Even with power-tools, faking the Address instance and setting the rest of the behavior settings just for retrieving the directory is quite a lot of work, which means a longer unit test with a long setup. If we extract a getDirectoryFromAddress method:
public String getZipCode() {
Directory directory = getDirectoryFromAddress();
return directory.GetZipCode();
}
We get more readable code, and we’ll need to mock only one line.
While extracting methods in legacy code has its upside, making a method a seam comes with some baggage. If the method is private, and we use power tools to mock it, coupling between the unit test and code is increased. If we make it public, someone can call it. If it’s protected, a derived class can call it. Changes for testability is a change of design, for better or worse.
Up next: Creating accessors.
Image source: http://commons.wikimedia.org/wiki/File:Dentist_extracting_tooth..JPG
2 Comments
rliesenfeld · October 24, 2014 at 8:37 pm
I think you should show the Mockito test for the refactored method. It will have to use a spy in order to mock the extracted method, and use of spies (aka “partial mocking”) is not a recommended practice. In general, you want to mock a dependency of the class under test, and not the class under test itself. The article should be explicit about this downside.
Gil Zilberfeld · October 25, 2014 at 8:02 am
Thank you for the suggestion!
Like any technique, it’s about context. And it’s not about what I want, it’s about what I have. I may have a whole lot of freedom, and I can go to extract class. When I have constraints about code changes, I may choose to use partial mocking.
The constraints are not in the code – this week I worked with a client who’s afraid to change other people’s code. His manager backs him up. I was allowed to extract a method on his class, but extracting to another class was not “acceptable”.
Work within the limitations.