Archive for March 2009
Essentials of Code Review
Why Code Review?
We write code for two audiences. One audience is the hardware: the compiler, and the platform where the executable runs so that the user can operate it. The other audience, no less important, is the “next developer”: someone else (or even yourself a few months later) who has to modify the code to fix it or add new features.
The hardware’s reaction to the code is clear from testing. But the only way to find out if the code is easily maintainable is to have someone else—a knowledgeable developer—read the code. And the earlier, and more often, the better, since errors can be caught sooner and cheaper as well.
Additional benefits of code review: improving the code author’s coding (footnote 1) through the experience of guided correction, and finding and preventing systematic errors.
Code Reviewer and Code Author are Partners
The code reviewer must know the programming language well, be familiar with the technology (footnote 2) that the code implements, and be patient and effective in giving constructive criticism. The code author is the current maintainer of the code. Both must see themselves as working together to improve the code.
Review Comments, Reactions, Tracking to Closure
The code reviewer reads the code and makes comments. The code author responds by changing the code to improve it according to the comments s/he accepts. All comments must be recorded, electronically, both for tracking to closure, and for later review for systematic errors. If the author makes some changes during the review meeting, a “diff” of those changes may substitute for a list of those accepted comments.
First, Leave the Reviewer Alone to Read the Code
The reviewer first reads the code, alone, in a code browser that maps, and supports easy navigation of, the code’s structure. This method best simulates the “next developer’s” experience: the code author does not come packaged with the code—it must stand on its own.
Afterwards the reviewer meets with the author. If there are multiple reviewers, they may meet together with the author in a group review, but each reviewer must have first reviewed the code alone. A meeting should be in-person, but may be (less desirably) telephone or electronic, as long as it is interactive.
What Code Author and Code Reviewer Provide to each other
The author makes a labeled version of the codebase available to the reviewer. The requirements for the new, improved, or fixed code, and any required design, should also be known to the reviewer.
The reviewer reads the code for clarity = readability + understandability. Code is readable if it has the right balance of abstraction vs. detail at each level of the code. Code is understandable if the reviewer can, without help from the author, see what the code does, and why. The reviewer may use detailed checklists of common errors, and use the coding standard as reference, but review for clarity will lead to all the others.
When to Review Code: At Minimum, Before Check-in to a Shared Work Area
Code may be reviewed at any time. Here are some different times, and the benefits of review at those times:
- As code is written: Find errors earliest, prevent repetition by improving the author’s performance
- Before static analysis: Avoid analysis time on code that doesn’t belong
- After static analysis: Avoid review time finding errors that a tool could find
- Before check-in: Maintain quality of the codebase
- After check-in: Allows time for thorough review
- Before delivery: Catch errors before testing group
- Before debugging: Narrow down debugging steps; possibly find error without debugging
- After field deployment: Understand problem areas reported from the field; make improvement recommendations
The team, group, or department lead may decide when to review code according to his or her desired benefit. However, if time constraints limit code review to just one of these stages, the required stage is before check-in to a shared work area, to maintain the quality of the codebase that others will learn from and use.
Which Code to Review?
All new or changed code should be reviewed as above, at least before check-in to shared codebase. More complex or historically problematic code—more thoroughly. Take care also to review the new or changed code’s “nearest neighbors” to make sure the code still works with existing code it interfaces with.
Footnotes
- If there are multiple reviewers, the reviewers can also learn from each other’s review comments. But there are tradeoff costs of multiple-reviewer reviews: variation in quality of review, need for a strong moderator, and more time. Better for reviewers to learn from code review in the code author role, or in code walkthroughs specifically planned for cross-training.
- There may be exceptions in special-purpose code reviews such as performance or security reviews, where the reviewer must then be knowledgeable in those areas, but not necessarily in the technology.