Activity: Review Code

General Recommendations

When you are building high-quality software, reviewing the implementation is a complement to other quality mechanisms, such as compiling, integrating and testing. Before you review the implementation, compile it, and use tools, such as code-rule checkers, to catch as many errors as possible. Consider using tools that allow the code to be visualized. Additional errors may also be detected and eliminated prior to implementation review if the code is executed using run-time error detection tools.

The benefits of reviewing the implementation are:

• To enforce and encourage a common coding style for the project. Code reviewing is an effective way for members follow the Programming Guidelines. To ensure this, it is more important to review results from all authors and implementers, than to review all source code files.
• To find errors that automated tests do not find. Implementation reviews catch different errors to those of testing.
• To share knowledge between individuals, and to transfer knowledge from the more experienced individuals to the less experienced individuals.

There are several techniques that can be used to review the implementation. Use one of the following:

• Inspection. A formal evaluation technique in which the implementation is examined in detail. Inspections are considered to be the most productive review technique, however it requires training, and preparation.
• Walkthrough. An evaluation technique where the author of the implementation, leads one or more reviewers through the implementation. The reviewers ask questions, and make comments regarding technique, style, possible error, violation of coding standards, and so on.
• Code reading. One or two persons read the code. When the reviewers are ready, they can meet and present their comments and questions. The meeting can be omitted, however, and reviewers can give their comments and questions to the author in written form instead. Code reading is recommended to verify small modifications, and as a "sanity check."

Skill requirements for this role are similar to those for Role: Implementer; people playing this role are often considered experts in the programming language used for the code being reviewed. In most projects, this role is staffed using senior programmers from the implementation team.

See also Guidelines: Reviews.

Establish Checkpoints for the Implementation

This section gives some general check-points for reviewing the implementation, just as examples of what to look for in a review. The Programming Guidelines should be the main source of information for code quality.

General

• Does the code follow the Programming Guidelines?
• Is the code self-documenting? Is it possible to understand the code from reading it?
• Have all errors detected by code-rule checking, and / or run-time error detection tools been addressed?

Commenting

• Are comments up to date?
• Are comments clear and correct?
• Are the comments easy to modify, if the code is changed?
• Do the comments focus on explaining why, and not how?
• Are all surprises, exceptional cases, and work-around errors commented?
• Is the purpose of each operation commented?
• Are other relevant facts about each operation commented?

Source code

• Does each operation have a name that describe what the operation does?
• Do the parameters have descriptive names?
• Is the normal path through each operation, clearly distinguishable from other exceptional paths?
• Is the operation too long, and can it be simplified by extracting related statements into private operations?
• Is the operation too long, and can it be simplified by reducing the number of decision points? A decision point is a statement where the code can take different paths, for example, if-, else-, and-, while-, and case-statements.
• Is nesting of loops minimized?
• Are the variables well named?
• Is the code straightforward, and does it avoid "clever" solutions?

Prepare Review Record and Document Defects

Following each review meeting, the results of the meeting must be documented in a Review Record. In addition, defects must be documented in Change Requests (and eventually assigned to someone to own and drive to resolution).

Rational Unified Process