Legacy Code To Testable Code #2: Extract Method

Standard

This post is part of the “Legacy Code to Testable Code” series. In the series we’ll talk about making refactoring steps before writing tests for legacy code, and how they make our life easier. Other posts include:

The Legacy Code To Testable Code Series

General patternsAccessibilityDealing with dependenciesAdvanced patterns
IntroductionAdd settersExtract methodStatic constructors (initializers)
RenamingMore accessorsExtract classMore static constructors
Add overloadIntroduce parameterInstance constructors
Testable objectConditionals to guard blocks

As with renaming, extracting a method helps us understand the code better. If you find it easy to name the method, it makes sense. Otherwise, you just enclosed code that does a lot of things. It can be useful sometimes, although not as extracting small methods that make sense.

Extracting a method also introduces a seam. This method can now be mocked, and can now affect the code as it being tested. 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:


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.


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 testing – there’s no need to do the extraction. Sometimes, however mocking things with power-tools is not easy. Problems appearing in static constructors may require more handling on the test side. It may be easier to wrap in a separate method.

There are times when extracting helps us regardless of the mocking tool. We can use method extraction to simplify the 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:


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 test with a long setup. If we extract a getDirectoryFromAddress method:


We get more readable code, and we’ll need to mock only one line.

While extracting has its up side, making a method a seam comes with the baggage. If the method is private, and we use power tools to mock it, coupling between 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 thoughts on “Legacy Code To Testable Code #2: Extract Method

  1. rliesenfeld

    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.

  2. 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.

Leave a Reply

Your email address will not be published. Required fields are marked *