The Code Review Process

A small list of rules to apply to enhance the code review process
By Radu Badiu Posted 19 May 2016

The Code Review Process

A bad day

You, the “victim”, after “painfully” writing an awesome piece of software engineering code, submit your work, bravely, to the soul crunching machine that is the code review. People who work with you, people you never heard of, people you once called friends tell you that you are wrong, that the code you wrote needs to be deleted until it “lays eggs” or worse, proper indentation was not used at line 42 and you misspelled some variable name. You then start to get angry, you start to ask yourself “How could they not see the awesomeness before them?”, your work of art needs to go unchanged in the central repository, “Are they blind?”. Even worse, the deadline is tomorrow. Your know where your, soon to be ex, friends live. But you can’t “take care” of them all until tomorrow. So you start changing “your baby”, “the most awesome piece of code ever written” into something ugly, mainstream, that everyone can understand and read. I know, this is one bad day.

Bug

Working in the software development industry for some years now, one of the most beneficial process used in all companies I worked at was the code review. For those unlucky enough to work at companies where code review was a mythical creature, only heard of but never seen, a code review is essentially a discussion between the author of some piece of code and other developers.

There are numerous articles out there on the benefits of code reviews, including knowledge sharing, code quality, developer growth and so on so we will talk about something else: the way we carry the aforementioned discussion.

Reviewers’ rulebook

While doing a code review I have found that following some simple “rules” made the discussion more productive.

  • Be thoughtful when doing a code review: all comments should be positive and oriented on improving the code. Emphasis on local standards, program specs, performance issues etc. are welcomed.

  • Prefer asking questions to making statements: a statement is more often than not accusatory. “You din not follow the SRP principle here” is an attack - even if unintentional. A question here would be better in order to drive the discussion forward and to not put the author on the defensive. For example: “What was the reasoning behind the approach used here?” asks for more information that may clarify the approach used.

  • Avoid the “Why” questions: building on my previous point “why” questions should be avoided as that will substantially improve the mood of the author. Most such questions can be reworded to dramatic effect. For example: “Why didn’t you follow the standards here…” versus “What was the reasoning behind the deviation from the standards here…”

  • Praise the author: while most of the code review focuses on finding faults in the author’s code when quality code is found a good job comment is all but necessary. Since writing code is a creative work many developers find the code they write close to their heart and emphasizing this bits of code can do wonders for the author’s state of mind.

  • Propose a solution for a bad piece of code: no one wants to find that after writing a “jewel of software engineering” a reviewer comes and says “This is wrong. Fix it!”. Even worse, that piece of code that is “wrong” and needs to be fixed is not trivial. This can lead to frustration. To alleviate this, always provide possible solutions to bad code.

  • More than one way to skin a cat: as the saying goes you should always have in mind that there are many ways to approach a problem. If you would have written the code differently that does not make the current implementation bad. The goal is quality and maintainability. If the code meets those goals that is all you can ask for.

  • Don’t rush it: in order to provide a useful code review please take your time. The code review should have the same priority if not higher than that of writing code. A good code review is one that points out flaws not one that is done quickly and most often than not the code is “OK”.

  • Be prompt: that being said you should also be prompt. Studies have shown that after one hour the effectiveness of code reviews drops as we lose focus. So as a general guideline one hour code review sessions are prefered.

The author mindset

Now that we covered how a review should be carried we must talk about the mindset of the author. After years of code reviews both given and received I can say for sure that sometimes you, as the author, are susceptible to feeling like a victim.

Code Quality

As the author, the developer needs to keep an open mind during code reviews. It is easy to get defensive when reading the reviewers’ comments since they are talking about your code, a very personal matter.

We need to understand that if a question is asked it is because the code is not all that clear, or if a suggestion is made it is to improve the overall quality of the code. In short, we are here to improve the code base not to criticise the code of others so there is no need for people to get upset. Unfortunately this is easier said than done.

Radu Badiu
Radu Badiu
Senior Software Developer