Tests uncover Design Smells

TLDR; Tests, even written after the code, help tremendously in pointing to production code that could

use an improved design. If writing a tests hurts, then it is often the designers fault, not the tests.

Tests, wether written before/during (TDD) or after pushes us to choose a design that will make both tests and code relatively painless. That easiness is of course bounded by the design options we already know of. Many times in life my tests have hurt, without me knowing what to do about it. But then weeks or months later I discover a new way of designing my code that would have made it better. My default assumption whenever I can’t find a way to make my tests painless, is that I have something to learn about software design. I believe this is a state of mind that is very useful if you want to have better code on your project.

Here’s a stripped down typical example of a piece of untested code. I came across this recently and I’ll show how the tests point me to a weakness in the design, and one way to deal with it. First let's have a look at the tests to know what the code does.

$ mocha test/LangBuilder.spec.js 

  LangBuilder.buildLang()
    lang.id
      ✓ is the language found in the pdf file
      ✓ can be explicitly overridden
      ✓ is french by default
    lang.face
      ✓ is a collection of pages in the pdf file
      every face
        ✓ has the width and height of the first page
        ✓ has a unique id

But its not the whole story, sure it returns a lang, but it also submits another piece of data to a message queue. Admittedly that is a violation of the Single Responsibility Principle, but I’m going to concentrate on a more subtle design smell. So here are the interesting parts of the code, in particularly we'll be interested in the _sendOffSplitJob function.

BTWYou can find a link to the full piece of code at the bottom of this post under "References"

So what could the tests for the sendOff part look like? It’d be something of an ugly stub/mock-hell that is one of the main reasons many people shy away from unit testing. Just for the record I show one of the test, but please don’t spend energy on understanding it. It's just proof of uglyness in case you didn't trust me.

Why is this test difficult to write? Well we deal with a third party library that wasn’t designed for easy mocking and certainly not designed to optimise one specific use case of our application (that’s not saying the library is ill-designed), so from the standpoint of LangBuilder we can simplify the design of the message queue library.

Imagine we had a function: submitJob(splitJobData) that took care of the queue naming retries and eventual send-off. A side-note on this example; javascript has first-class functions so I’m using function-composition by passing a function to the constructor of LangBuilder instead of passing an object as in object-composition. Here's that function

And the refactored LangBuilder

Then we could simply assert that LangBuilder calls it with the correct arguments, like so: 

In the process LangBuilder became easier to use, it doesn't have to bother with configuration related to the library. 

Looking at the submitJob function it became easier to use too, we didn’t just move the complexity of the queue management to a different class, we simplified it's desing with respect to our needs. In it’s original design it was mixing non changing parameters like the queue name and the number of attempts with the changing parameter splitJobData. Now we can pass those parameters at boot time creating a function that takes only the constantly changing parameter. And that simple function can be passed to any function or object that needs it.

Then we can test the submitJob function in integration with a redis server. I always put this kind of test in a different suite for practical reasons, more on that in a follow-up post. The basic idea of such a test is isolate the smallest sensible amount of code into a first-class object or function. Then test that unit extensively with the real thing.

Conclusion

Writing that test was a pain. Acknowledging it as a possible design smell allowed us to improve design as LangBuilder now respects a bit more the Single Responsibility Principle. Both the tests of LangBuilder and jobQueue are easy to write. jobQueue is now a first-class function that is easy to reuse elsewhere in the application.

Listening to tests allows us to spot where the tests and the production code could use an improved design. Of course tests help us spot problems and protect us during refactoring, but they don't find good alternative design. That's up to us developers to find and chose.

Reference :
The full code of LangBuilder before refactoring