Sunday 26 January 2014

Drive By Refactoring


Today's post is a code post, specifically about homebrew programming but actually about programming mindsets in general. I've done some code review recently - professional, personal and random internet peoples code where I just look at what they are doing and see how it differs from what I've seen before.

I'm going to talk about the style of code reviews I've been enjoying recently - drive-by-refactors. In a formal review or refactor process, its sometimes difficult to know what to focus on and its easy to go into too much or too little detail. The Drive-By-Refactor is a way of identifying which changes you need to make and which  can wait.

As an example, I was quickly browsing the API of a package, and about to use it when I noticed an unusual declaration...
public: virtual void MyMethod( int parameter ) {}
Spotting a method in an interface with a default implementation led to a drive-by refactor.  Why a default implementation? Does the concrete class implement it? This rather poor practice has probably pushed a compile-time error to a runtime error.
Of course I changed it to a pure virtual / abstract declaration and boom! compile error. Could not instantiate abstract class.  The game is afoot!
I followed the trail of breadcrumbs and found the problem.  It used to take a filename and now takes a file handle, but the interface had both methods with the obsolete one providing a default implementation.

The drive-by refactor is fun because they are generally small, self-motivated and quality driven. You get to flex one of the too-often underused refactoring tools - the gut feeling. Often referred to as code smells, your gut reaction to a bad fragment of code can count for a lot and you have the chance to eye-spy a problem before it gets any worse.

Drive-By refactors can be great for those moments where you spot a problem and want to investigate, but sometimes you don't need to take any action beyond noting that you've found a problem - in this case just leave a tagged refactor comment and carry on driving would have been enough.
There are times when you don't need to take immediate action and it's acceptable to describe the problem in a comment and move on.
A drive-by review is a quick overview of code where you don't get to see the detail, but do spot the sore-thumbs. You are only going to spot things that are obviously wrong, and you'll find your tolerance for wrong shifts the more you review.

This brings us on to the second thing that a drive-by spots. It spots review comments where you-in-the-past or somebody else has written "Law of Demeter Violation" or "method doesn't use its parameter" or "method operates on side-effects only" or "this class has too many constructors"
You might see one of those and really agree with it, and decide now is the time to fix it - or you might decide its no sufficient to take action.

And that's the take home lesson for today - when you spot a problem you should make a note of it. Make a note in the code, because that's the only place that it'll be read. Make it useful to the future programmer, be brief, descriptive, concise. To start you only need to point out problems, and you can stop there. A typical drive-by code review for me includes spotting problems or responding to previous review comments as much as it is leaving review comments or making code changes.

Typically I'll invest time in the refactor if the functionality is on the critical path to my current sprint feature set, while if its incidental then I'll leave a review comment.  One day I'll be working in that code anyway and see the previous review suggestion and it might be a good time to deal with it.

By learning to do Drive-By reviews, you practice quick decision making, and its this practice that winds its way into your day-to-day programming. Instead of writing something you know to be second rate, you might find yourself adding a review comment explaining what's wrong with it.
Before long you find yourself writing the review comment, and stopping to fix the problem. It starts to become automatic and you can do it at the smallest level.

Review fast, review often. Read a lot of code. Read what you wrote today, read what you wrote yesterday and learn from it. See which shortcuts you took, and unlearn your bad habits. Study history or find yourself repeating it. Drive-By yesterdays code, free your mind and make different mistakes tomorrow.

I hope you've learned something from the Drive-By refactor and are able to add it to your toolbox. Review code every time you read it. Drive by a lot of code, because the more you see then more you learn, but stop when you need to.

No comments:

Post a Comment