Five ‘Ws’ of Code Reviews
posted by Dave Dame
November 26, 2009
WHY do we do Code Reviews? There are three reasons for doing Code Reviews:
- Testing – Does the code do what it is supposed to do? Are there any errors in logic that can be corrected BEFORE we get to the test execution phase of the project?
- Qualitative Assessment – How well has the code been written?
- Management Supervision – Is there any code in the program that shouldn’t be there (i.e. fraudulent, malicious or unauthorized code)?
WHEN is a Code Review warranted? Research suggests that Code Reviews are up to 60% more effective at finding defects than the various forms of testing, making them a highly cost-effective way of improving the quality of the end product. Code reviews also present an ideal opportunity for knowledge transfer and skills development. More junior staff are given the chance to learn first-hand from more seasoned professionals, while the senior staff are presented with the opportunity to hone their coaching skills and pass on knowledge that might otherwise stay in their heads, preventing them from moving on to more challenging work. No wonder they are considered a “best practice!” While it is highly recommended that they be included in the overall quality assurance plan for every project, the decision to conduct code reviews rests with the senior management guiding the project work.
Here are some factors to consider when deciding if they are warranted:
- Complexity of the problem and/or algorithms the code must handle
- Familiarity of the YOUR ORGANIZATION and/or the developer with the development tools (e.g. language)
- Developer’s experience, both at YOUR ORGANIZATION and overall
- Overall test plan (i.e. is this the only form of testing being done on this particular piece of code)
- Risk of the project or work package the code is being developed in
- Supervisory/management oversight requirements (i.e. are we due for a “spot check”)
WHERE in the process do we do a Code Review? To satisfy the “Testing” and “Qualitative Assessment” reasons for the review, the review can be done before unit testing so that defects can be detected and corrected quickly without the effort of testing. To satisfy the “Management Supervision” requirement, the review must be done AFTER the developer has finished his or her testing, just prior to hand-off for the test execution phase. At this time, the code should be “frozen” from the developer’s perspective, such that he or she no longer has the ability to change it. Regardless of the reason for the code review, it needs to be completed and the results incorporated into the code BEFORE the Test Execution phase. Changing code after testing invalidates the testing, resulting in at least one more pass of testing.
WHO does the Code Review? The Senior Development Manager is ultimately responsible for ensuring that the final system matches the approved design and coding standard. It makes sense, then, for this individual to conduct the code review. However, there are a few things to keep in mind when deciding who will conduct the review.
- Does the reviewer have sufficient experience in the technology and/or development language? Does he or she know what constitutes good coding practice in the language/technology being used?
- Is he or she adequately familiar with YOUR ORGANIZATION’s environment and standards?
- Is he or she sufficiently knowledgeable about the application the code belongs to and the database/data model the application uses?
- Does she or he have a “big picture” view of project and what it is to deliver?
- Do they have a good grasp of the requirements, what role the specific code under review is playing in delivering to those requirements and how the code in question fits into the overall design?
WHAT do we look for in a Code Review? As per the WHY section above, code reviews serve three purposes: Testing, Qualitative Assessment, Supervision/Management Oversight. The testing component of the review focuses on WHAT the program does and the results it will produce. The following questions should be considered:
- Will it produce the results expected as per the requirements?
- Are there logic errors? (e.g. using AND instead of OR)
- Does the program handle all situations or are there “gaps” in logic that would cause the program to hang or loop continuously?
The Qualitative Assessment looks at HOW the code was written. It focuses on the performance and maintainability of the code by considering the questions:
- Does it follow coding standards?
- Is it efficient?
- Does it follow the “KISS” principle (Keep it simple, stupid) or is it difficult to understand?
- Is it formatted to allow for easy reading?
- Is it adequately commented, including “tombstone” or history of changes information?
- Does it follow the “one function per module” principle? Or is it a several-thousand long program that “does everything”? (really a design consideration but if design hasn’t gone to this level of detail, it should be looked at here)
- Does it employ readily understood features of the programming language while taking advantage of time-saving functions, etc?
- Has the code been written as simply as possible?
- Does it match design?
- Does it take full advantage of existing code? (also a design consideration)
- Is there inappropriate “hardcoding”?
- Is there indiscriminate use of “global” variables which will make maintenance and debugging difficult?
The third component of the Code Review is management oversight. This is focused on ensuring management knows exactly what code is being implemented into production and on protecting YOUR ORGANIZATION from fraud or unauthorized update. The following questions should be considered to satisfy this aspect of the code review:
- Is there extraneous code – code that is not necessary to satisfy the requirements?
- Does the code go beyond the bounds of authorized work?
- Are there more modules checked out or modified than there should be as per the design?
HOW do we do a Code Review? There are three types of code reviews: Code Reading, Walkthrough and Inspection. Code Reading is the easiest form of review to implement. As suggested by the name, it consists of the reviewers reading the code and looking for errors. They can also comment on the qualitative aspects of the code and, depending on who is doing the review, can provide the management supervision. Feedback is provided to the author either in a meeting or via written communication. Because the meeting is not strictly necessary, this approach can be effective from an elapsed time perspective. It is an especially effective technique to use when the reviewers and code authors are geographically dispersed. Walkthroughs are loosely defined and can be very informal. It is usually a working meeting moderated by the author, where the focus is on improving the technical quality of the code, so it can be used for both Testing and Quality Assessment purposes. Walkthroughs should be considered when code is particularly complex and/or the technology is new to YOUR ORGANIZATION. An Inspection is a very formal, facilitated discussion where a specially trained moderator leads a discussion of the code. Other participants in the inspection are one or more reviewers who are actively looking for errors in the code and the author of the code, who is there to provide further explanation, if needed. Inspections are only meant to address the “Testing” component of reviews. It does not include discussion on the quality of the code, or check for the existence of unauthorized code.
For this reason, and because it requires specialized training to do it effectively, this approach is not recommended for use at YOUR ORGANIZATION at this time. Regardless of the method used, the reviewers must make the following crystal clear to the developer/author of the code:
- What changes MUST be made to the code before it can be released for testing
- What critiques are suggestions, meaning the author can choose NOT to make the changes
- What further review process, if any, must be followed.