The TextConverter exercise (in the Racing Car kata suite) is a very interesting exercise for refactoring legacy code. As I am reading Working Effectively with Legacy Code I found two ways of solving it.
Refactoring can only be done safely with unit tests and so I did, rigourously in TDD. Though, in this blog post I am not going to show you all the testing process, but only the two ways of decoupling the dependency.
The code comes with a classic dependency issue: a method that secretly (from the point of view of the client) reads from an external file that is created using the filename passed in the constructor. So, no way for the client to know about this file before or to avoid its usage. Here the original code:
As you can see, the filename “fullFilenameWithPath” is passed in the constructor and then it’s used internally by FileReader. This creates an annoying hidden dependency that prevents the testing of the method convertToHtml(). If you try to call the method convertToHtml() from your tests you will get a “File Not Found Exception”.
Let’s leave aside that ugly mix of levels of abstractions in this method (FileReader, BufferedReader, < br />, static call to escapeHtml() all together .. blah). I found that this will be fixed almost automagically once you figure out how to extract that dependency from there.
So, how to extract that dependency so that we can test it?
An injection is the passing of a dependency to a dependent object (a client) that would use it.
In this case the dependency is FileReader and the dependent object is our HtmlTextConverter class.
I want to pass the dependency from the constructor, but we can’t change the existing one. In Working Effectively with Legacy Code this is called Preserving Signature, but more easily we can’t change an API that is used by the clients.
So, I can refactor this class in 3 steps:
- I extract the new FileReader from the method so that now it is created by a ReaderFactory (I am using the Factory Design Pattern here, so that I can create a FileReader, or more in general a Reader, with a method createFactory())
- I create a new constructor that takes a ReaderFactory interface as input
- I make the old constructor to call the new one passing the implementation of ReaderFactory that does the same job of new FileReader().
The point 3 is called Parametrize Constructor in Working Effectively With Legacy Code and this is the result:
The fact that I extracted the FileReader from the method allowed me also to better define how I wanted to use it, so to get rid of BufferedReader, now handled in HtmlFormatter class. This allowed me also to remove the global variable fullFilenameWithPath from HtmlTextConverter. A simple refactoring that made me, at the same time, decouple the dependency and make the code cleaner by removing that ugly mix of levels of abstractions.
With this implementation I can now test the method just by creating a fake implementation of ReaderFactory in order to read from a String instead of a file:
In this technique you extract all the work involved in the creation of FileReader in the constructor into another factory method. Then, you create a testing subclass and override the method. This is the result:
I have extracted the new FileReader() into another method called getReader(), so now I can override it in a subclass called FakeHtmlTextConverter as follows:
This technique allows me to not change the constructor, but instead using a fake implementation in the tests, like so:
Working Effectively With Legacy Code is an illuminating book and i strongly recommend it to read. It’s pretty dense with content, but reading it along with small exercise like this one it’s a great way to absorb it faster and make it yours.
This exercise is interesting and puts you a bit uncomfortable since the refactoring is becomes pretty big very quickly and it’s difficult to do it in small steps. You stay in the red zone (test don’t pass) for long, at least the first time you do it and mostly if you’re not familiar with design patterns or any refactoring strategy in particular.
Regarding the techniques, the 2. is strongly linked to the programming language used. You can’t use it in C/C++. The 1. is more clean. Both are two great ones.