One of the observations that I made earlier is that refactoring for clean code as advocated by Robert C. Martin would greatly simplify unit testing. So let me give you an example, using some code I’m actually working with, not just something I threw together to support my argument.
This is from my personal motorcycle app that I use to display navigation, time, temp, and weather forecast while riding. I currently have a Weather model file that provides basic weather properties such as current temp. During initialization of this object, it needs to setup a timer to periodically check for any changes in the weather, as well as register to receive location notifications. This particular object was written long ago, before I became a 100% code coverage fanatic. I didn’t know how to test the init method back then. But now that I do, I’m going to refactor this code both to make Uncle Bob happy, as well as to add full unit test coverage for it.
So here’s the original init method:
This code looks pretty typical for an init method, and there isn’t anything functionally wrong with it. Ignoring the NSLog statement, and standard call to super and return of self, it does 3 things:
- Set the default current location. Location is needed for getting local weather, and since I’m on a motorcycle, it will change periodically. I initialize it with a constant that is the zipcode where my home is located.
- Setup a timer to fire off every 5 minutes to update weather.
- Register to receive location change notifications from a Location object I also created.
I’ve used comments to identify what is happening on 2 of these things, the 3rd being so obvious I didn’t bother. However, I’ve learned over the past few years that comments should be considered smells. I believe that a better approach is to extract the commented code to a separate method, and name the method using the text from the comment. Let’s see how that also helps make unit testing easier.
Let’s extract the first thing to its own helper method: setLocationToDefault. We can use the refactor command as shown here:
Following that, we’ll do the same thing for the other 2 things that init is doing. This results in the following refactored code:
Now our code reads like prose, and the extracted helper methods do exactly what you would expect. These are 2 attributes of good code.
Create a class extension file to contain the declarations of the helper methods. This file can then be #imported in the unit test case file, but kept private to normal class users:
Prior to making these changes, our unit tests for init would require verifying that:
- The current location (lastLocation property) is set to our default value.
- The timer was set to callback every 5 minutes
- A location notification is set.
The first item is very easy to test. But the inclusion of the other 2 things in init could complicate and make the first test unreliable and/or outright fail. What if a side effect of the other 2 causes the current location to change? There is a potential race condition here. Even if it didn’t break at first, it might in the future as changes are made to the code. This code is fragile.
In this particular case, it probably isn’t a problem. But it shows the type of problem that unit testing can encounter whenever a method does multiple things. In Clean Code, Uncle Bob admonishes us to have each method do exactly 1 thing (the Curly principle?). That may sound a bit extreme, but look how it has simplified things here, and made things safer to extend or change later.
After refactoring, the unit tests for init can focus on making sure that it calls the methods that do these things, and not worry about exactly how they are done, how long it takes, or whether they have conflicting effects. We’ll use a partial mock object to do this, as shown here:
In the code above, self.weather refers to the test weather object created in the setUp method. The partial mock created will intercept the expected calls, but pass anything else on to the test object. This code therefore tests only the code in the init method. The other unit tests that verify that the 3 things happen correctly would call the helper methods directly, thereby eliminating the possibility of any conflicting side effects.
4 thoughts on “Refactoring to Improve Testability”
> Create a class extension file to contain the declarations of the helper methods.
How do usually name such files? I’m currently name it like `FFFreeLine-Private.h` for FFFreeLine class. Would be interested in suggestions. Thanks.
“ClassName-Private” seems appropriate. Anything that conveys the intent is ok. I think it is important to be consistent, whatever naming convention you use. I’d suggest the use of words like “private” as you have used, or also “internal”, or “ForTesting”. I like that you’ve mixed camel case with a dash. This clearly separates the name from the additional qualifier.
thank you, Ron! =)