October 27, 2012
Refactoring Legacy Code #1 - Making Classes Unit-TestableA question that often comes up is "how can we get unit tests around our legacy code?"
The problem is usually one of dependencies that make it impossible to separate your code from external software, such as database servers and web/app servers, so your code won't work unless those external systems are there.
Here's a toy - but typical - example, inspired by some refactoring I worked with a client on recently:
We want to write unit tests for our CustomerServices class, but the calls to the static data access methods on DataRepository - which, let's presume, involve a visit to a database of some kind - mean that we can't test the code unless that database is there. We could write tests for the code as it is, but those tests will run much slower and we may have to run database scripts and wotnot to set up test data.
To write unit tests around CustomerServices that will run entirely in this application's process and won't involve external databases, we need to refactor the code so that we can substitute some kind of test double - e.g., a stub - where we're currently invoking static methods.
While we're about it, can you see why I tend to favour instance methods by default?
We can achieve this in several steps. First, let's turn these static methods into instance methods. (They're stateless, so it's pretty straightforward. A find-and-replace in both the CustomerServices module and the DataRepository will do the trick. Replace "static " with "", and in CustomerServices, replace "DataRepository" with "new DataRepository()".)
Next, because my end goal is to stub DataRepository in my tests, I want to abstract it for those purposes. So I'm going to extract an interface.
Next, I want to inject instances of IDataRepository into CustomerServices, rather than having them created internally. I could introduce them as parameters to both methods, but that seems like a kind of duplication. Better, I think, that CustomerServices should be born with its collaborators, so I'm going to inject it in through the constructor.
I'll do this is two steps. First, introduce a field of type IDataRepository that's initialised in the constructor. Then introduce it as a constructor parameter. A doddle in Resharper.
This is what we call a dependency inversion. Our high-level module CustomerServices depended on low-level details. Now it depends only on an abstraction, IDataRepository.
So I can write unit tests that pass in a stub that implements IDataRepository that allows me to inject test data menaingfully, and the web service that uses CustomerServices can pass in a real DataRepository that will connect it to the database.
Once I've got some tests around what Michael feathers call an "inflection point" in his excellent book Working Effectively With Legacy Code, it's safer to do more fine-grained refactorings on the internal design.
Was it safe to do the refactorings I did to get those tests in place in the first place? This is where the chicken meets its own egg, so to speak.
Our code couldn;t unit tested because we needed to refactor it to make that possible. But it's not safe to refactor without testing throughout to make sure we haven't borked the software.
What I would do at this delicate stage is opt for a number of potential choices to assure myself my code was still working.
I could, for example, have written those unit tests first, and just taken the hit that it would involve trips to a database until I could remove that dependency.
Or I could have chosen a higher-level inflection point and written some automated tests at that level. Often, this means system tests (e.g., GUI), or unit tests that call remote web services.
Or, if you don't have the skills or the tools necessary to automate system tests, the last resort might be - gasp! - manual testing. Yes, sometime we just have to run the app and click some buttons.
My advice is, if you are going to do manual testing, you need to be extra disciplined about it. Write proper scripts, choose real and meaningful test data, and be vigilant as to the results, down to the smallest detail.
Yes, it's a pain in the behind. But that's why we wanted to get some unit tests in there, right?
In the next post, I'll touch on a common problem in web apps - dependencies on HTTP session and application objects.
Posted 6 years, 1 month ago on October 27, 2012