Guide to an Effective Code Review

Sometimes, a code review can leave both the author and the reviewer with a great deal of frustration. But it doesn’t have to be that way.

In this blog, I’ll share some of the things that I’ve learned over the years reviewing code as a developer. Although I’ll touch on some things that affect the author of the code, I’m writing mainly from a code reviewer’s perspective.

But before I begin, let me tell you a story…
As a developer at one of my previous teams, I regularly performed code reviews on my coworkers’ pull requests. When I was asked to mentor a team of interns, I had one intern in particular who, whenever I made any comments on their pull requests, just couldn’t take constructive criticism … or so I thought.

It turns out that I wasn’t very good at giving constructive criticism. I thought all I had to do was point out the issues in a cold, to-the-point way. That works fine for some people, but not for others.

Something had to be done, and it got me thinking about how to perform an effective code review.

Critique, don’t criticize

All too often I hear developers say, “I don’t want to be the one who criticizes another person’s code.” The word “criticize” comes from the same root word as “critical” and “critique.” To critique something is not a bad thing. So the next time you do a code review, try not to think of it as criticizing; think of it as an honest and helpful critique of someone’s work.

cri·tique /kriˈtēk/

an analysis or assessment of something, typically art, literature, or music

Be kind

Choose your words wisely. Use suggestive, collaborative and encouraging words instead of demanding something.

For example, instead of commenting:

“Change this to use a temporary variable.”

You might want to say instead:

“This might be more readable if we changed this to use a temporary variable like `let`.”

See how this changed the tone of the discussion? It brings the author into the conversation. Who knows, the simple gesture of asking may even spark a better alternative solution.

It’s everyone’s code

When you work on a team, the responsibility of gating dirty code from getting into the code repository is shared by everyone. Some of the most valuable advice I give new developers is to treat the source code not as your code, or their code, but as "our" code.

At some point down the road, you may find yourself working on code that another developer is checking in today. It’s not only your right, but your obligation, to make sure that the code is of the highest quality.

Let your tools be the "bad" guy

The last thing that you want to do as a code reviewer is to comment that the author forgot a semi-colon, or a space before a parenthesis. It is a waste of your time. You will most likely have more helpful, substantive, and encouraging things to say in your review. Leave the rest to automated services.

The modern software development world has many tools to automate what used to be a manual process of reviewing code. There are a number of commercial and free services (like TravisCI and CircleCI) that can perform unit tests before a human ever needs to get involved.

If your organization has rules about code style, let an automated linter do the complaining (ie ESLint for JS projects). Most of these services are free for personal or open source use. Use them!

Don’t be afraid to be wrong

If you see something in the code that doesn’t look right, but you’re not quite sure, go ahead and ask. It’s OK to be wrong. Just leave a comment and ask.

“Are you sure that this shouldn’t return a PENDING status instead?”

It causes the author to recheck their work. They are, after all, the subject matter expert for the PR. You may get a reply like this:

“Yes, I’m sure. LOADED is the correct return value here.”

And that’s fine. In fact, you just learned something. But who knows – you may get a reply such as:

“You’re right. This should return PENDING. Good catch!”

Congratulations. In this scenario, you just caught a possible production bug!

Never just rubber-stamp it!

Here’s a scenario for you. You’ve been asked to do a code review for a highly respected senior developer on the team. You go to this person all the time for help and advice. “Their code can’t possibly be wrong,” you say to yourself. “I should just approve it.”

Here’s another scenario. A fellow intern or co-worker just did you a favor last week. Now they are under pressure to get their latest feature out the door, so they come to you and say: “This pull request is pretty minor. I refactored a few things and added a few files. No big deal. Just approve it for me, will ya?”

The answer to both situations above is: No! Neither the skill level of the author nor the urgency of the situation matter. You should approach each code review with the same level of commitment.

Give helpful suggestions

If you are suggesting a change, please don’t just describe your issues, or say something like, “This is all wrong.” Try to give practical code samples or links to supporting documentation where applicable.

Teamwork makes the code work

Too often, developers fail to remember that coding is a partnership. You should give the author every opportunity to succeed. Don’t make them guess what you are thinking. You should always treat the developer with as much respect as you do the code, and vice versa. Writing code in an organization is a team sport.

Remember… There is no “I” in “CODE.”

==== Follow me on Social Media(@mrinasugosh) ====
Dev.to: @mrinasugosh
Github: @mrinasugosh
Twitter: @mrinasugosh
LinkedIn: @mrinasugosh