How to Review Code

We recently shipped a bug that broke our credit card form.

When you’re a SaaS company like Tanda, being unable to record credit card details is not a good thing. If there’s one thing you want working 24/7 it is the ability to have people give you money.

This bug should have been caught in code review, but it wasn’t. So in the spirit of profiting from our mistakes, we tightened things up a bit. Here’s two key lessons we learnt.

Review deletions as closely as you review additions

Bad deletions can be just as catastrophic as bad additions. This bug was caused by the removal of a single line of code. It stuck out like a sore thumb going back over it because there were no other modifications in the file, and the file had nothing to do with the changes at hand. But nobody was looking for suspect deletions during code review so nobody saw it.

I personally like to review code in the two column layout, with deletions on the left and additions on the right.

I found that my eyes tended to follow the code in a “U” shape, going top to bottom through the additions on the right then skimming back up through the deletions on the left. That is how you get bugs.

I now review code in what more closely resembles a “Z” shape, reading deletions and additions at the same pace, comparing them, questioning them. “There’s a deletion on this line without a nearby addition. Why?”

It’s important to spend as much time thinking about what a removed line of code did as it is thinking about what a new line of code does. Innocent-looking deletions can take your site down.

You, the code author, are the first code reviewer

The mindset that you write the code and someone else makes sure it doesn’t have any bugs is exactly how you get bugs.

Is that what you want?

At the end of the day, your code has your name on it. You can’t blame bugs on your code reviewer. Unless you’re new to the codebase, you should understand the implications of your code at least as well as your reviewer. In other words you are well-placed to review your own code, so do it.

By “review” I do not mean “yeah nah I reviewed it as I wrote it she’s all good mate,” I mean looking at the diff. Do exactly what your code reviewer would be doing. Code review is a separate act from writing code that requires a different mindset. If you’re truly reviewing your own code then you should occasionally spot things.

Code reviewers are not an insurance policy, they are assistants in your quest to not ship bugs. Don’t forget about the deletions.