I’ve been working on a project at work for the past few months that started with a basic idea of what was required of it, and then after a series of proof-of-concept demos we now have a better idea of what this thing needs to do, and more importantly how it needs to do it. This type of organic development process is great, but it can have some side effects for the code base. Primarily there are some code paths that made it into the code base to support some functionality that is no longer required, as well as some implementations allowing for requirements that have either gone away or changed, leaving some strange looking implementation artifacts. Secondarily we did not have time to write tests and setup CI and all those other best practice and stability things. I spent most of my week addressing these two things.

Code clean up

We did some refactoring a couple weeks back to get some of the more egregious code smells taken care of, where we had gone partially down a path for some functionality and then abandoned that bit of functionality, yet the code still had some artifacts of it left over.

After (and during) that, it became apparent that the next thing we needed to do (for our sanity), was tests. It’s really hard to tell when a new feature you’ve added is really working the way you think it is. Verifying a bug-fix didn’t just make a new bug in the process is also really hard when you’ve no tests to prove it.

In looking at writing some tests for the project we quickly found that some restructuring was going to be required. And that is what I’ve been doing with most of my week; Refactoring a bunch of functioning code into testable units.

Testable Units

The first thing I did was diagram all of the components as they were currently implemented and map out the communication points between them. That basically gave me each component’s current API. Sure there are easier ways to do this, but visualizing it was very helpful for me. It allowed me to move things around and really get the big picture (pun!) in my head all at once.

Then I drew up a new diagram which moved some of the responsibilities around from one component to another. This involved collapsing a couple classes into other classes where the complexity added for communication between the classes did not outweigh the benefit from separating them. There were also cases where I made a new class to separate out the more complicated pieces where the benefits of separation did outweigh the complexity imposed by communication.

I did a few iterations on this diagram and boiled it down to what I thought was the core set of components needed with very specific inputs and outputs defined for their interfaces and then went over it with the rest of my team. The new design made sure that each component had very specific inputs and outputs that could be generated by hand, as opposed to the previous design which took a lot of the data required to do its work either from the operating system directly or from internal properties which were the output of other processes.

Therein lies the key to making the thing testable: specific inputs and outputs and compartmentalized actions. The most basic requirements to make something ‘Testable’.

Conclusion

This all may sound obvious, at least it does to me upon review, but when dealing with things like location as do most things I work on at work, crafting these things based on inputs and outputs of your own code as opposed to events from the device/system becomes less straight forward. This week’s refactor was a bit of revelation in this regard for me, and I wanted to write about it. So here we are, this is me, telling you that separation of concerns and clearly defined, testable APIs with discrete inputs and outputs are a good thing. You probably already knew that, but sometimes it’s worth saying again.

The other thing of note here is that the mere process of making this code testable, led to restructuring it in a way that made it far more loosely coupled, which is an improvement in stability even without having written a single test.

Posted by Ryan at