Getting Better All the Time via Code Review

I read a great blog called Ask a Manager, which recently posted a question that I think had more of a technical than business solution:

[New coworker’s] HTML and CSS work are terrible. This is his first “real” job out of college, so I understand that he has some catching up to do as far as honing his skills (as well as how he conducts himself in the office, but that’s another story entirely), but I don’t think that excuses his lack of front-end coding skill if that was part of his job description. I’m currently trying to jump in to help develop one of his projects, and I’ve wasted the past two hours trying to pick apart his code into something I can use. My other coworker has met the same frustrations when trying to use this person’s code.

If someone’s straight out of college, they don’t know how to code professionally. They might be an irredeemably terrible programmer, but they can probably be whipped into shape by code review.

But won’t code reviews take a long time? Well, honestly, yes. It slows you down. However, you’re making your code base better, saving future pain, and passing it forward by teaching the next generation. One of my coworkers came up with a good strategy to review code as fast as possible by making multiple passes:

Pass 1: Style

Make sure spacing is sensible, variable names are more than one letter, it looks like the code around it, and generally follows whatever style guide you’re using (Google has public style guides for most languages, just use one of those if you’re a small company).

Pass 2: Readability

Now that the code isn’t blinding you on contact, see if you can comprehend what the code is supposed to do. If it takes you more than a minute to understand a section, leave a comment that you don’t understand what it’s doing, it needs to be simplified or, if it really has to be complex, it needs a comment explaining what it’s doing.

Pass 3: Functionality

Now that you understand what the code is doing, make sure there are no obvious errors and that there are tests. For new employees/interns, I’ll actually run the tests and make sure they actually pass, since that seems to be a problem sometimes. Also in this pass, make sure the interface is documented and makes sense.

Pass 4: Sparkle

This stage can be project-specific, but it’s the general gotchas that you, as an experienced engineer, know to watch out for that they may not. Do they need to add hooks for monitoring? Are there error cases they’re missing? Should they add another layer of abstraction or use a different class hierarchy?

You can combine these passes, especially as the coworker starts producing better code.

Finally, if you’re reviewing their code but they just aren’t getting any better, you can resort to Joel Spolsky’s advice for slowing down a lousy programmer (see the “Neutralize the Bozos” section).