Reading Time: 5 minutes

In this post we’ll cover code review, what it is (and what is not), why you should do it (if you’re not already doing it), tips etc.

So, what is a code review? Code review is examination of source code; a process where developers inspect a set of program code, which can find and remove vulnerabilities in the code such as bugs and memory leaks, ensure code is correct, maintainable and readable. Code review gives developers opportunities to correct bad habits, learn new tricks, and expand their capabilities. It provides a second pair of eyes to find defects and better ways of doing something and spread knowledge of the code to the entire team. Also this process encourages developers to be more thorough in their work since they know it will be reviewed by one of their colleagues. During this process you can detect a lot of mistakes overlooked in software development, thus improving the overall quality of software.

And don’t mistake code review for testing – code review is a developer-to-developer business and it doesn’t involve any testing. Code review should check if the requirements are met in the cleanest possible way. You don’t tell others what to review – the same way you don’t tell a tester what to test, you should never tell your colleague what to review.

 

Maintain a positive attitude

For successful code reviews teams need to maintain positive attitude that code reviews are useful (for many reasons) and that finding defects means that the author and reviewer have successfully worked as a team to improve the code (and product). It’s not a case of “developer created a defect and the reviewer found it.” It’s more like a form of pair-programming.

Reviews present opportunities for all developers to learn new tricks. They can correct bad habits, learn from their mistakes and expand their capabilities and knowledge. If developers are afraid of the review process, the positive results disappear. You should never single out developers that made mistakes, especially not in front of their colleagues (this can seriously damage morale).

As a developer, keep an open mind and positive attitude and be prepared to make compromises if your reviewer suggests a different approach. Don’t take review comments personally. Just because someone feels that you should refactor some duplicated code into a reusable function, it doesn’t mean that you are any less of an attractive, brilliant and charming individual.

As a reviewer, be tactful and consider whether your proposal is really better (or just a matter of taste) before suggesting some change.

 

Why do we do code reviews?

Main code review objectives are:

• provide a second pair of eyes to find defects and better ways of doing something
• catch bugs, ensure code is correct, maintainable and readable
• ensure that at least one other person is familiar with your code
• help train new staff by exposing them to the code of more experienced developers
• promote knowledge sharing by exposing both the reviewer and reviewee to the good ideas and practices of the other
• encourage developers to be more thorough in their work since they know it will be reviewed by one of their colleagues

However, these goals cannot be achieved unless appropriate time and care are devoted to reviews. Quickly scrolling through a code does not constitute a thorough code review. Reviewer must take time and understand code he’s reviewing and not only check indentations and that all the variables use lower case – he should understand how the code fits into the larger context of the application, component or library it is part of.

 

Lightweight code review

About thirty years ago, if you wanted a process that delivered results, your only choice was the dreaded Formal Inspection. It was a multi-phase, multi-meeting, paperwork-heavy system popularized by Michael Fagan at IBM (this is a reason why this process is also called Fagan Inspection). Properly done, it takes a lot of time (couple of hours) to review at most 200 lines of code. We can all agree it’s a sluggish rate that makes this system impractical even if it does uncover bugs. In the past 10, 15 years a variety of evidence has shown that many of the components of the traditional “heavyweight” inspection process are inefficient or even unnecessary.

Four lightweight techniques:

• Over-the-shoulder: One developer looks over the author’s shoulder as the latter walks through the code. This is the informal and easiest technique of code review. Pros: Easy to implement, it doesn’t require any tools. Cons: Hard to know which code is reviewed, interrupts reviewers, impossible to measure and track. Requires coordinating with someone else’s schedule.
• Email pass-around: Developer (author) emails code to reviewers. Reviewers respond to changes when they have time. Pros: Easy to set up, nothing to purchase, works remotely. Cons: No tracking – don’t know if found defects were fixed, often reviews never happen because everyone assumes someone else looked at it, no metrics. Hard to follow conversations as emails get replied to and forwarded among several people.
• Pair Programming: Two authors develop code together at the same workstation (you can read more about it here). Pros: Deep level of review; teams know review is happening but it’s difficult to track. Cons: Large time investment, some developers dislike it. Requires coordinating with someone else’s schedule.
• Tool-assisted: Author and reviewer use specialized tools designed for code review. Pros: Removes most of the busywork associated with file-collection, discussion/defect management, and metrics/reporting. Tracks reviews and defects. Cons: Have to learn and integrate another tool, can cost money.

 

Code review tips

And for the end of this post here are some code review tips:

• Do code reviews regularly
• Review 200 – 400 lines of code at a time – code review study made by Cisco showed that for optimal effectiveness, developers should review fewer than 200-400 lines of code (LOC) at a time. Beyond that, the ability to find defects diminishes.
• Aim for an inspection rate of fewer than 300–500 LOC per hour – take your time with code review. Faster is not better. Research shows that you’ll achieve optimal results at an inspection rate of less than 300–500 LOC per hour.
• Take enough time for a proper review, but not more than 60–90 minutes – never review code for more than 90 minutes at a stretch.
• Developer should annotate his code – by adding comments developer will double-check his work and possibly eliminate defects – shorter review time.
• Use checklists, because they substantially improve results for both authors and reviewers – checklists are especially important for reviewers because, if the author forgets a task, the reviewer is likely to miss it also.
• Verify that the defects are actually fixed
• Foster a good code review culture in which finding defects is viewed positively – team leaders and managers should promote it as a means for learning, growing, and communication. It’s easy to see defects as a bad thing (after all, they are mistakes in the code), but fostering a negative attitude toward defects found can sour a whole team, not to mention sabotage the bug-finding process. The point of software code review is to eliminate as many defects as possible, regardless of who “caused” the error.
• Review important (right) things, and let tools do the rest – you don’t need to argue over code style and formatting issues. There are plenty of tools which can consistently highlight those things. Ensuring that the code is correct, understandable and maintainable is what’s important.
• Everyone should code review – some people are better at it than others. The more experienced may well spot more bugs, and that’s important. But more important is maintaining a positive attitude to code review in general and that means avoiding any ‘Us vs. Them’ attitude.
• Review all code
• Adopt a positive attitude
• It’s OK to say “It’s all good” – don’t get picky, you don’t have to find an issue in every review.