Software benefits tremendously from code review. Establishing an effective review process requires management commitment and follow-through, but it is well worth the effort. In this article, Ed Hartnett suggests some best practices to follow in your code review process. This article originally appeared in the Tech Support section of Volume 3, Issue 3 of [email protected].
It would be like researching and drafting this article, only to submit it without checking the spelling. And yet many organizations will spend time and money to support the production of software without a formal software review process in place to make sure they're getting something they can use.
It has been proven again and again: Software benefits tremendously from code review. The key for organizations is to embrace the process. Establishing an effective review process requires management commitment and follow-through, but it is well worth the effort.
Code reviews are primarily aimed at finding defects before testing. It is far more efficient to catch mistakes early. Once software reaches the testing stage or, even worse, the customer, the price to fix mistakes increases dramatically. Reviews won't catch every bug, but they can eliminate a significant proportion of them and save a lot of money. This alone justifies the cost of code review, but there are other benefits as well. Reviewed code is easier for multiple programmers to work on and to maintain. Reviewed code raises visibility and standards, encouraging best practices and discouraging bad habits. And reviewed code can improve morale as good code brings praise and recognition to the team.
Do it Often
Code needs to be reviewed if it might have mistakes in it-that means all code needs review. A team of five or six programmers, for example, should go through three or four code reviews a week. Because only about half the team needs to be at each review, that means about twice a week for each team member. This is by no means an unreasonable burden.
Do It With Diplomacy
The most important rule for a good code review is to keep things positive. Negative and destructive criticism will make reviews an ordeal. Positive and constructive criticism will make them a pleasure. Unfortunately, some software engineers lack diplomatic skills.
Yet the project manager, who should have better diplomatic skills, isn't always the best person to oversee the code review because many engineers won't point out each other's mistakes in front of an outsider. Unless the project manager is a programmer, too, it may be wiser to pick a respected senior programmer to run the code reviews. This individual is responsible for ensuring politeness and sensitivity. A good sense of humor and a genuine desire to develop the team's long-term capabilities are helpful.
Do It On the Record
Record-keeping is essential to code review. The team should methodically document what code has been reviewed and what defects have been found. Each defect should be recorded and categorized as "major" or "minor." Any defect that could cause a noticeable bug to the end user is a major defect. Any defect invisible to the user is minor.
It is the project manager's responsibility to ensure that any recorded defects are cleaned up by the programmers. Photocopy the records of each review, giving one copy to the programmers who will have to fix the defects. Then follow up soon after to ensure that defects are being fixed in a timely manner.
Do It Now
Within a month of establishing a good review process, bugs in the code will start to decrease. Less time will be spent debugging, allowing programmers to concentrate on producing better code from the outset. Some project managers think they have no time to spare on asking their team to implement and follow a formal code review process. This is a perfect example of the phrase: "Never time to do it right, but always time to do it over."
Experienced software project managers know that it always saves time to do code right the first time. Not only will your project finish on time, you will get far fewer of those gut-wrenching calls from dissatisfied customers and management, complaining of broken code and unmet promises.