Within last few months I was introducing tests into untested code. It was not old code, just some code that was not Unit Tested. As much as I was eager to do so, I found myself getting in a bit of a pickle.
Code was actually quite complex and difficult (smell) to understand. I was also a little scared that I will misunderstand the code and its intention. I assume that I will not be the only person facing this problem while working on the piece of code.
Fortunately there was some testing coverage in a form of a higher level Acceptance tests (not sufficient though).
I decided to refactor the code a little to make it more manageable. Extract few methods, rename few variables, remove unnecessary conditions and loops, etc.
When I finished refactoring and made sure build was still successful, I checked in the code and off I go to write some Unit Test. That was the point when I hit another brick wall.
Necessary dependencies in a class that I wanted to test were not provided to an object upon creation, but were instantiated internally. This means that code is not flexible to changes in a future and is impossible to test (smell). Making all the dependencies passed through constructor will make it more obvious to what the class will do, and make it possible to unit test.
So, after some refactoring again, building and submitting the code, I had my dependencies all wired up through constructor, just the way I like (mmmm, delicious).
Many of the dependencies were still concrete classes. Pity, actually you don’t care what’s in the guts of your collaborators (most of the time) as long as they do what they indicate. This could be replaced by interfaces (or some abstract types in worst case) as dependencies. They also make it easy and simple to stub or mock your collaborators, so the testing is simpler to setup and you’ll be testing only the correct object.
Another refactoring session ended in a bunch of new interfaces.
So I started to write a test. I created my object and started to pass some dependencies. The problem was that I had to pass some other dependencies to those dependencies, and dependencies of dependencies and even more dependencies and … you know where this is going.
This great monster had too much setup code than the one I was going to test.
Too many collaborators
This got me into thinking that something was not right. The thing was that the tested class was trying to do too much. This was the main reason of mentioned previously code complexity. Testing was not easy and possibility of introducing new bugs during future changes was great.
So, extracting few methods here and there (around same functional area) and I was able to pull those methods into new object. This new class was very simple and was responsible for one area of functionality only. I could unit test that new class.
Last problem during the tests was that I got some unexpected behaviour. As I looked into the code it turned out that there was one more smelly thing left.
There was some code that was calling to a static method of another class. It was impossible to mock or stub it. What’s even more horrible, it was using some internal static variables. That caused some un-expected behaviour.
As it turned out, I found problems in a code even before I started to write a single line of test. Just thinking about the ways to test, revealed first issues. More of the issues appeared while testing. The work I had to do in order to make code tidy, simple and testable could be avoided by writing it in TDD/BDD way.
I’m convinced there could be more problems revealed. Here are these that I found once again:
- complex code
- instantiated (new Foo()) dependencies inside tested class
- concrete classes with no interfaces
- too many collaborators (class was doing too much)
- static methods (brrrr)