Sign In

Communications of the ACM

Viewpoints

Kode Reviews 101


comical code review

Illustration by Andrew Stellman

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

The full text of this article is premium content


Comments


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


Displaying 1 comment

Log in to Read the Full Article

Sign In

Sign in using your ACM Web Account username and password to access premium content if you are an ACM member, Communications subscriber or Digital Library subscriber.

Need Access?

Please select one of the options below for access to premium content and features.

Create a Web Account

If you are already an ACM member, Communications subscriber, or Digital Library subscriber, please set up a web account to access premium content on this site.

Join the ACM

Become a member to take full advantage of ACM's outstanding computing information resources, networking opportunities, and other benefits.
  

Subscribe to Communications of the ACM Magazine

Get full access to 50+ years of CACM content and receive the print version of the magazine monthly.

Purchase the Article

Non-members can purchase this article or a copy of the magazine in which it appears.