This is a summary of material on the subject of software reviews and
inspections. If your machine has access to our cluster, you can also
view the
pt.kernel reviews page.
More material to follow...
All technical reviews, despite their differences, are based on the idea that developers are blind to some of the trouble spots in their work, that other people don't have the same blind spots, and that it's beneficial to have someone else look at their work.
McConnell provides evidence and references for the effectiveness of reviews. Reviews are more effective than testing; reviews also catch different kinds of errors; reviews contribute to a quality program even in the presence of effective testing. McConnell describes three kinds of review: inspections, walk-throughs, and code reading.
Inspections
Inspections use checklists, and focus on defect detection, not correction. Meetings are prepared beforehand. Design and code inspections remove 60% or more of the defects in a product.
Each person involved in an inspection has a distinct role: moderator, author, reviewer, or scribe. Management is not allowed in. The moderator's job is to manage the inspection and keep it moving. The author's job is to clarify and explain. The reviewer's role is to find defects, both before and during the meeting. Each of these roles are to be done by separate people. Inspections should have between three and six people, and last no more than two hours.
The correct frame of mind is essential:
The purpose of an inspection is to discover defects. It is not to explore alternatives or to debate about who is right and who is wrong. The point is most certainly not to criticize the author of the design or code.
Reviewers spend about 90 minutes preparing for the meeting, to become familiar with the code. In the meeting, the author explains the code, the reviewers detect errors, and the scribe note them. Solutions are not discussed during the review -- only defects.
The moderator produces a defect report and assigns defects to people to fix. Usually this is the author.
Acknowledging a criticism doesn't mean that it is true. The author should not try to defend the work under review. After the review, the author can think about each point in private and decide whether it's valid.Reviewers must remember that the author has the ultimate responsibility for deciding what to do about a defect.
Removing or combining parts of the inspection is a false economy: all parts are necessary (which is why you should read the chapter, not just this summary).
Effective inspections require checklists, but McConnell does not provide a sample.
Walk-throughs
The term "walk-through" is loosely defined. Walk-throughs are less formal than inspections, and require less practice to be effective -- they can also be substantially less effective than inspections.
A walk-through is usually moderated by the author. The purpose is to improve the technical quality of a piece of code -- again, the emphasis is on error detection, not correction. Participants prepare for the meeting by reading the code looking for errors. Walkthroughs last 30 to 60 minutes.
Code reading
You read code and look for errors, commenting on design, style, maintainability etc. Code reading focuses on individual reading of the code than on the meeting. (The meeting is not even necessary.)
Two or more people read the code independently. The meeting focuses on problems discovered by the code readers, and lasts one to two hours. The code is not examined line-by-line. The author fixes the identified problems after the meeting.
McConnell points out that products with the lowest defect rates have the shortest schedules, and that early detection of defects is more effective than late detection. He summarizes code reading, walk-throughs, and inspections, and makes this comment:
Technical reviews are a useful and important supplement to testing. Reviews find defects earlier, which saves time and is good for the schedule. They are more cost effective on a per-defect-found basis because they detect both the symptom of the defect and the underlying cause of the defect at the same time. Testing detects only the symptom of the defect; the developer still has to isolate the cause by debugging. Reviews tend to find a higher percentage of defects (Jones 1986).
Desk-checking is a private review and debugging carried out by individual programmers and analysts who were not involved in the software product creation. Defects are identified and logged during the review. Defect resolution, status tracking and communication take place after the review (Capers, 1994). Desk-checking advantages and disadvantages are: it is the least expensive, it is easy to schedule and complete; however it is typically the least effective review method and effectiveness depends largely on the reviewer.