silk and spinach

Eighteen months ago I posted a Ruby version of Bill Wake’s Amazing maze refactoring challenge. I looked at it again this week and discovered that it is rubbish: the transliteration from Java is not very literal, and one of the test examples is broken. So I’ve re-created it and uploaded a revised version. My apologies if you tried working from the original version.

In the fresh light of day, I have a slightly different way of expressing the ideas in yesterday’s rant…

When we’re designing and developing a system (and hopefully we do those two things at the same time), most of us have checklists of some kind in place to ensure that we consider the system’s various users (end users, managers, administrators, support persons, etc). What we often forget, though, is to consider another important group - programmers. We take great pains over user experience, always thinking that means only those folks who’ll use the system for it’s intended purpose. But unless we consider the user experience of the developers, the other users may never see the features they really want, because the code is too impenetrable to work with.

The (current) canon of dogmatic Agile (with a capital ‘A’) and test-driven development demands that we RefactorMercilessly at every turn, but that seems somewhat simplistic to me. When I refactor, I do indeed strive to refactor mercilessly; but sometimes I don’t feel the need to refactor at all…

Given a suite of acceptance tests for a software system, there is clearly a (countably) infinite set of possible program sources that will pass those tests. (If you don’t see that, consider that every variable in the program has an infinite set of possible names; the same is also true of function parameter ordering, curly brace positioning, sorting algorithm, method breakdown, class naming, whitespace…) And as the system grows during development and maintenance, we navigate this infinite space of programs. Every time we write, change or remove a line of code, our system moves to a new point in that space; every time we add or change an acceptance test, we define a new (countably infinite) space of possible programs, and our system instantly sits at one point in that space.

And so every time we add a new feature, or fix a defect, we have an infinite number of possible points to which we could move our system within the program space and still pass all the acceptance tests. Furthermore, these “neighbouring” points are not all equivalent: the points in the program space have attributes, or qualities. Each program possesses different amounts of readability, maintainability, resistance to certain directions of change, coupling, cohesion, and so on. Which means that each time we write a line of code we make choices about the qualities of the resulting system. And all too often we forget to make those choices consciously.

Now those qualities I mentioned all have one thing in common: they directly influence both the speed and cost of change of the system. That is, each point in the program space, given a fixed set of acceptance tests, will support some kinds of change cheaply and other kinds more expensively. So as we develop and maintain we should consciously select programs which align with our system’s intended lifecycle; in particular, a write-once-and-forget-about-it system will need different cost-of-change qualities than a system that is intended to survive and be supported for more than a few months.

Refactoring is the means by which we move around the program space. As soon as we have a passing test, we can cast around for a nearby program whose qualities are a good-to-great match for our business needs. How much time we spend doing this is an investment trade-off against those future needs, and should be judged accordingly. Sometimes any old thing is fine; other times it is worth chasing down every last ounce of duplication.

The point of all this is that every day the programmers make choices which affect the system’s qualities. We should do so consciously, and according to the needs of our business and of our users.

Here’s something I’ve seen many times in code right across the spectrum, and which I caught myself doing just the other day. It seems to occur particularly often in codebases that weren’t TDD’d, and which haven’t been kept lean with regular refactoring…

Imagine a codebase in which one particular text string occurs frequently. The string may be some kind of message header, for example, or an XML tag (both of these examples are usually accompanied by another repeated string, representing the message footer or the closing tag). As the codebase grows, you notice the string popping up more and more frequently, until the “duplication” bell goes off. What to do?

It turns out we’ve all been taught the answer: Create a manifest constant (a #define or a static final String or whatever, depending on the language), thereby giving the common string a name, and replace all instances of the string by use of the constant. There’s even a name for this refactoring in Martin Fowler’s catalogue: Replace Magic Number With Symbolic Constant. Duplication sorted, right? Wrong!
Read the rest of this entry »

Getting Serious about Preserving Behavior is a great little article by Mike feathers, in which he introduces a refactoring trick he calls sensing variables. The gist is this:

You want to refactor some code, but it’s too ugly to test as it currently stands. So there’s a danger that the refactoring will somehow inadvertently change behaviour. Mike suggestes introducing a new variable outside of the scope of the code under test, and then altering the code to report state into that variable. Now, write tests that check the values in the variable, under a variety of conditions. Those tests will tell us whether our subsequent refactorings have changed the variable’s value - in other words, whether we have accidentally changed some deep behaviour somewhere. Very nice.

(The technique is also covered in Mike’s book Working Effectively with Legacy Code - see Chapter 22 “I Need to Change a Monster Method and I Can’t Write Tests for It”)

Last week I posted about using tests as a failsafe. Silly me, I should have known just how many people would comment out the failing tests! So what makes it easier to say “oh, let’s just comment them out so we can get on with our real work”, instead of fixing the tests? I’ve seen it happen many times, and I’ve done it myself - but why?

I think time is the driving force: if we try to fix the tests we’ll miss our deadline. Why? In every case I’ve seen, those failing tests were also refactoring no-go areas. The test code is in such a mess that either it would take an age to fix, or there is no-one left on the project who understands it. So it becomes much easier (and therefore quicker) to comment them out than it would be to fix them.

Why? How did the tests get into that state? In general, it seems to me that there are two main reasons: either the team didn’t know they had to keep the tests adaptable and maintainable; or the “unit” tests actually exercise chunks that are way too big.

Why? What causes the tests to be too chunky? I think in most cases the tests were written after the production code, when its too late to make the design decisions that lead to de-coupling and testability. In fact, whenever I see big chunky unit tests, or tests that are not malleable, I view them as a signpost to smelly production code: too much coupling, or leaked responsibilities, or inappropriate abstractions.

So it pays to work hard on the “ilities” of the test code. Keep the tests simple to setup; keep them independent from each other; remove duplication from them; and have each test check a very few things. (Oh, and don’t over-use mocks … but that’s for another day.) Refactor them just as you would refactor production code, and just as frequently. Because then they are less likely to break unexpectedly in response to application changes that are apparently unrelated. And then maybe you’ll be able to use them as a quality control…

In Declutter your Code Rachel Davies makes a nice argument for de-commissioning the term ‘refactoring’. (And I suppose me calling the codebase gemba is no better…)

In assertions on domain objects Alan Francis posts in some puzzlement on the coding practices of Chris Stevenson: In order to reduce duplication in his JUnit tests, Chris apparently now has JUnit assertions in methods on his domain objects.

Unless I’m very much mistaken, this means that those domain classes have a dependency on JUnit, which in turn means that JUnit must be on the classpath wherever his application is deployed. Now while that may be only a slight headache for deployments such as in-house or web-based projects, it would be a complete no-no for a shrink-wrapped or off-the-shelf product of any kind.

Looking at the examples on Alan’s blog, I do like the look and feel of Chris’s resulting code. But that dependency makes it hard to swallow, so I guess I’d vote for Alan’s own style as being a reasonable compromise.

On reflection, is this maybe a problem of the language? In Ruby, for example, the test suite can add shouldXXX() methods to class Object, and these methods won’t appear in deployed production code. The rSpec framework does this, for example. Yet another reason to switch to Ruby, if you haven’t already done so…

I wanted to try William Wake’s refactoring challenge (look for the More Resources section at bottom-right). But I wanted to get away from Java for a while, so I’ve translated the challenge into Ruby. Download it here and have a go. I’ll post my refactored version, with a commentary, when I get the time to work on it.

Update:
This version is a really bad translation from Java to Ruby. I’ve done another that is much closer to the spirit of the original, and so acts as a much better starting point for the challenge. Jump here to download it.

In My Favourite Smells Chris Wheeler tells an all-too-familiar story of code in which the use of primitives (such as ints or strings) obscures the true underlying design. Then in Primitive Obsession James Shore echoes the same thought:

“Primitive Obsession is one of my favorite smells as well: it’s easy to spot, easy to fix, and yields some really great designs when thoroughly stamped on.”

Well I just thought I’d add my “me too!” to the list…

I actually view this smell as a subclass of feature envy, in the sense that a primitive obsession often becomes apparent when a section of code spends too much time manipulating a primitive. When I said recently that my approach to refactoring legacy code is based on removing feature envy, I subconsciously included fixing primitive obsession too.

I wonder how much of what I write here is based on mental models that I haven’t surfaced yet? Probably most of it…