Communications of the ACM

A review of code review do's and don'ts.

Olivier Scalbert

Hello,

I have read your interesting article on code review.
Here are some points (based on real life) I would like to add ...

Scope and timings
Doing a code review for a nuclear plant or a medical device context is not the same thing as doing a code review for Flash animation actionscript code !
So scope and timing should be very well defined and understood by everyone.

Coding rules and style checking tools
These tools can do a lot of work for you, automatically.
They can be seen as a prerequisite before a formal code review.
After code review, new rules can be added to such tools.

Unit tests
It is much more comfortable to do a code review for a code fully covered by unit tests.
If you need to understand a very complex piece of code it is perhaps easier to try to play with this code by writing some unit tests.
If you can not write unit tests for this code, I have serious doubts you can do a code review on it !
Code coverage tool can be your friend there.
Also unit tests can be seen a assets. They can be run again later automatically (which is not the case of a code review).
They can help to clarify missing specs.
Having unit tests prove at least that the code under review can be compiled ! Which is far from obvious with only code review.

Missing specs
Sometimes (read often!) specs are missing or not enough formal. Having a domain expert can help.
Specs should then be fixed.

Fix after code review
Treat the code review results as a problem report, so it can be prioritized, assigned, fixed, validated and closed.
The fix should be done in separate branch to help code reviewers.

Human factors
Code review is not a trial. Code author is not guilty. Code reviews without a little of psychology could lead to disasters ... Egos ...
There are several way to avoid this:
- start code review process early in the project (cultural aspect and avoid black box or even black hole);
- do a little of peer programming which can be seen as a small real-time code review.

Best regards,

Olivier Scalbert