EFFECTIVE CODE REVIEW: Uprooting Source Code Defects

Preface

In software development, a small coding error can result in a critical vulnerability that may lead to malfunction of an entire system. There are many techniques for detecting software defects such as static code analysis, manual and automated testing, pair programming, continuous integration, and of course, code inspections. However, detecting defects in software product development requires serious effort, so it is important to use the most efficient and effective methods.

This series of articles describe the whys and hows of the code review process. It is a primer for those who need to understand the concepts as well as the value of peer review methods. In case you faced a dilemma whether to integrate code review in your development process? If you are looking for the answer of how code reviews can benefit you to improve your code quality.  What do you get out of code review? This article will guide you through the different aspects of code review best practices.

The following content is intended for programmers, project and product managers, and individuals who take part in the software development process. It is assumed that readers familiar with basic programming aspects, software development processes, and approaches, and with version control systems usage.

What Code Review Is?

Code review also referred to as peer review, is a systematic examination of software source code. Such code analysis is performed to find bugs, defects, architecture shortcomings, and improve the overall quality of the software. In other words, it is the evaluation of work by one or more people of similar or higher competence to the producers (authors) of the code. Actually, the review may be applied to a software product in general. The “software product” term  means any technical document or partial document, produced as a deliverable of a software development activity, and may include documents such as contracts, project plans and budgets, requirements documents, specifications, designs, source code, user documentation, support and maintenance documentation, test plans, test specifications, standards, and others.

The job of a code review is to determine what is being done well as well as what can be done better. It is not a witch-hunt. By sharing these findings, the top echelon of developers and brain new junior developers can all learn to be better developers. They can develop the style that meets team’s needs. However, the same things that can be helpful for one group or team may not be helpful to another. Creating things like coding standards and code analysis rules can be helpful in some organizations. Creating an overly stressful environment of giving people, what they can do and what they cannot do — is not helpful in every case.

Why Review Code?

Code review is an essential mechanism that allows programmers get feedback about their code. Code review techniques are also employed to maintain standards of quality, improve performance, and provide credibility.

Within your organization, you could be reviewing code and should be reviewing code as well. This is not something you can bring in outside out to do in a systemic sense. You going to want your team, your groups or even individual developers to be looking back at what they have written to learn from it. Again, this is about learning opportunity. Therefore, we do code reviews because we want to improve the quality of the team, improve the quality of the communication between team members (and often between teams) improve the quality of the code, and the last thing you are going to be looking for when you review code is to find coding problems. All four of these are equally important. Again, doing code reviews is going to allow you to improve all four of these elements. Not just one or the other.

1. Improve the quality of the team.
2. Improve the quality of the communications between team members and often between teams.
3. Improve the quality of the code.
4. Find coding problems.

All of these four items are equally important.

McConnell provides a lot of evidence for the efficacy of code reviews in Code Complete. Here are some of them:

Software testing alone has limited effectiveness — the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for integration testing. In contrast, the average effectiveness of design and code inspections are 55 and 60 percent. Case studies of review results have been impressive:

• In a software-maintenance organization, 55 percent of one-line maintenance changes were in error before code reviews were introduced. After reviews were introduced, only 2 percent of the changes were in error. When all changes were considered, 95 percent were correct the first time after reviews were introduced. Before reviews were introduced, under 20 percent were correct the first time.
• In a group of 11 programs developed by the same group of people, the first 5 were developed without reviews. The remaining 6 were developed with reviews. After all the programs were released to production, the first 5 had an average of 4.5 errors per 100 lines of code. The 6 that had been inspected had an average of only 0.82 errors per 100. Reviews cut the errors by over 80 percent.
• The Aetna Insurance Company found 82 percent of the errors in a program by using inspections and was able to decrease its development resources by 20 percent.
• 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.

The only obstacle to a code review is finding a developer you respect to do it, and scheduling the time to perform the review. Once you get started, I think you will quickly find that every minute you spend in a code review is paid back tenfold.

How to Review Code?

Your job as a code reviewer is to take your experience to bear. This means you need to take what you have learned through the years of developing your skills of development and look at the code through that lens:

• Find things to laud and things to fix
• Look for common problems, not one-off issues
• Review 200 – 400 lines at a time
• Reviewing the whole code base is time-consuming

What to look for?

Here are some concrete examples of problems to look for:

Bugs or potential bugs.

Disagreement between code and specification. Off-by-one errors. Risk variable scopes. Optimistic, undefensive programming.

Unclear, messy code.

Bad variable or method names. Inconsistent indentation. Convoluted control flow (if and while statements) that could be simplified. Packing too much into one line of code, or too much into one method. Failing to comment obscure code. Having too many trivial comments that are simply redundant with the code.

Misunderstandings.

Misuse of == or .equals(). Misuse of arrays or Lists.

Missing (or failing to use) essential design concepts.

Incomplete or incorrect specification for a method or class. Representation exposure for a data abstraction. Immutable data types that expose themselves to change. Invariants that are not invariant, or are not even stated. Failure to implement the Object contract correctly (equals and hashCode).

Respect

Be polite

Sarcasm, insults, and belittling words have no place in a code review. It does not matter whether you are talking about a person (a fellow reviewer or a code author) or about code. Do not call code “stupid,” because that transfers all too easily to the author of the code, whether you meant it that way or not.

Be constructive

Do not just criticize, but also be helpful: point the way toward solutions. “Hopeless mess” is not a constructive comment. “Name the variables more descriptively, e.g. tmp1 is not a great name” is much more constructive.

As a reviewer, you should down-vote comments by others that are unconstructive or rude.

As a code author, you should read your feedback carefully, and keep an open mind. Do not get defensive. If your reviewers find your code confusing, then you should consider what this says about its clarity and maintainability in the real world. If you disagree with a comment, you can reply to it and engage the reviewer in a discussion.

Benefits

The benefits of code review in practice are several.

Reviewing helps find bugs, in a way that is complementary to other techniques (like static checking, testing, assertions, and reasoning). Reviewing uncovers code that is confusing, poorly documented, unsafe, or otherwise not ready for maintenance or future change.

Adhere coding standards

Completed source code should reflect a consistent style as if a single developer wrote the code in one session. At the inception of a software project, establish a coding standard to ensure that all developers on the project are working in conjunction.

When the software project will incorporate existing source code, or when performing maintenance upon an existing software system, the coding standard should state how to deal with the existing code base.

Although the primary purpose of conducting code reviews throughout the development life cycle is to identify defects in the code, the reviews can also be used to enforce coding standards in a uniform manner.

Adherence to a coding standard can only be feasible when followed throughout the software project from inception to completion. It is not practical, nor is it prudent, to impose a coding standard after the fact.

Knowledge sharing

Reviewing also spreads knowledge through an organization, allowing developers to learn from each other by explicit feedback and by example. Therefore, code reviewing is not only a practically important skill that you will need in the real world but also a learning opportunity.

Education

Somebody can teach you the core principles of snowboarding. An instructor can tell you about gravity, friction, angular momentum, the center of mass, and so forth. The instructor can also demonstrate how to stand on a snowboard, keep balance, and slide with acceleration. However, you would still fall down the first time you try.

Software development is no different. You could write down all the “feel good” principles of clean code and then trust you to do the work (in other words, let you fall down when you get on the snowboard).

You cannot learn how to write without learning how to read, and programming is no exception.  Learning to write clean code is hard work. It requires more than just the knowledge of principles and patterns. You must sweat over it. You must practice it yourself, and watch yourself fail. You must watch others practice it and fail. You must see them stumble and retrace their steps. You must see them agonize over decisions and see the price they pay for making those decisions the wrong way.

Training

In many companies developers are assigned to the project because they are available, rather their skill sets are suitable. Generally, such team members learn from on-the-job training. On-the-job training took place in many forms. Less experienced developers studiously copy the class designs and implementations produced by senior developers. While copying is a useful mechanism for propagating knowledge, the less experienced developers do not always know whether what they are copying is appropriate to the problem.

Learning by example is a fine strategy, but you will still need to provide mentoring to the development staff to make sure that they are learning the appropriate lessons.

The wide variety of skill levels placed a burden on the more senior developers to answer questions on demand. The senior developers fell into the habit of making daily visits to the other team members on arrival in the morning to provide guidance and feedback. The strong communication skills of the senior developers are a great asset in helping the less experienced developers learn and grow.

Of course, some developers understand how to write clean code by using best practices and common patterns without any code reviews.

Metrics

Code review may become more effective by collecting different metrics, which can later be used for the process improvement. For instance, metrics for types and quantity of defects may be placed on record. Further analysis of these metrics may reveal problem code areas.

Code Coverage

If you are the project manager or team leader, have you ever wondered how much of the application code is covered by code reviews? Alternatively, if you are the developer, have you ever wished you knew if there is any code that is no longer being called? Code Coverage report makes it possible for PM professionals to know how much of the application code is being covered by Code Review.

You might be asking how this is different from Unit Testing Code Coverage. Essentially, both measure and track how much of the code is being covered by tests. Unit Test code coverage is a good metric but does not necessarily measure the quality of the tests. Unit tests that cover a lot of code may still not be good tests. Therefore, if all of the application functionality and scenarios have to be executed and there is still code that has not been tested, this code should be investigated if it is valid.

Read Also

EFFECTIVE CODE REVIEW: Formal Approach to the Process