Wednesday, 17 December 2008

Peer code reviews

Peer code reviews can have a devastating effect on software bugs, greatly reducing their number and scope. However, not all code reviews are the same and some are more effective than others. There is no one-shoe-fits-all guide to code reviewing, but there are ways of getting the right code review process for your situation.
At the bottom end of the effectiveness scale is a peer looking at a screen resplendent with the review code. This is better than no review, but still has drawbacks:
  • The review procedure will vary according to the reviewer and their state of mind, some reviewers will be more picky about certain issues or even miss things others would find easily.
  • Any problems found and commented on in the review may not be fixed - due to forgetfulness, other 'more important' pressing issues or thinking that they are trivial (to the author under review). Anyhow, the problems are probably not followed up afterwards to make sure they're resolved.
  • A lack of reviewing guidelines can lead to some comments being taken a bit too personally.
  • Most people when reviewing will only look through the code once from top to bottom, and pick out any issues of any type as they go along (syntax issues, loop counting and memory allocations for example). This is much less efficient than looking for one type of issue at a time, and scanning the code once for each type - if you don't believe me, try it!
In order to mitigate missing as many bugs as possible during a code review, the generally agreed best practice contains the following guidelines:
  1. Use a checklist, some examples can be found in the links below.
  2. Review for one type of issue at a time.
  3. Improve the checklist to look for bugs which slip through the reviews as time goes on.
  4. use code printouts on which you can scribble comments - I've not come across any software tool that is as effective as paper.
  5. Log any issues not resolved during the review process, for example in the form of a bug database entry - even it does seem trivial now, experience has taught me that it might be the the most important thing in the world next month! It should also be possible to make the reviewer aware of any previous issues found with the code under review.
  6. Have your process geared towards reviewing little and often.
  7. Be balanced - make sure to note what the code does successfully, as well as it's issues and offer praise where it is due.
More thorough details and further reading can be found here:

No comments: