Sunday, October 22, 2006

Formal inspections

Formal inspections are an efficient and easy way to find errors in software. A formal inspection is a meeting where the code is reviewed. It is planned, moderated and must have a concrete follow-up. When doing an inspection, there are some principles to keep in mind:

  • the scope of an inspection is finding errors, not correcting them
  • an inspection is not a personnel evaluation, so it is better to keep management out of this

People that participate in the inspection are assigned roles. Here they are:
Moderator: keeps the inspection running at the required pace, not to be too slow or too fast to catch errors. Must be technically competent, not necessary an expert, but must understand the important details. He organizes the meeting, by providing checklists for the others, setting the date, preparing the work environment etc. Also, he must make sure that there is action following the inspection. He is not directly involved in the inspection, instead he makes sure that it runs as planned.
Author: The author of the software. In case the reviewers are not familiar with the project, he holds an introductory session, providing general knowledge of it. Besides that, he has the duty to explain parts of the code that are difficult to understand when asked and to explain things that are treated as errors and are actually acceptable.
Reviewer: The one that finds the errors. Must prepare beforehand by reading the materials that are supplied by the moderator. Must keep the focus on error finding, not error repairing.
Scribe: The person that records all the errors that are found.

It is recommended to never have less than 3 persons in an inspection(at least the roles reviewer, author and moderator should be played by different persons). More than 6 persons is a bad idea, because the group becomes hard to manage. Also, I repeat that management should stay out of this. The presence of a manger will make it seem like a evaluation with consequences for the author. Also, being a technical activity, the presence of a manager will add little if any value to the inspection.

The inspection has some predefined steps.

Planning - the author gives the code to the moderator. The moderator must select the reviewers and provide them with information that keeps them focused on the important aspects.
Overview - when the reviewers are not familiar with the project, the author should make an overview of the system. Yet, this step should be left out, because the overview creates a mindset and this mindset might hide errors. The code and design should speak for themselves and need no introduction.
Preparation - The reviewers take some time alone to read the materials handed over by the moderator. They also have a look at the code, to get an initial idea about what comes next. They should be assigned perspectives, e.g. one should keep an eye on security, another on data validation etc.
Meeting - The actual meeting. Its rhythm is crucial. A slow meeting will decrease concentration; a fast one will let errors slip away. Even if there is no widely accepted inspection rhythm, probably 150-200 non-blank, non comment lines of code per hour represent a good start. Yet it greatly depends on complexity, design goals, experience and many others.
Report - In less than 24 hours, the moderator creates a report with all the discovered errors and their importance, making it public. That ensures that there will be no forgotten errors.
Rework - The initial author or another is assign to fix the discovered errors.
Follow-up - The moderator must decide if another inspection is needed to re-test the program and look at the previous inspections and try to find patterns in the errors, finding causes for them.
Third-hour meeting - Even if solutions are not to be discussed in the inspection, some might feel the need to talk about them. In this case, the moderator should organize an informal meeting with this scope.

The main objective of an inspection is to find errors. The author should not feel threatened. So, the reviewers must be trained in making good comments ("I never seen something so stupid" is not a good comment). Their job is to find errors, not evaluating the programmer. They should also not suggest solutions, respecting the author's right to do this.
Mixing evaluation with formal inspection is a very bad idea. The author will try to hide the errors and he will probably succeed, minimizing the inspection’s primary objective.
Skipping steps or mixing roles will greatly affect quality. If you cannot measure the effect of a change, you should not do it. The formal inspection is a qualitative process, so decreasing its quality will render it useless.

No comments: