This Code Is Untestable! (Part 2, for Developers)

In my last post, I argued that “untestable code” is code that cannot be efficiently unit tested. Today I’d like to walk through my top five anti-patterns that make writing unit tests painful. If you aren’t in the habit of writing unit tests, hopefully these tips will help you build more testable code and see how test-driven development can help drive clean design.

1. Lack of Dependency Injection

Let’s start by considering a worst-case single line method:

class CrisisHandler
  def code_one
    MissileControl.new.launch_nukes
  end
end

What’s so bad about this code? Well, let’s assume that this is a pretty important feature to get right, and that your code had better work as expected. We definitely want to be able to validate this method.

However, we run into two problems. First, we can’t actually call this method without starting WW III. Its MissileControl dependency is not injected — it’s new’ing up the object directly. This means we have a hard coupling between the CrisisHandler and the MissileControl, and no way for our test to ask the CrisisHandler to exercise its logic but with different, less catastrophic side-effects.

The second problem is that we have no explicit return value to validate — we have no way of knowing whether #code_one has taken any action at all. We could try some hacks, say… test for the well-being of known targets, but this implies that we know a lot about the #launch_nukes algorithm and that we’d be able to validate the state of the objects it might modify.

We can fix both of these problems with dependency injection:

class CrisisHandler
  def initialize(missileControl)
    @missileControl = missileControl
  end
  def code_one
    @missleControl.launch_nukes
  end
end

Now our test can inject a mock MissileControl that will not destroy the world when called, and we can place assertions on our mock to ensure that the #launch_nukes method is actually invoked as expected.

We gain some other nice benefits — one can imagine a real MissileControl object would be expensive to create and initialize. It may pull in all sorts of other code and have many other unpleasant side-effects (notify the president?). Perhaps the MissileControl implementation is not yet complete? Or its implementation has bugs, and we don’t want the CrisisHandler tests to fail because of defects in other areas of the codebase? We can avoid all these problems and keep our unit tests running quickly by using a nice, cheap mock.

Perhaps you are thinking that this is all fine and good, but you don’t want to create the objects ahead of time? Or you aren’t sure how many objects you will need at runtime? Inject a factory then — the production factory can create real objects as needed at runtime, and your test factory can create mocks.

A nice additional benefit we get from using dependency injection is that we’ve now broken the tight coupling between the objects involved. Assuming that the CrisisHandler’s requirements are well-defined, any object implementing that interface can now be injected to handle missile launching duties.

2. Poorly-Defined Responsibilities

The Single Responsibility Principle is perhaps the golden rule of coding — following it carefully will help you avoid so many mistakes. However, I’m sure you’ve seen it violated. Perhaps you have this class in your current application:?

class EverythingController

  # ... 540 lines of code ..

  def do_all_the_things
    # ... 100s of lines of code ...
  end

  # ... 800 more lines of code ...

end

Ok, so it’s a bloated, poorly-organized class, and that’s unfortunate, but what makes it untestable? Well, first of all, it’s extremely important to know that your test suite is thorough, that you are covering all your bases. When a class is cluttered with various responsibilities, especially when it stores state for each, it becomes extremely difficult to read through the class, read through the test suite, and know that you have all the important cases tested. Instead of having a focused “team” of tests that validate a class, the suite becomes more of a “crowd” — it’s hard to know what tests cover which functionality, where you have duplication, and where you have gaps.

A second concern is that as classes grow to cover more and more functionality, they tend to also gather more and more dependencies. Instantiating a class with many roles becomes painful; the setup time to create all the dependencies for each instance is expensive both when writing the tests and at runtime.

Your unit tests will be much easier to read and write when your classes have single, clear responsibilities, so writing “testable” code encourages you to follow good principles.

3. Collections Treated as Individuals

We all like to feel important and be recognized for our unique qualities. Your objects, however, won’t suffer as a result of being treated anonymously. What I am getting at is that I frequently run into code that, when boiled down, looks something like:

  def barnyard_noises
    @cow.moo
    @rooster.crow
    @pig.snort
  end

What’s the problem here? Well, at the moment we have three mocks that we need to inject and validate. Give our project a few more iterations, and maybe that list has grown to five or six. The class is taking more and more dependencies, it’s becoming more painful to instantiate in our tests and more painful to validate. What we really want is:

  def barnyard_noises
    @animals.each { |a| a.vocalize }
  end

Now we can easily create and validate this method with a couple mock objects regardless of how many animals might end up in our barnyard. The benefits to the software as a whole should be obvious here as well. We’ve now removed the need for this class to know specifics about each of its dependencies — it no longer needs to change as new animals are added, we have less code, and the algorithm is easily understood.

4. Hidden State

A long time ago, on a project far, far away, somebody wrote some really nasty code using the internal state of other objects. This tight coupling led to great suffering and sadness, and developers round the world decided that this could never be allowed to happen again. Any method that exposed state was to be avoided whenever possible.

While I agree whole-heartedly that objects should know as little as possible about each others’ implementation, I think the pendulum has swung too far. I frequently encounter code where API is provided to manipulate an object’s state, but then there is no parallel API to query whether that state has changed to the expected value. In nicer languages, this may not be a problem — mocks can once again come to our rescue to validate that we have invoked APIs correctly. But if you’re using a less friendly language (cough, C++), you may not be able to create mocks for third-party objects. As a general rule, I would suggest that if you’re providing an API to allow clients to modify your object’s state, then you should also provide an API allowing them to query that state.

5. Missed Opportunities for Pure Functions

Consider the following awesome explosion animator:

class Explosions
  def animate_explosion(type)
    if type == :small
      @animator.explosion_radius(1)
    elif type == :medium
      @animator.explosion_radius(5)
    else
      @animator.explosion_radius(10)
    end
  end
end

Does this code work? Well, yeah, and it’s testable, I suppose. But it’s more painful to test than it needs to be. What if we wrote this code instead:?

class ExplosionInfo
  def radius(type)
    return 1 if type == :small
    return 5 if type == :medium
    10
  end
end
class Explosions
  def animate_explosion(type)
    @animator.explosion_radius(@explosion_info.radius(type))
  end
end

Now we can write much simpler tests. The logic is gone from #animate_explosion, and the #radius method is a pure function. This code is more easily tested, and also I would argue much easier to read — the complexity of the #animate_explosion method is greatly reduced. Also, now that our code is extracted to a #radius method, we can easily see that this logic might be more clearly expressed with a data structure mapping types to radii.

Conclusion

If you’re new to unit testing, look for these anti-patterns, and your test writing should go much more smoothly. Pay attention and you’ll notice that the changes you make to enable unit testing also clean and simplify your code — not a bad deal.

If you’re an old hand, what are your most hated anti-patterns, and which ones should have made this list?