What Are Code Reviews and How to Do Them Effectively

Umer Umer Follow Apr 03, 2016 · 11 mins read

What is Code Review?

Code review is the process or rather an activity in which code written by a developer is inspected by other developers to look for defects and improvements. In other words, developers work on their code and ask for one of their peers to review their changes before they are merged into the main codebase.

In the last few years, code reviews have become part of normal workflow for large and small teams, ensuring that every change gets looked by at by least one other person. They are an integral part of almost every large company like Microsoft, Google, and Amazon to name a few, where every line of code is reviewed and approved by developers before it is merged into the main codebase.

What To Look For in Code Reviews

It varies from team to team, but generally, a code reviewer should consider the following:

  • Does the code meet the requirements that it’s addressing? Does it do what the developer intended it to do?
  • Does the overall design makes sense and fit with the rest of the architecture?
  • Are there general defects like race conditions (for concurrent code), edge cases, and other bugs that users might encounter?
  • Is the code readable and maintainable? Are future developers likely to struggle to understand what’s going on? Is it more complex than it needs to be?
  • Does the code have appropriate Test coverage? (Unit, Integration, or End to End tests)
  • Does the code adhere to your coding standards or style? This includes things like naming, comments, etc. Note: Automate style-checking as much as possible.

Why do Code Reviews?

Code reviews take time: the change is held up until it is reviewed by usually 2 people. So why do it?

When done correctly, code review is a proven technique that helps improve code quality and spread core knowledge across the team. It forces developers to hold themselves to a higher standard because they know that their code will be reviewed by their peers. It’s also a great tool for mentoring new team members on nuances of the code base. Or as Martin Fowler puts it:

Code reviews help spread knowledge through a development team. Reviews help more experienced developers pass knowledge to less experienced people. They help more people understand more aspects of a large software system. They are also very important in writing clear code. My code may look clear to me, but not to my team. That’s inevitable–it’s very hard for people to put themselves in the shoes of someone unfamiliar with the things they are working on.

So even though code reviews introduce extra time, it’s an excellent trade-off considering all the benefits that we get out of them. (You could even argue that they save time that’d be spent in future on bug fixes, direct knowledge sharing or paying technical debt as a result of unmaintainable code.) According to Code Complete 2, code reviews are very effective at detecting bugs and cites several case studies:

  • IBM’s 500,000 line Orbit project used 11 levels of inspections. It was delivered early and had only about 1 percent of the errors that would normally be expected.
  • A study of an organization at AT&T with more than 200 people reported a 14 percent increase in productivity and a 90 percent decrease in defects after the organization introduced reviews.
  • Jet Propulsion Laboratories estimates that it saves about $25,000 per inspection by finding and fixing defects at an early stage.

However, many teams struggle with effective code reviews. In fact, when done incorrectly, code reviews can be quite painful. In dysfunctional teams and organizations, it can quickly turn into a rather nasty experience for everyone involved:

  • Code reviewers show off their skills - or sometime even get back at the author - by demanding that their pointless opinions be implemented, that would make absolutely no difference on the outcome.
  • Code reviews take a long time to complete introducing delays to feature releases and become annoying for the author to keep resolving merge conflicts.
  • Authors ignore review comments and get their ally to approve.
  • It creates friction between developers.

How to do Code Reviews

Let’s look at some techniques that the authors of the pull request, reviewers and company management can use to do code reviews effectively.

Advice to Management

Managers should make sure that everyone understands the goals and importance of code reviews. Unless code reviews are part of your culture, developers are not going to ask their peers to review their code. Make sure developers get enough time in their Sprints for code reviews.

Managers can also help by setting up the right tools and adapting the release workflow to make code reviews a mandatory activity. Source control management systems like GitHub and Gitlab have built-in support for code reviewers that allows comments on specific lines of code, blocking merge until required number of approvals have been granted, etc. You can also use 3rd party tools like Crucible from Atlassian.

Make sure developers have enough bandwidth in their Sprints to review code. Otherwise, code reviews can end up taking a long time to complete.

Everyone: Remember the Human

In his book, Peer Reviews in Software: A Practical Guide, Wiegers writes:

The dynamics between the work product’s author and its reviewers are critical. The author must trust and respect the reviewers enough to be receptive to their comments. Similarly, the reviewers must show respect for the author’s talent and hard work. Reviewers should thoughtfully select the words they use to raise an issue, focusing on what they observed about the product. Saying, “I didn’t see where these variables were initialized” is likely to elicit a constructive response, whereas “You didn’t initialize these variables” might get the author’s hackles up.

It is easy to become fixated on the code, but remember, there’s a human at the other end of the table (or computer). A human who has opinions. A human who is entitled to have an ego. Remember that there are many ways to solve a problem.

  • Be humble. I have seen both highly productive reviews and very unproductive ones because someone decided to be a prick – don’t be a prick:-)
  • Make sure you have coding standards in place. Coding standards are a shared set of guidelines in an organization with buy-in from everyone. If you don’t have coding standards, then don’t let the discussion turn into a pissing contest over coding styles (opening braces ‘{‘ on the same line or the next!) If you run into a situation like that, take the discussion offline to your coding standards forum.
  • Learn to communicate well. You must be able to clearly express your ideas and reasons.
  • When it comes to dealing with opinions, reviewers and authors should seek to understand each other’s perspectives but shouldn’t get into a philosophical debate.

Advice to Reviewers

  • The author isn’t there to be a sitting duck. Remember the purpose is to not demonstrate who the better programmer is: it is finding defects and ensuring that the code is simple and maintainable (or whatever your objectives are.)
  • Leave actionable feedback.
  • Ask questions when you are not sure about something. Don’t make demands or statements which could sound accusatory. For example, don’t say: “You didn’t use the XYZ library here”. A better way would be to genuinely seek to understand the developer’s perspective: “What do you think about library XYZ and if it applies here?”.
  • Avoid “why did you”, “why did you not” style questions when possible. It could put people on the defensive. “Why did you make this a global variable?” could be better expressed as “I don’t understand why this needs to be a global variable. Can you explain?”.
  • Use we instead of you.
  • If you are providing a suggestion, call it out as such. For example, instead of saying: “make the color one shade more neutral”, you should say: “(suggestion/opinion) we should make the color one shade more neutral”.
  • Review quickly and block time for code reviews.
  • If the author addressed your comments or did something great that you noticed, tell them! Most code reviews focus on finding mistakes but you should offer your appreciation and encouragement if the developer did something great. This type of feedback is super encouraging to developers and goes a long way!

Some of these things won’t work if they come off as rehearsed or said in a sarcastic tone. Treat the code review as you would a normal conversation. You are listening to another person and should genuinely seek to understand their perspective. Offer suggestions and tips when they are necessary. If the code is great, don’t be compelled to find something negative to say about it.

Advice to Authors

  • Don’t take things personally. Remember that the villains are the defects or inadequacies in the code, not you. Recognize that you may be attached to your code and that it is normal. If you take pride in your work, that’s a good sign that you are someone who cares about the craft. Have just the right amount of ego – enough to trust and defend your ideas, that is the ability to negotiate.
  • Don’t create mega-gigantic pull requests (10+ files changed.) Ask for reviews often and keep your pull requests small. If you can’t avoid this, give heads up to others (ahead of time such as during Sprint Planning) and set up some time to describe your changes and requirements.
  • Describe your change: provide a complete description and link to the JIRA ticket so that reviewers can understand the requirements.
  • Add the right reviewers. You want to get the best feedback so it is important that you add the right people. For example, if you are making a change in the React code, make sure you add people who are not only familiar with not only React, but also with that part of the code. If you are making a change in a microservice that’s owned by a different team, add them as reviewers. On instances, you might even need to assign different people to different parts of the PR.
  • To err is human. The reviewer is acting as a second set of eyes and could point out things that you might have overlooked. Questions are as valuable as concrete advice.
  • Ask specific questions. “Does it make more sense to move all these classes into their own package?”
  • Respond to all feedback, whether you agree or disagree.

Code Review Example

Modern tools like GitHub, GitLab, Crucible and others have made the code review process easier than ever. Let’s look at a good code review example I found on GitHub. The developer made some changes and opened a pull request (PR). The first thing to notice is the detailed and to the point commit message by the author describing the change:

GitHub PR Commit Message for Code Review

Next, we see that the code reviewer suggests some changes, and explicitly calls out “good ideas” to encourage the contributor.

GitHub PR Comment for Code Review GitHub PR Comment for Code Review, Good Idea

The reviewer then approves the PR and leaves this final message:

It’s great to see this fixed, thank you! Nice explanation + commit message too.

Another improvement we’ve considered is to debounce the scrollback management #8959 (or ideally put it in a ring buffer, but I’m not sure if that can work with libvterm).

Summary

Code review is an excellent technique for improving software quality when done right. Code reviews not only find defects but also helps developers grow by getting them feedback on their code from other developers, spreading code knowledge throughout and are a useful tool for mentoring new team members. Code reviews are part of a healthy engineering culture.

Updated: June 2022 to add Code Review example.

I would love to hear your feedback, comments, thoughts on conducting effective code reviews. Please leave a comment below sharing your experience or anything that would add value to this article and its future readers.

#codereviews #programming

You May Also Enjoy


If you like this post, please share using the buttons above. It will help CodeAhoy grow and add new content. Thank you!


Comments (3)


Radoslaw Szulgo

I’m rather an idealist. I tend to point out details in code review like “empty blocks” or misspelling in comment. People might find it offensive, rude etc.

what’s you experience in that ?


Umer

I definitely look for spelling mistakes in variables and other identifiers. Similarly, comments are code documentation and should be free of spelling errors. I wouldn’t consider it pedantic, just good practice :-) It takes one broken window to convey sense of abandonment or that “no one here cares”. (https://pragprog.com/the-pr…

Most modern IDEs (IntelliJ) will detect misspelled words so its not that difficult. Also, to conform code to common style (i.e. remove empty blocks), consider integrating a ‘style checker’ (https://github.com/google/s… into your build process.

I hope this helps.


trankang

one dev I worked with insisted adding full JavaDoc style comments on every private method including 10-liners


Speak Your Mind