Stop all indentation, TDD from religion to pragmatism

Decoupling responsibilities is the most important part of your design, I have seen so many snippets of code that do not delegate responsibilities and create code that is not ready to be scaled.  A good system is one that you should feel the pain of your wrongdoings as soon as possible. When you find yourself testing completely different things in your tests - it is time to delegate responsibility and mock it. You’ll also find here my current conclusions regarding the role of TDD in your development toolbox.

Once upon of time

During the days when I was still working at Intel, I was walking towards my cubicle when a colleague stopped me and asked If I have some TestHelper class to test String equality, something that disregards whitespace and newlines. We were both working together on a framework for marshalling and unmarshalling configuration objects, and I knew he is currently developing ConfigurationUnmarshal class, of course we both did it with TDD, there isn't a task more suitable for TDD then this one. ConfigurationUnmarshal is a class that was responsible for transforming in-memoryJava objects to human readable configuration. The output configuration was quite similar to JSON (I wish we knew about JSON back then) and his code went over introspecting the java class and the particular instance data, then writing the human configurable format into a file. (Oh the happy days of java and exploiting every possible use of annotations). It was a weird question because 20 minutes before, I finished deleting my StringTestHelper class from our repository, it did exactly what he was looking for, so I answered “you don’t need it, you need a better design” (yes, I can be annoying that way :) )  

“What has been will be again”

A couple of years later, which are a couple of month ago we, at rollout.io, started extracting  data from the debug symbols (also known as the .dsym file) into a JSON file. We were referencing LLVM open source implementation (thanks!!!)  for dwarfdump and my colleague generated something similar to this:

static void dumpParameter(raw_ostream & os, uint indent, DWARFCompileUnit *compileUnit, const DWARFDebugInfoEntryMinimal *parameter, bool returnValue, const char *currentDirectory)
{
    //some logic code
    os.indent(indent) << "{n";
    uint innerIndent = indent + 4; //was some constant but I changed it for the blog
    if(typeStruct.size){
       os.indent(innerIndent) << format(""size":"%d",n", typeStruct.size);
    }
    os.indent(innerIndent) << format(""type":"%s",n", (const char *)typeStruct.type);
    os.indent(innerIndent) << format(""origin":"%s",n", (const char *)typeStruct.origin);
    os.indent(innerIndent)<< format(""kind":"%s",n", kindString);
    os.indent(innerIndent)<< format(""file":"%s",n", typeStruct.file[0] ? (const char *)typeStruct.file : NULL);
    //some more code
    dumpParameter(os, innerIndent, compileUnit, DWARFDebugInfoEntryMinimal *parameter… ) // recursive call to for dumping Objects
    os.indent(indent) << “}n”;
}

I commented out all the actual code that this class/function is really responsible for, that is how to extract significant data from DWARFCompileUnit. But you can easily notice that function also gets the raw_ostream object and an indentation counter and responsible for formatting of the JSON. If you would have tried to unit test this, you would soon find out that you need to test two things:

• Do we read the data correctly and write it into a JSON

  • Traversing over all the objects from dsym

  • extracting the right the data from dsym data structure

  • Creating the right recursive structure for structs/enums/union/Pointers/primitives/etc…

• Do we write the JSON correctly

  • start/close a json new object with { | }

  • surround strings with quotes

  • Start/close arrays with [ | ]

  • Indent the output

  • Add new lines when needed

  • And, off-course, the problem that developers dealt with from the beginning of time, only add comma ( ,) when necessary (not on the first object in the array)

Now, it is time to remind ourselves one of the most important principal of a good design, the “Single Responsibility Principle”. Uncle Bob describes responsibility as “one reason to change”  - a class, a function or a module should have only one reason to change. If there is a bug in the way we extract the data then one module should change and if there is a bug in how we format the output a different module should change. This function will clearly need to be changed if we choose to print the indentation with tabs, or if we decide to skip the entire whitespace characters and just print JSON as one long line.

"All problems in computer science can be solved by another level of indirection"

I want to show you how the code looks today:

void dumpParameter(JsonWriter *jsonWriter,  DWARFCompileUnit *compileUnit, const DWARFDebugInfoEntryMinimal *parameter, bool returnValue, const char *currentDirectory)
{
    //some logic code
    jsonWriter->startNameLessObject();
    if(typeStruct.size){
     jsonWriter->addKeyValue("size" , typeStruct.size);
    }
    jsonWriter
       ->addKeyValue("type" , typeStruct.type)
       ->addKeyValue("origin", typeStruct.origin)
       ->addKeyValue("kind", kindString)
       ->addKeyValue("file",  typeStruct.file[0] ? (const char *)typeStruct.file : NULL)

    //some more code
    dumpParameter(jsonWriter , compileUnit, DWARFDebugInfoEntryMinimal *parameter… ) // recursive call to for dumping Objects
    jsonWriter->closeObject();
}

Here, we introduced a new object JsonWriter which is responsible for the actual writing of a JSON, it is so simple I didn’t even write it with TDD (don’t worry,  it is covered by acceptance tests). The JsonWriter class is responsible for how we write the JSON, it has very few functions in it’s interface, lets have a look:

JsonWriter* startNameLessObject();
JsonWriter* startObjectWithName(const char* name);
JsonWriter* endObject();
JsonWriter* addKeyValue(const char* key, const char* value);
JsonWriter* startArrayWithName(const char * name);
JsonWriter* endArray();

TDD - listen to your tests

What’s interesting here, is that both colleagues (at CloudBees Feature Management.io and Intel) did the same thing, as well as the amazing developers of LLVM. My colleague from Intel was the only one who really suffered from his bad design decision. When he tried to test his functions, he started noticing that every change in the format requires to rewrite  all his expected values. So he wanted to write a StringTestHelper class that will disregard the format of his result. Some of you might have heard the famous talk “is TDD dead” by  Kent Beck, David Heinemeier Hansson and Martin Fowler, in my opinion, Kent and Martin (who are definitely included in my hero list) didn’t really address the issue that David talked about which is “test induce damage”. Test induce damage is real and it is painful. But here we are seeing that test driven development was able to put to surfaces real design issues, when your tests are fragile, when you are testing too much in one class, it is probably the time to check if your are SOLID. Is the new design better? An interesting thing happened with our JSON writer class, when we started optimizing our process we quickly discovered we need to optimize the structure that was created by the JSON writer, there was a lot of duplication in data because of the way compilation units are generated at compile time. What did we do? We replace the implementation of JsonWriter to a JsonWriterToNsDictionary with the same interface JsonWriter was using, the new class creates a data structure instead of printing it to the output. That class was a bit more complex, this one I wrote with TDD, it is beautiful… TDD is live and kicking –  In your face @dhh  ;)

I am no longer a fanatic TDD’er -  I have evolved

I have been a fanatic TDD’er for a couple of years, In the first year I end up deleting most of my tests because they were too fragile, I tried to BDD and went back to TDD. I tried testing how classes interact with each other and felt like I was testing implementation. I used mocks and spies and did Katas… the state of code writing and TDD has changed so much in the last 7 years that I want to share my current filings about TDD A couple of thought about TDD:

• You can't force someone to TDD

• TDD implies continues design – which is hard!!! and not for newbies

• TDD is not suitable for every task – especially not UI If I see another screencast of people demonstrating how to TDD UI I will freak out!!! Here is why it never works:

  • The design of a UI solution depends on the presentation layer, in TDD design is usually determined by what test you choose to write and when. TDD for UI doesn't help you do a better design, it only helps you write testable code

  • Usually, the resulting test are fragile

  • It takes too long to write, you focus on testability and not design (and introduce test induce damage)

• Always start your system with a design in mind, just don’t expect this design to be the right one. Design decides the order of your test, you need to have some roadmap ahead of you.

But, when I think about why I love TDD is because:

• The knowledge of the system you are developing increases while you are developing it:

  • Continuous design is the solutions for people who can deal with it

  • Write your code from use cases and extract abstractions when appropriate (and only then) . Do not create frameworks first, you'll just create redundant levels of abstractions

• The feedback loop is addictive

  • It is best to get the red-green feedback every time (when suitable, choose TDD)

  • if you're not getting unit test into the system find another way to get feedbacks fast.  (UI is easy)

• Professional developers only ship tested code - TDD is not enough, you need to have automatic QA tests

• Don’t try to read Kent’s second chapter of Test-Driven-Development-By-Example “The xUnit Example” it is too complicated for humans ;)

So I ended up being a fanatic continuous designer, that is hooked on getting feedback from his code. It works best when I can TDD but there are many circumstances when it just isn't cost efficient to TDD.