Unit tests versus code quality

Do unit tests improve code quality? Some famous consultants might disagree, but I think they don’t. Testable code isn’t automatically better code. Depending on the capabilities of your language, it’s probably worse.

Now don’t get me wrong, unit testing is a good thing. But I think we need to realise that we’re often making a trade-off between simplicity and testability. To me, simplicity is the most important factor of code quality, but many people lean towards testability and are very successful with that. Maybe a complex, well tested system is better than a simpler system with less test coverage, I can’t answer that. But you can’t have both, at least not in static languages.

Let me explain what I mean: In a unit test, you’re testing a small part of your system in isolation. You’re ensuring that a single module, class or function, works as expected, now and in the future. You only test the unit as it can be used from the outside, and that’s good, because its implementation details shouldn’t matter to the rest of the system. But it’s also a problem.

It’s a problem because you often have to actually change the unit to make it testable. There are plenty of examples for this, but I’ll stick to one in this post: You’re testing a unit that uses a random number generator. Since the behaviour of the unit will be different every time you use it, you need a way to take control of that random number generator in order to test it reliably.

If your language supports object-oriented programming, the common approach is to introduce an interface for the thing you need to control and inject it from the outside. Let’s say we’re creating a RandomNumberGenerator interface and pass an instance of it to our unit. You can then create a fake implementation that does just what you want, and pass that one to the unit from the tests. Now you can make sure that your unit works fine for various random numbers.

However, we have just added to the system’s complexity. We have created a facade for a random number generator, which is very likely already available in your language’s standard library. Anyone working on your code base will now have to know that your facade has to be used instead of the standard method. We have also introduced an interface that doesn’t make much sense right now: Ignoring the tests, there is only one random number generator – why have an interface if there is only one implementation of it? That’s nothing but unnecessary code other poor souls will have to wrap their head around. Maybe you even introduced a dependency injection framework – this is going to make your code base a lot more complex.

Languages that support monkey patching (most dynamic languages, e.g. JavaScript) are an entirely different matter: You can simply rebind the dependencies in your tests. I think that’s how testing is supposed to be: We should just write simple and clean code and be able to test it, without having to think about how to test it and what trade-offs to make. But static languages are still around, popular, and the only option for many applications, so I guess we will need to make such trade-offs for quite some time. Let’s at least be honest about it: It sucks.

10 Responses

  1. Interesting article but not sure I agree. From my perspective unit testing is probably the best thing that ever happened to programming, especially when you work with the types of developers with mixed abilities I work with.

    Here are my counter arguments:

    1. We probably should be wrapping static APIs in Adaptor classes anyway to ensure good decoupling. For example, you should always create an adaptor class for your system clock and inject that into your code, instead of directly accessing the system clock API call. The number of times I’ve seen bugs revolve around time, the ability to inject your own times into code to test this is invaluable. If a piece of code doesn’t do this, inevitably I have to add it.

    2. most people use a DI framework these days, anyway. IMHO DI frameworks dramatically reduce the code complexity of the systems I work with.

    I’m also not normally swayed by code complexity arguments. What they boil down to is the YAGNI principal which is fine, except it doesn’t hold if you are actually gonna need it.

  2. I think in the case of the RandomNumberGenerator, it would actually suffice to implement it as a static method, and then allow a static method to setNextRandomNumber, similar to how Joda-Time does it with setCurrentMillisFixed to allow tests to set the current time.

    You say testable code is “probably worse”, but my personal experience is that, more often than not, a testable implementation is synonymous to a simple and elegant solution. There will of course be cases where the opposite is true, and a trade-off is required, but I’m guessing that’s more the exception than the norm.

    I think this conflict of thought arises from when a skilled programmer arrives at a design not using TDD, he/she finds it some times reduces quality to make it testable, whereas a TDD-believer will automatically arrive at a different, but this time testable design, and see it as the best solution. Which design is the best one I think will be a matter of taste, and not something you can generalize.

  3. @Ricardo:
    I’m not saying that having unit tests is bad, I think it’s great. I may not have brought it across well, but my central point is that you do have to make your code base (minus tests) somewhat worse in order to write unit tests. Making your code testable does not magically improve its quality – quite the contrary IMO.

    I think your experience suggests that a well-tested code base is worth it, even at the expense of simplicity. I don’t know if that’s something that can be said in general though, I think it depends on what you’re working on, and with whom.

    You’re not going to say that having a non-standard static method to generate random numbers and some global variable to modify it’s behaviour is a simple and elegant solution? It’s a hack. Better than mine though, I’ll give you that 🙂

    I agree to your point about TDD, a believer will probably think that’s nice and simple code he’s writing driven by tests. From my limited experience with TDD, I think it’s because they _want_ to believe that. If you look at the code a year later, it’s not that great anymore. That random number example was from some code I wrote using TDD. Looking at that code now makes me wonder whether I was sober back then.

  4. Felix,

    Thanks for the post!

    I think that the dependency injection (DI) pattern resolves the issue of “changing the unit to make it testable.” I disagree that a DI framework makes your code more complex. In Java, the Spring Framework allows you to have less code complexity via DI, by removing the need to instantiate any objects, resolving the issue you describe above, whereby additional interfaces are created just to test a respective class.

    You can design your class hierarchy to implement whatever functionality needed, then “inject” the needed values in your test package, without needing to create a separate implementation/interface for production and test.

    The behavior changing every time you use a respective class, as in your random number generator example, would be resolved by dependency injection, as the data/injection would be resolved at runtime, rather than compile time. You can certainly use dependency injection to define aspects of your classes at compile time, i.e. database connections in test being different than development and production, etc, but the dependency injection design pattern allows for component choice/data to be made at runtime.

    I know you were referring to C++ in your article, but Java has dependency injection built into its language as of version 6.

  5. I think I agree with Thomas, its about how you define “better” code. For me, better code is testable. In the case above it adds complexity, but in general, for me, it reduces the complexity of the large systems I work with.

    That said, unit testing does encourage more cohesive code.

    In the case above, for a trivial application it may add what appears to be an unnecessary level of complexity to create an adapter class for the underlying Random API. But if you use random functions elsewhere in your code, and you suddenly find that there is a security problem with the random function you’re using, you now how to go through each line of code and replace it. With an adapter you simply create a new implementation.

    This seems better to me.

  6. @Ricardo:
    Hm, that made me think. In many cases, unit tests force you to expose implementation details. For instance, what if you want to test an an algorithm implemented as a private function? You need to somehow make it public, e.g. by putting it in a new class. Until now, I thought that was always undesirable, because you wouldn’t normally want to make that function public if you’re using it just once. But I’m not so sure anymore. It’s more cohesive, but there’s still a new component in the system that’s probably never going to be used in more than one place.

    I don’t agree with your second paragraph though. If we follow all the way through with your argument, that means we should implement a layer of indirection on top of our language’s whole standard library, because there could be some bugs in it. Doesn’t sound like a good idea to me.

  7. @Rick:
    How is DI supported by Java 6? Well I guess it’s always been supported since you can just use constructor parameters or setters. But there’s no IoC container built-in?

    The problem I have with DI frameworks (basically IoC containers if I understand that correctly) is that it’s some pretty invasive framework your whole code base depends on. I’ve worked on a project where the DI framework broke our neck countless times, although I never had any such problems with Spring. Still – it’s more third-party code you have to understand in order to really understand your system.

  8. I don’t know about other languages, but in Java you can make a class testable if it uses the default class permission so it isn’t exposed outside of the implementing package.

    I disagree that what I said means you have to create wrappers for all the APIs in your platform: in Java, certainly, most APIs are class-based don’t need to be wrapped. Also, I don’t think you need to mock everything, such as low-level objects like File or List objects.

    However, when you’re using some resource-based, static utility methods I think you probably should create a wrapping adapter class. This is also useful for cases like random number generators or system clocks because you might need to adapt/configure their access (e.g. set seeds, make seasonal changes to clocks) across your whole application. How do you do this kind of thing without creating an adapter class/interface?

  9. I’m not sure about the invasive argument, java has the framework-agnostic @Inject annotation now so your classes don’t have to depend on framework-specific annotations.

    If you use XML or code-only object creation, your objects never need to be aware of Spring. Not sure how DI works for other platforms.

  10. @Ricardo:
    Touché I guess 🙂

    You’ve probably got a point that “good code” is somewhat subjective. To me, an instantiable class without state is a smell. (You only need to instantiate a class if it has state, otherwise it could be a utility class in Java, or just a namespace in C++.) Then again, I’ve spent my first years as a programmer doing mostly C, so I guess that defined my idea of good code.

Leave a Reply