Reviewing

• People – even engineers! – are fallible
• Psychologically you tend to see what you expect to see

Reviewing is a process by which others try to find faults in your work.

Useful at various stages of development:

• Design review
  • have all eventualities been thought of?
  • is it more complicated than it needs to be?
• Code review
  • does this code do what it was meant to?
• Test review
  • are there cases going untested?

Reviewing can seem like an informal process. Unlike, for example, a compiler's syntax checking it does not guarantee to find every fault (of a certain class) that may be present. Nevertheless it can be a very valuable process. Being a human process the ‘terms of reference’ are not strictly limited and all sorts of things can turn up.

The basic principle is to get other engineers to look at a design (code etc.) and for them to try and spot things the designer hadn't thought of.

Suggested review process:

• The designer(s) invite a small group of colleagues to perform the review. Ideally reviewers should have some background knowledge of the project but not be ‘too close’ to the topic of the review.
• Designer(s) prepare copies of any background documentation (such as code listings and diagrams).
• Hold a candid meeting where everyone is prepared to raise issues without embarrassment. Some mistakes may be revealed; some ‘dumb’ questions will be asked; that is okay.
• Each designer ‘walks through’ his/her product in detail. For example, for code this will usually involve looking at each statement individually. Each item should be explained so that everyone is clear about its purpose.
• Reviewers try to think of cases that are not covered and may cause a malfunction. E.g. “What if this value is negative?”, “Did you miss out a ‘default’ here?”, etc.
• Often, simply the act of trying to present a system to others causes the designer to be the first to spot problem cases.
• Take notes, so the problems will be remembered. Annotating code listings is typically convenient.
• Thank the reviewers.
• Revise the design according to the suggestions.
• Re-review, concentrating on the altered areas.

Illustration: a classic fault in C: the sort of thing which is difficult to spot:

if (x = 1) …^†

• Legal syntax
• May do what you expected …
  • … by chance …
  • … unless you make a point of testing the case when it shouldn't happen

^†In case this is unfamiliar, in C (at least) this is an assignment which will return a ‘TRUE’ (non-zero) value so will always do the statement following. Some programmers will write “if (1 == x) …” instead because this will force a syntax error if mistyped as “if (1 = x) …”.

Up to testing.

Back to tasks.

Forward to regression testing.