[Lab 1] [Lab 2] [Lab 3] [Lab 4] [Lab 5] [Lab 6] [Lab 7] [Lab 8] [Lab 9] [Lab 10] [Lab 11]

COMP2100
Lab 7: Code Review

Summary

In small groups you will carry out two tasks:

• Review your defect logs and produce a code review checklist; and

• Work through a piece of code provided to you, and try to locate as many defects as possible.

Aims

• Prepare for working with the enhanced PSP for the rest of the semester.

• Gain experience of the code review process.

• Gain an appreciation of how difficult it is to read someone else's code.

• Gain some experience of teamwork.

Preparation

Read through the notes for Lecture 18 and these instructions before you come to the class.

Bring your lab notebook with all the defect logs from your PSP homework exercises so far. Take a few minutes to read through the defect logs and make notes on anything that stands out.

Print out a copy of words.e, the code you will be reviewing. Read carefully through it and make notes on any defects you find, both in style and correctness, and anything you don't understand.

---

For this lab you need to work in groups of five or six. You may want to have one group member logged in so that you can check interfaces of library classes or other classes used by the class you will be reviewing in Exercise 2, but that's the only possible use you will have for a computer. This class is more about thinking and communicating.

Spend about an hour on each exercise. Don't worry if you don't finish Exercise 2.

1. Defect logs

These instructions on how to prepare a code review check list are adapted from Chapter 14 of Introduction to the PSP.

The idea is to review your defect data to see which types have caused the most problems. Then you create a check list for code reviews so that you remember to check for the errors you know you have made in the past.

In the future you will follow this process alone, working with just your own defect data to prepare a completely individual code review check list. But at this initial stage, and with only a relatively small number of defects to work with, I think it's better if you combine your defect data and prepare one code review check list for the whole group. As you continue to follow the PSP using this check list you can refine the check list adding items to cover defects that you miss and removing items looking for mistakes that you personally don't make.

1. Make a table of all the defects that all group members have found in all the PSP programs you have written so far, broken down by type. (Next time you do this it will be necessary to break this down also by the phase in which the defect was removed: you'll be looking for those defects that you miss in your code review phase, but since you haven't been doing a code review so far, we need to look at all defects.)

   Here is an example, taken from my PSP homework so far. I have written 6 programs: Lab 2 and Homeworks 3-7. I have a total of 23 defects in all.

   Type	Number
   10	1
   20	4
   30
   40	4
   50	3
   60
   70	1
   80	10
   90
   100
   Total	23

2. Rank the types in descending order of the number of defects found. In my example you can see that type 80 function defects account for almost half of my defects. So these are the ones I need to concentrate on. For your group it will likely be different.

3. For the top few defect types, examine the defect logs in more detail to see what specific types of problems caused the most trouble. You might want to compile a list, perhaps trying to further break them down into groups that are more specific than the simple classification we've been using.

4. For defects resulting from these most important problems, work out in your group what steps you would need to take in a code review to find them.

   For example, suppose you make a lot of type 40 assignment defects that turn out to be caused by forgetting to declare local variables. Then you might find it very helpful to check every variable you use in your code to make sure that it has been properly declared.

   Or if (like me) you often forget to increment a loop counter, it could save a lot of time to check every loop to make sure that the counter has been incremented. (Or perhaps you could add to the coding standard that every loop must have a loop variant.)

5. Add items to your code review check list to make sure that you follow these steps every time you review your own code.

6. Think also about the general code review instructions and advice from Lecture 18, and add items to the check list so that you make sure you do a thorough check of your code. Your aim is to remove all defects in the Code Review phase, so that there are none left in Compile and Test. This is an extremely difficult, if not impossible, goal to attain, but you want to get as close as possible.

   Some items you might want on your check list:

   • Check that the code is complete, that everything in the design has been coded.

   • Check every line of code for correct syntax.

   • Check that parentheses balance.

   • Check that the code fulfils the requirements. (This is much harder than criticising style.)

   • Check for robustness: will the program crash if the input doesn't conform to expectations?

   • Does the code conform to the coding standard? (This is set out in the Eiffel Coding Standard page, which is based on Chapter 26 of Object-Oriented Software Construction.) The next few items here address this at least in part:

   • Are identifiers appropriate? Function and attribute names should be nouns, procedure names should be imperative verbs and boolean function or attribute names should be true/false statements or adjectives.

   • Check for unhelpful or misleading identifiers.

   • Check for inappropriate uses of manifest constants.

   • Check for missing, useless, incorrect or misleading header comments.

   • Check for too many, too few, confusing or incorrect non-header comments.

   • Check for missing, incorrect or useless require or ensure clauses.

   • Check for missing, incorrect or useless loop invariants and variants.

   • Check for incorrect indentation.

   • Check for routine bodies which are too long or too deeply indented.

   • Check for confusing structure.

After you have used the checklist a few times, review the new defect data the same way (counting only those defects found in Compile and Test, that is after the Code Review phase). If the checklist was effective at finding these defects, you could start looking at the new most frequent type of defect and add an item to address that. If the checklist was not effective, you may need to modify it so that it works better at removing your most common defects. If some of the items in your check list are not finding any defects, then you might want to consider merging them to save time, or even deleting them. Perhaps you no longer make those mistakes.

How to use a code review checklist

Your finished code review checklist should look something like this:

Description	Program #7	To Date	To Date %
Check that each variable used is correctly declared	X	3	20.0
Check that everything in the design has been coded	1	5	33.3
Check that all parentheses balance	X
Check that all loop counters are incremented	2	7	46.7
...

As you work through the check list, put an `X' if you didn't find anything, or the number of defects found if you did. Keep track of the number of defects found by each item in the checklist; the To Date number should be the number found on this sheet plus the To Date number from the previous one. This will help you to see which items on the checklist are most effective.

---

2. Team code review

First assign roles to some of the team members:

• The moderator, who is the chair of the meeting;

• The reader, who walks the team through the inspection process, reading the code aloud;

• The recorder, who records all problems found in the code.

Now work systematically through the code, with the reader reading each line of code aloud slowly, and the team members discussing any potential problems. The recorder should record any defects in his or her engineer's notebook.

The class you have been given (class WORDS) is an attempted solution to this week's homework problem of printing out a number in words. It has at least fifteen defects, some serious, some unimportant. Your task is to find them all.

Don't forget to use your new code review checklist.

Recording the results

For each defect found, the recorder should write:

1. How severe the group believes it to be.

2. What type of defect it is (e.g. style or correctness).

3. Which line or lines it occurs on.

4. The exact nature of the defect.

5. Any other comments.

Remember that it is not your task to fix the defects you find, or even to suggest how to fix them. You are only required to locate and describe the defects as clearly as possible.

Don't worry about precise categories for severity or type: just use ordinary descriptive language for now.

Send it to me

Type up your code review report and mail it to comp2100@iwaki.anu.edu.au. This can serve two purposes:

• to show how smart your group is, and

• to show me how bad my code is.

Go ahead, make my day...

[Lab 1] [Lab 2] [Lab 3] [Lab 4] [Lab 5] [Lab 6] [Lab 7] [Lab 8] [Lab 9] [Lab 10] [Lab 11]