October 01, 2001
When Two Eyes Aren't EnoughEnlist your colleagues to help you find those elusive problems in your software.
Hey, Maria, do you have a minute? I can't find a bug in this program. Would you take a look at the code?" "Sure, Phil. What's it doing wrong?" "It's not aligning these images correctly. They're supposed to be left justified, but each one is indented farther in. I'm pretty sure the problem's in the DisplayImage method, but I've been looking at it for 15 minutes and can't see anything wrong." "Hmmm, let's see. It looks like ... No, that looks fine. But check out the top of this loopI think this parenthesis is in the wrong place. If you move the index variable inside that paren, I don't think the images will be stair-stepped any more." "You're right! Thanks, Maria. I can't believe I didn't see that." "No problem. I'm glad we found it now rather than later." Most programmers have asked their colleagues to help them find elusive problems in their code through an ad hoc review like the one Phil and Maria performed. Often you're too close to your own work to spot any errors. As you study the code, your brain recites what you created earlier (or what you intended to create) because you're following the same reasoning you used when you made the mistake. You need a pair of eyes that haven't seen the code before, and a brain that thinks in a different way. There are several distinct peer review approaches, but software practitioners use conflicting terminology to describe them. This article describes several kinds of peer reviews that span a range of formality and structure, and provides guidance for selecting an appropriate review technique for a given situation.
Figure 1 places several common review methods on a formality spectrum. The most formal reviews, exemplified by inspections, have several characteristics:
Even informal reviews are worth doing and may meet your needs in certain situations. You should recognize the strengths and limitations of the various approaches so you can select a review process that fits your culture, time constraints and business objectives. All peer reviews involve some combination of planning, studying the work product, holding a review meeting, correcting errors and verifying the corrections. Table 1 shows which of these activities are typically included in each of the review types shown in Figure 1.
Inspection A fundamental aspect of inspection is that participants other than the work product author lead the meeting (moderator), present the material to the inspection team (reader) and record issues as they're brought up (recorder). Participants prepare for the inspection by examining the material on their own to understand it and to find problems. During the meeting, the reader presents the material a small portion at a time to the other inspectors, who raise issues and point out possible defects. The reader helps the team reach the same interpretation of each portion of the product, because the inspectors can compare their understanding to that expressed by the reader. At the end, the team agrees on an appraisal of the work product and decides how to verify the changes that the author will make during rework. Inspections are more effective at finding defects than informal review techniques. One telecommunications firm, I'm told, detected an average of 16 to 20 defects per thousand lines of code by inspection, compared to only three defects per thousand lines of code using informal reviews. The close scrutiny of an inspection provides a thorough test of a product's understandability and maintainability, and reveals subtle programming problems. Different inspectors spot different kinds of problems, but this contribution doesn't increase linearly with additional participants. When one inspector notes a defect, it's common to hear another participant say, "I saw that, too." The interactions between inspectors during the meeting can reveal new bugs as one person's observation stimulates another's thinking. You lose this collaborative benefit if your peer review doesn't include a meeting, although some studies have questioned whether the synergy adds any real value (see Lawrence G. Votta Jr.'s "Does Every Inspection Need a Meeting?" Proceedings of the ACM SIGSOFT Symposium on Software Development Engineering [ACM Presss, 1993], and Adam A. Porter and Phillip M. Johnson's "Assessing Software Review Meetings: Results of a Comparative Analysis of Two Experimental Studies," IEEE Transactions on Software Engineering, Mar. 1997).
Team Review they're a bit less formal and comprehensive. As I've practiced them, team reviewssometimes called "structured walkthroughs"follow several steps found in an inspection. The participants receive the review materials several days prior to the meeting, and are expected to study the materials on their own. The team collects data on the review effort and the number of defects found. However, the overview and follow-up inspection stages are simplified or omitted, and some participant roles may be combined. The author may lead the team review, and in contrast to most inspections, the reader role is omitted. Instead of having one participant describe a small chunk of the product at a time, the review leader asks the participants if they have any comments on a specific section or page. Team reviews cost more than having a single colleague perform a peer deskcheck, but the different participants will find different sets of errors. The recorder or scribe captures issues as they're raised, using standard forms the organization has adopted. While little data is available, one industry study (see Erik P.W.M. Van Veendendaal's "Practical Quality Assurance for Embedded Software," Software Quality Professional, June 1999) found that team reviews discovered only two-thirds as many defects per hour of effort as inspections revealed. IBM Federal Systems Division measured coding productivity for projects that practiced team reviews at about half the productivity of a set of similar projects that used inspections. Team reviews are suitable for a team or product that doesn't require the full rigor of the inspection process.
Walkthrough Walkthroughs typically don't follow a defined procedure, require no management reporting and generate no metrics. They can be an efficient way to review products modified during maintenance because the author can draw the reviewers' attention to those portions of the deliverables that were changed. However, this runs the risk of overlooking other sections that should have been changed but were not. Because records are rarely kept, there is little data about how effective walkthroughs are at detecting bugs. Inspections at Ford Motor Company discovered 50 percent more defects per thousand lines of code than did walkthroughs (see Bankes, Kirk and Sauer's "Ford Systems Inspection Experience," Proceedings of the 1995 International Information Technology Quality Conference, Quality Assurance Institute, Orlando, Fla.). When walkthroughs don't follow a defined procedure, people perform them in diverse ways. In a typical walkthrough, the author presents a code module or design component to the participants, describing what it does, how it's structured and performs its tasks, the logic flow, and its inputs and outputs. Finding problems is one goal; reaching a shared understanding of, and a consensus on, the component's purpose, structure and implementation is another. Design walkthroughs provide a way to assess whether or not the proposed design is sufficiently robust and appropriate to solve the problem. If you don't fully understand some information presented in a walkthrough, you might assume the author is right. It's easy to be swayed by an author's rationalization of his approach, even if you aren't convinced. Sometimes, though, a reviewer's confusion is a clue that a defect lurks nearby. Walkthroughs have a place in your peer review toolkit, but the author's dominant role and the unstructured approach render them less valuable than either inspections or team reviews as a defect filter.
Pair Programming Culturally, pair programming promotes collaboration, collective ownership of the team's code base and a shared commitment to the quality of each component. At least two team members become intimately familiar with every piece of code, reducing the knowledge lost through staff turnover. The pair can quickly make course corrections because of the real-time review. I classify pair programming as a type of informal review because it's unstructured and involves no process, preparation or documentation. It lacks the outside perspective of someone who isn't personally attached to the code that a formal review brings. Nor does it include the author of the parent work product as a separate perspective. Pair programming is not specifically a review technique, but rather a development strategy that relies on the synergy of two focused minds to create products superior in design, execution and quality. However, pair programming is a major culture change in the way a development team operates, so it's not a simple replacement for traditional peer reviews in most situations.
Peer Deskcheck The peer deskcheck is the cheapest review approach. It involves only one reviewer's time plus perhaps a follow-up discussion with the author to explain the reviewer's findings. This method is suitable for low-risk work products, if you have colleagues who are skilled at finding defects on their own or if you have severe time and resource restrictions. A peer deskcheck can be more comfortable for the author than an intimidating group review. However, the only errors found will be those that the one reviewer is best at spotting. Also, the author isn't present to answer questions and hear discussions that can help him find additional defects that no one else sees. Peer deskchecks provide a good way to begin developing a review culture. Find a colleague whom you respect and trust, and offer to exchange work products for peer deskchecks.
Passaround As an alternative to distributing physical copies of the document to reviewers, one of my groups placed an electronic copy of the document in a shared folder on our server. Reviewers contributed their comments in the form of document annotations, such as Microsoft Word comments or PDF notes, for a set period of time. Then the author reconciled conflicting inputs from the reviewers, making obvious corrections and ignoring unhelpful suggestions. Only a few issues remained that required the author to sit down with a particular reviewer for clarification or brainstorming. This method allows each reviewer to see the comments others have already written, which minimizes redundancy and reveals differences of interpretation. Watch out for debates that might take place between reviewers in the form of document comments; those are better handled through direct communication. Document reviews of this type are a good choice when you have reviewers who can't hold a face-to-face meeting because of geographical separation or time limitations.
Ad Hoc Review
Choosing a Review Approach
You can also select a review technique based on your stated objectives. Table 3 suggests which review approaches are best suited to achieve specific objectives. The best way to select a peer review method is to keep records of review effectiveness and efficiency. You might find that inspections work well for code, but team reviews or walkthroughs work best for design documents. Data provides the most convincing argument, but even subjective records of the kind of reviews you did, the work products that you examined and how well the reviews worked will be useful. Most importantly, begin developing a culture that embraces peer review as a contributor to increased software quality, productivity and individual learning.
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||