5.4. Code ReviewsA code review is a special kind of inspection in which the team examines a sample of code and fixes any defects in it. In a code review, a defect is a block of code that does not properly implement its requirements, that does not function as the programmer intended, or that is not incorrect but could be improved (for example, it could be made more readable or its performance could be improved). In addition to helping teams find and fix bugs, code reviews are useful for both cross-training programmers on the code being reviewed and for helping junior developers learn new programming techniques. 5.4.1. Select the Code SampleThe first task in a code review is to select the sample of code to be inspected. It's impossible to review every line of code, so the programmers need to be selective about which portion of the code gets reviewed. Many teams have found that it takes about two hours to review 400 lines of code (in a high-level language such as Java), although this estimate differs dramatically from team to team and depends on the complexity of the code being reviewed. At that rate, there is no way a team could review all of the code for a software project. Nor would the team want toin any program, there is a good deal of uninteresting code that looks very similar to the code already developed in previous applications, which has a lower risk of containing as many defects. The purpose of any inspection is to find and repair defects. Since a relatively small portion of the code will be reviewed, it's important to review the code that is most likely to have defects. This will generally be the most complex, tricky, or involved code. There are a few useful rules of thumb that are helpful:
It is important to select a sample of code that an inspection team can review in about two hours. The project manager should try to keep the meeting to two hours or less, to avoid "meeting fatigue." 5.4.2. Hold the Review SessionThe team selection and preparation in a code review are similar to any other kind of inspection. An inspection team of 3 to 10 people must be selected. Each of these people must be technically capable of reading and understanding the code being reviewed. Before the meeting, the moderator distributes the code sample to each inspector, who does individual preparation exactly as in the inspection. The main difference between a code review and any other kind of inspection is in the review session. While the code review session is similar to the inspection meeting (see "Page-by-page review" above), there are a few important differences. In addition to the moderator, there is a code reader who reads the code aloud during the meeting. The code reader can also be one of the inspectors; the only requirement is that the reader must have enough technical expertise to understand the code. The purpose of the reader is simply to keep the team's place during the inspection; the team should have already read the code and identified defects during their preparation. Since code is usually organized in logical units or blocks, it is more useful for a reader to go through those, rather than having the moderator go through the document page by page. The reader starts at the beginning of the code sample and announces the first block or logical unit. She does not literally read the commands in the code; she simply gives a brief description (about one sentence) of the purpose of that block. If anyone (including the reader) does not understand what the code does or disagrees with the interpretation, the author of the code explains what it is the code is supposed to accomplish. Sometimes the team can suggest a better, more self-explanatory way to accomplish the same thing; often it is simply a matter of explaining the purpose of the code to the person who raised the issue. If any inspectors found a defect in that block of code, the issue is raised and the team either comes up with a fix on the spot or tags it as an open issue for the programmer to fix later. The moderator then updates the inspection log, and the inspection continues until the reader completes the code sample being inspected. Another important difference between code reviews and document inspections is that the code review is much more focused on detecting defects, and less on fixing them. This is because many defects in documents can be corrected with one or two sentences, or with a change in wording. Defects in the code can be much more involved, and there are often many ways that they could be fixed. The discussion of each defect is longer in a code review than it is during an inspection, and there are usually many open issues at the end of the code review. In the code review, the moderator needs to be especially careful not to let the meeting turn into a problem-solving session. Programmers love to solve problems. It's easy for them to get caught up in a small detail and turn the meeting into an analysis of a minute problem that covers just a few lines of code. However, long discussions like this will prevent significant amounts of code from being reviewed. That's not to say that these discussions are not valuablethey just don't belong in the code review meeting. If a discussion looks like it will take more than three minutes or so, the moderator should stop the discussion and add it to the inspection log as an open issue. There should be few open issues, however, because most code defects should be straightforward to describe and document once they have been identified. There are effective ways to modify the code during the review. Many inspectors have found that it is very helpful to refactor the code during the review. By applying refactorings on the spot, the team can make the code much more readable and identify additional defects. (See Chapter 7 for more information on refactoring.) After the inspection meeting, the code author performs the rework and closes the open issues, and the moderator follows up with each of the inspectors and gains their approval. Instead of getting formal sign-off with physical signatures, it is usually sufficient to indicate the approval in the log comments when the changes are committed to the version control system (see Chapter 7 for more information on version control). There are several additional benefits for the code review. One is that people learn how their teammates think about the code. A good way to encourage this is to switch off code readers in each review, so every team member gets a chance to be a reader. Reading code aloud and explaining it helps programmers think through problems. Every programmer should be able to explain his ideas well; discussing code during a code review is good practice for that. Another benefit is that people who know that their code may be inspected tend to write more maintainable software. It's very common for programmers to not include comments or to write very terse, confusing code when they know that they are the only people who will ever read it. But if a programmer knows that someone else will be looking at it, he may put a lot of effort into making it readable. This can have enormous savings in maintenance efforts down the road. 5.4.3. Code Review ChecklistThe following attributes should be verified during a code review :
|