Next / Previous / Contents / Shipman's homepage

10. The peer review process

Whenever possible, get others to look at your code. They may perceive blind spots and invalid assumptions that you had when you wrote the code. They can also help you catch simple problems like unbalanced parentheses.

A good peer review panel should consist of one or a few people who meet these qualifications.

Here is an outline of the peer review process for one prime.

  1. Find a suitable meeting place. You will want enough seating and tables so the review panel can comfortably review every detail of the code without time pressure (avoid running peer reviews just before lunch or quitting time!).

  2. Make the code available to all the reviewers. A projector is a good tool, provided that everyone can read it. The legibility test is strict: everyone should be able to tell a period from a speck of dirt – computers care about periods and commas.

  3. Review the overall intended function and the intended functions of its component pieces for clarity. If it is not clear to each reviewer that the intended function's meaning is completely clear, the author must rework it and try again later.

  4. The author uses the proof rules for the four standard branch constructs to argue that the code is correct. During this phase, ignore the actual source code: stick to comparing the prime's overall intended function to the intended functions of its components.

    Again, the test is very strict: unless ever peer reviewer is convinced that the decomposition of the prime into smaller pieces is correct, the prime is sent back for rework.

  5. Provided that the decomposition of the prime into smaller primes is correct, the review panel then checks each piece's intended function against the code that implements it. This is why reviewers must know the language well.

    If the code calls any methods or functions that are also part of the overall project, the intended functions for those primes must be available during the review. The panel will check that both the intended functions and the actual calling sequences match their definitions.

    What about calls to Python language primitives and modules? Here we must treat these facilities as trusted components. Although they are out of our direct control, it is necessary to assume that the underlying components are correct. This is admittedly a potentially major limitation with the method, but short of implementing our own languages and libraries, we may not have any other reasonable choices.

    If the review process catches minor errors such as unbalanced parentheses, there is no reason to send the prime back for rework; the author can correct them on the spot and proceed.

    However, once again the test for correctness is quite strict. Unless it is completely obvious to all reviewers that the code is correct, it is sent back to rework.

If one of the reviewers has a better idea for the general approach, data structures, or algorithms, try to achieve consensus on whether it is worthwhile to abandon the current approach. Consider the real-world constraints (time, budget, floor space, staffing). If there are two or more promising approaches to a design problem, and there are no strong reasons to choose one of them, whoever writes the code gets to pick the approach.

Once a prime passes review, place a comment in the source code specifying the date and time when it was reviewed, and the full names of all the reviewers. If defects appear later, we can inform the reviewers about anything missed, so they can improve their reviewing skills.

If a prime is changed after passing review, it must be reviewed again. Furthermore, any other certified component that uses the changed prime must also be reviewed again. For smallish projects, this process can be managed by informal means.

However, for Cleanroom to be effective in a large project, we would really like some software tools that keep track of dependencies, and can insure that all affected modules are re-verified. Such software tools would timestamp the definition of each prime, track the verification state of each prime, and be able to compute a list of modules to be re-verified.

10.1. Guidelines for reviewers

  • Be respectful and courteous. Avoid terms like “brain-dead” or “stupid” or “asinine;” engineering practice is hard enough without involving people's egos and emotions.

    A better approach is to use a phrasing like “In step 2, did you mean…” or “Would it be clearer…” or “Maybe we could reduce the number of steps if….”

  • Be positive. Emphasize why a change you suggest is better, not why the current code is worse.

  • Pay attention. Try to see both the big picture and the tiny details and everything in between.

    If the big picture isn't clear, ask the author for more explanation of the context. Most primes can be reviewed without very much reference to outside entities, except when the code calls or uses outside definitions, functions or methods.

  • If the code uses a language feature that you don't recognize, you are within your rights to ask for an informal explanation of the feature, or a quick visit to the Web page where the feature is defined or explained.