This past weekend, I refactored a little bit of jQuery for a WordPress plugin I’d written, open-sourced, and released a few years ago. It wasn’t a particularly good bit of jQuery to start, but it worked. In refactoring it, I made sure my script told a clear English-language story about what it was doing. And I loved the results, so I thought I’d share.

I’ve touched on these topics before, too. I wrote two essays — “Programming is About People” and “Programming is Storytelling” — that were driving at this idea. But those were, understandably, critiqued for being a bit theoretical rather than concrete and actionable. So today, we’ll run through a very simple case study to hopefully flesh out the ideas those essays present that you can and should strive to make your programs tell comprehensible stories to future human readers.

The Code Before It was Refactored

I thought about explaining the problem before I showed the code, but since most of what I’m hoping to show is that the refactoring made this code much simpler for a new person to see and comprehend, I’ve chosen to present the old one without comment. If you get what it does without a second’s thought, you’re way ahead of where I was when I picked up the old script, and we may disagree on the value of this whole effort of refactoring.

Enough preamble, here’s the code:

jQuery(document).ready(function($) {

  if ($('body').find("#nofeature-message").length===0) {
    $('h2').after('<div id="nofeature-message"></div>');
  }

  setInterval(detectWarnFeaturedImage, 5000);
  detectWarnFeaturedImage();

  function detectWarnFeaturedImage() {
    if ($.find('#postimagediv').length !== 0) {
      if ($('#postimagediv').find('img').length===0 ) {
        $('#nofeature-message').addClass("error")
            .html('<p>'+objectL10n.jsWarningHtml+'</p>');
        $('#publish').attr('disabled','disabled');
      } else {
        $('#nofeature-message').remove();
        $('#publish').removeAttr('disabled');
      }
    }
  }
});

It’s not long, just 12 lines of any substance. And it’s nothing too fancy. No possibly obscure playing with jQuery prototypes of anything. But I’m not really able to quick understand what it does. Are you?

There’s obviously something important in the div with the id of postimagediv. And then something’s going on with a different div this script seems to create called '#nofeature-message, and something else going on with an element with an id of “publish”.

A Quick Explanation of the Problem

The WordPress plugin for which I did this refactoring assures that a user has attached an image to their post. Essentially it does this on the server, and on the client side, but for a combination of laziness and sanity those two checks are completely distinct.

The client-side Javascript check relies on the fact that WordPress almost always puts this attached image in a specific place with an expected name in the markup. Then the script just does a simplistic, “if there’s an image in there” check and lets the user know that they’ve done their duty by removing a warning message.

The Refactored Code Tells the Story

To reify that summary, here’s the meat of the refactored code (this lives inside the same jQuery document ready block as the other):

function detectWarnFeaturedImage() {
    if (postTypeSupportsFeaturedImage()) {
        if (lacksFeaturedImage()) {
            disablePublishAndWarn();
        } else {
            clearWarningAndEnablePublish();
        }
    }
}

detectWarnFeaturedImage();
setInterval(detectWarnFeaturedImage, 5000);

Well that’s pretty nice. Not perfect, but nice. (Here’s a link to the whole final code.) We have a function that’s executed once on page-load and then again every 5 seconds (I admit that setInterval isn’t great from a comprehensibility perspective, but it’s close).

And what does that function do? Well it’s supposed to detectWarnFeaturedImage — a function that probably could be clearer if I wasn’t a little worried about type-ability. And that function only acts if the postTypeSupportsFeaturedImage and if it does, it checks if it lacksFeaturedImage. If it does, it’ll disablePublishAndWarn, and if not it’ll clear the warning and allow publishing. (There’s an argument to be had about the nested ifs and use of an else clause there, but we’ll leave it lay for this piece.)

As you might have guessed, these were rather trivial refactorings. All the things that the old code did inline but never explained are now just wrapped in functions that explain what they do.

So What? Who Cares?

There’s a tradeoff at play here. There was ten minutes of work needed to take the code that did what I wanted and make it do it in named functions. And arguably, there’s a performance hit that goes with that. And let’s not forget that we’ve nearly doubled the number of lines in this very very simple script. Is it worth it?

That’s really an impossible question. As I hope that my essay “Why Bad Software Succeeds” makes clear, we’re really always talking about tradeoffs. “Good” and “bad” when it comes to software aren’t inherently meaningful concepts. A longer, (trivially) slower script is the cost, but the value is that there’s almost no need to understand the heart of the jQuery selectors in use or what they’re doing to understand what this tiny little script actually does. A non-programmer could likely pick up the script and, reading the part I showed above, understand precisely what it’s meant to do, and modify it in the way they’d like.

One could reasonably argue that the time-worn call for “more comments” is another solution to the fact that the first version of the code was hard to comprehend at first glance. But here again it’s mostly a philosophical question. Comments have advantages — they wouldn’t require all those extra functions — and disadvantages — “deceptive comments”, which trick you with their clear explanation of how the code used to work, and not how it does.

For me, I know this is better. More than 100% better. Well worth some trivial added line-length and the supposed inefficiency of these previously inlined function calls. (I’m not going to waste my time actually benchmarking, but I’d guess most modern Javascript JIT compliers will take care of this inlining and make the performance difference trivial.) What matters is that this code makes sense to me as a human reading it, and that it clearly tells its story in a place I can see. The machines are built (and are now plenty fast) to render the rest trivial.

Image Credits: ldorfman, CC0