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 doesnt have to be that way.

In this blog, Ill share some of the things that Ive learned over the years reviewing code as a developer. Although Ill touch on some things that affect the author of the code, Im writing mainly from a code reviewers 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 couldnt take constructive criticism or so I thought.

It turns out that I wasnt 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, dont criticize

All too often I hear developers say, I dont want to be the one who criticizes another persons 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 someones work.

critique /kritk/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.

Its everyones 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. Its 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!

Dont be afraid to be wrong

If you see something in the code that doesnt look right, but youre not quite sure, go ahead and ask. Its OK to be wrong. Just leave a comment and ask.

Are you sure that this shouldnt 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, Im sure. LOADED is the correct return value here.

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

Youre right. This should return PENDING. Good catch!

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

Never just rubber-stamp it!

Heres a scenario for you. Youve 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 cant possibly be wrong, you say to yourself. I should just approve it.

Heres 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 dont 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. Dont 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