Gears Within Gears

Seek simplicity, and distrust it.

Code comments, alias_method_chain, extensions, and the importance of expressing intent

Posted by Brian Guthrie Fri, 27 Mar 2009 17:14:00 GMT

At ThoughtWorks, everyone submits a code sample as part of their interview process. The comment I received from my interviewer about mine struck me, though, and it sticks with me to this day. “The code looks fine,” he said, “but what’s with all the comments?”

My introductory CS course at Northeastern University taught me a lot of things that have resonated in my experience in the industry, but we were taught one thing that, as it turns out, almost none of my colleagues agree with: that all of your methods, classes, and modules should be preceded by a simple, one-line comment that states the purpose of code unit in question. Far be it from me to gainsay Neal Ford, who believes that code comments are a smell, but there are days when I disagree: Even developers with the best intentions and training in the world can’t always write clear code.

A Brief Re-Introduction to Class Re-opening

The problem manifests itself most acutely through the Ruby mechanism of class re-opening (sometimes dismissively referred to as “monkey-patching”). You can go about it in a number of ways, and the codebase I’m working on these days has employed each and every one of them at some point or another. The advantage you gain from being able to reopen classes is that you can patch the framework you’re using unobtrusively in order to fix bugs or improve performance. The downside is that you don’t get to choose the names of the methods you’re overriding: they may make perfect sense in the context of the class you’re overriding, but none whatsoever as an isolated patch.

A reopened class looks something like this:

In the first example, we’ve patched String directly; in the second, class_eval’d it for an added layer of complexity; and finally we’ve defined our own module and added it to String after the fact, which is fact the recommended way of going about it. We can use IRB and the method method to try to figure out where our patch is coming from:

Notice that only the last one gives us a helping hand when we’re trying to figure out where this method was defined.

Overriding, not extending

Now imagine there’s a bug in the String class: a colossally poorly-designed method, something that’s significantly impacting your ability to get something done in the language. (While this doesn’t happen in Ruby very often, it’s like to have occurred at least once in the framework of your choice.) In the bad old days, you’d have had to patch Ruby directly; if you were good about it, you’d submit your patch to Matz, and it’d get integrated as part of some future release. But the language itself gives you an escape valve: override the method, and your implementation gets used.

On my current project, we do this all the time.

But what happens when the framework gets upgraded? Is this extension still necessary? Why did anyone put it there in the first place? What’s with this Subversion check-in comment? Why didn’t anyone say anything in the code?

Enter alias_method_chain

alias_method_chain is a Rails method patched into Ruby that allows you to extend a method without blowing away the old version. You can read Marcel’s post on it on the Rails blog from back in the dawn of time, two years ago, when they added it to the framework. In action, it looks something like this:

Suddenly our extended method has an identity again: it has a name that clearly indicates both the method it’s trying to extend and the feature we’re adding on top of it. Hallelujah!

Why does this matter?

The ability to re-open classes is an insanely powerful tool. I love it, not least because it never means having to write another XUtil (XmlUtil, DateUtil, StringUtil…) class again to make up for the deficiencies of someone else’s API. I could never tell you, in good conscience, not to use it.

But cleaning up becomes a bit of a mess when you have to upgrade frameworks (or language versions, for that matter) as we’ve had to do recently on our project. We carry a copy of Rails around in our vendor directory, like most projects do, but we’ve tried to be good about making our extensions externally, through class re-opening, rather than patching the Rails code directly as men and women did in days of yore. But because, out of habit, none of these extensions carry any comments, we’ve had a devil of a time trying to figure out not what they do (we can always read the code) but why they’re there, and whether we can finally, for the love of God, delete them now that we’re upgrading to Rails 2.2.

But take it from me, one developer to another, who hath had to maintain thine extension, yea, even unto multiple new Rails versions: using the tips above will help me, and those like me, figure out what the dickens you were trying to do. And for Heaven’s sake, when you don’t have the luxury of naming your methods to make it clear what they do or why they’ve changed, leave a quick comment.

Posted in | no comments |

Slides and video from last month's Handshake presentation

Posted by Brian Guthrie Mon, 11 Jun 2007 22:40:00 GMT

I should have posted these a long time ago, but better late than never: the slides from the presentation I gave at last month’s Boston RubyGroup on Handshake are available here in PDF format, and I’ve embedded the video below. I hate seeing myself on camera, so I haven’t watched it yet. Stay put for the big finish, where my laptop’s battery runs out of juice.

I haven’t had much chance to work on Handshake recently, no excuse really, but with any luck I’ll get a chance to push out another release of that and RubySearch before Sunday. Then I’m on to Chicago for a two-day initiation at ThoughtWorks, my new employer, followed by a six-week training session in India. I can’t wait.

Posted in | 1 comment |

Career announcement: I'm joining ThoughtWorks

Posted by Brian Guthrie Mon, 07 May 2007 03:51:00 GMT

Update: I’m not sure if I made this clear below, so just for the record: ThoughtWorks is a cool company with a great reputation and I’m thrilled to be joining them. All of my comments below should be taken in the spirit of that statement.

I’m tickled pink to announce that I’ve accepted a job with ThoughtWorks as some kind of Ruby guy and so I’m moving from Boston to Chicago to live the dream of 80-100% travel to places that are not Chicago. I thought it might be worth explaining briefly why I chose them.

In short: I get to work in Ruby (apparently), on a diverse range of projects, in diverse settings, at considerably greater job security than I might find at a startup; I expect to learn a tremendous amount about developing good software; it’s a change of scenery and a chance to travel; everyone at the interview impressed me, and were kind enough to let me geek out over Handshake, yea, even unto the whiteboard; and significantly, I couldn’t find anything credibly negative about the company, either from former employees or outside observers. If they suffer from a little hubris, well, there are far worse sins to have.

I’d also like to note that everyone I met at the interview event, in particular their founder Roy, was remarkably and even alarmingly candid. It was a breath of fresh air.

The job starts on June 18th, and shortly thereafter I’ll be attending ThoughtWorks University for six weeks. I’ve always wanted to see India. I also have to relocate to Chicago between now and then; I don’t know anything about the city, and if anyone has any advice about living and working there I’d love to hear it.

Posted in | 2 comments |