August 22, 2009

Solo refactoring

In my most recent post I described a recent refactoring exercise. Refactoring can be a dangerous exercise, if not done properly. Any time you alter code, there is a risk that a bug or performance issue can be introduced. Hence the strong attraction to the “if it ain’t broke, don’t fix it” rule of thumb.

But if done correctly, refactoring mercilessly is a very strong practice. According to Martin Fowler:
Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior. Its heart is a series of small behavior preserving transformations. Each transformation (called a 'refactoring') does little, but a sequence of transformations can produce a significant restructuring. Since each refactoring is small, it's less likely to go wrong. The system is also kept fully working after each small refactoring, reducing the chances that a system can get seriously broken during the restructuring.
My intent in refactoring the code was to make it more readable and understandable, without changing its external behavior. No adding features, no removing features, no bug fixes. And certainly no new bugs.
I think I was pretty successful with the code cleanup part, but I was not careful enough. Just a handful of hours after I posted, a reader named Ivan submitted a comment pointing out a case I had neglected to handle.

A case which was handled in the code I changed during my refactoring. I had broken the cardinal rule: I had introduced a bug.

Ivan, My New Pair


So why did this happen? It would be easy to say that I was just too careless. I worked alone, and didn’t ask for my code to be reviewed. Some would say I shouldn’t have refactored otherwise working code, anyway. I think these things just happen; bugs happen, even to the best of us.

But there are things we can do to minimize the chance of these bugs seeing the light of day. One of the things our group has been doing more of in the last year or two is pair programming. Having a second programmer next to you, keeping you on track by thinking strategically while you’re in tactical mode, helps to keep little errors from sneaking in. On the day I did this refactoring, my programmer pair was unavailable. Rather than try to implement a new feature solo, I thought it would be safer to pay down some technical debt with some small refactorings.

Anyway, long story short, the small oversight I made would probably have been caught nearly immediately if I had been pair programming at the time. In this case, Ivan from the Ukraine, across time and space, was my virtual pair for a few minutes. He noticed one small thing, made one small comment, and put me back on track. Thanks Ivan.

Test, Test, Test


Another way to severely reduce the risk of making breaking changes is with a good set of unit tests. If you have intention revealing code, and those intentions are explicitly asserted by your tests, it’s very hard to unknowingly break things by refactoring. That big red “test failed” indicator stares you right in the face.
Our group, as are most (I suspect), is still maturing in our unit testing methodology. I personally am still not very comfortable with Test Driven Development, but I try to keep all of my code as much under test as possible. In this particular case, there was no test wrapped around the particular situation I had missed and Ivan had pointed out. There is now, though. The next person (pair) who decides to refactor this particular code will have the benefit of a test explicitly reminding them of this case, and each of the other rules the class must follow. The tests are the executable specification.

He can be taught


So what I have learned? I learned that I need to be more careful when changing other people’s code. :-) I learned that pair programming is valuable even for simple code, and even across 10,000 kilometers. I learned that good tests help not only when first writing the code under test, but perhaps even more when someone else comes along to refactor it. I learned to triple-check any code I post on my blog. And I learned there are some very nice people out there willing to spend a few moments to help a stranger.

To be honest, I thought I knew all of these things before, but I guess it takes a little reminder now and then to refresh one’s memory. That’s probably the most important lesson of the day.