Ch 9: Reviews

1. Ch 9: Reviews • Humphrey proposes that personal reviews will result in improved quality. • Now that we have a defined process and some real data, we can augment our process with his suggestion and measure it and empirically determine if it makes a difference. • This will demonstrate how process optimization works using the PSP.

2. Personal versus Team reviews • Team reviews: • FTR • Inspections • Walkthroughs • Personal reviews • “Review” is short for “personal review”. • You examine your own work in a structured manner with the intent of finding defects.

3. Evidence of Review Effectiveness • Intuitive or anecdotal; most of us know that many of our errors are due to sloppiness or rushed work. Many of these errors would have been found had we double checked our work. • Anecdote about slow turnaround time and fast compilers as a bad thing. Optimal time = 20 min.

4. Evidence of Review Effectiveness - cont. • Data show that it is much more efficient to find defects in reviews than in testing • in unit test, typically only about 2 to 4 defects are found per hour • code reviews typically find about 10 defects per hour • experienced reviewers can find 70% or more of the defects in a product • unit test rarely exceeds a 50% yield • PSP data show that reviews find 2 to 5 times as many defects per hour as unit test

5. Defect Removal Comparisons1997 SEI Study

6. Evidence of Review Effectiveness - cont. • After unit test, defect removal becomes much more expensive • in integration and system test it takes 10 to 40 programmer hours to find and fix each defect • inspections typically take less than an hour per defect • Some inspection data • O’Neil: 80-90% yield at 25 minutes/defect • Philips: 15 minutes/defect versus 8 hours in test

7. Evidence of Review Effectiveness - cont. • The reason to eliminate defects as early as possible is because • every review, inspection, compile, and test finds only a fraction of the defects • thus, the cleaner the code entering a phase, the cleaner it will be on exit. • Early defect removal saves time and money! • defects cost more to find and fix later in the development process • defects cost much more to find and fix after product release

8. Rationale: reviews are direct, testing is indirect p236 • Testing produces symptoms • Debugging (or defect identification more specifically) is time you spend to get from symptoms to defect. Trivial defects can produce very bizarre symptoms. (Elicit anecdotes). • During code reviews we carry a mental construct of the intended solution and it’s easier to spot deviations.

9. Why Reviews are Efficient - 1 • In Testing • you start with a problem • then you must find the bug • next, you devise a fix • finally, you implement and test the fix • With reviews and inspections • you see the defect • then you devise a fix • finally, you implement and review the fix

10. Why Reviews are Efficient - 2 • In Testing, the system produces an unusual result, then you must • detect that it was unusual • figure out what the test program was doing • find where it was in your program • figure out what defect could cause such behavior • You are searching for the unplanned and unexpected. • This can take a lot of time.

11. Why Reviews are Efficient - 3 • With reviews and inspections • you follow your own logic • when you find a defect, you know exactly where you are • you know what the program should do and did not do • you thus know why this is a defect • you are therefore in a better position to devise a complete and effective fix

12. Motivation for code reviews • May seem solitary, boring, slow, laborious. • Try it and convince yourself with your own data. • If you don’t strive to produce defect-free work, you probably never will. • If you have a habit of sloppy work, you will be unable to produce quality work on the “important” project.

13. The PSP Review Strategy • The PSP objective is to produce the highest possible program quality at every process phase • The review strategy to achieve this is to • gather data on your reviews • study these data • decide what works best for you • adjust your process accordingly • This is a continuous learning process using data on your own work.

14. Review Principles • 1. Have defined review goals • The PSP goal is to find every defect before the first compile or test. • 2. Follow a defined review process with • entry and exit criteria • a review structure • guidelines, checklists, and standards • 3. Measure (track time and defects) and improve your review process

15. Skip Chapter 9.6 and 9.7 • CSc 409 does not use design reviews.

16. Other process improvements • Other suggestions you might try on your own: • write test cases first • hand trace (aka “desk check”) • single step through code, see if it matches hand trace • document first for readability standards • asserts and pre/post conditions • peer walkthroughs

17. The heart of the review: checklists • PSQ code check is not just “double-checking” -it is a formal defined process. • Checklists help when the number of items to be checked exceeds our short term memory capacity. Increases probability of covering all items.

18. Checklists • When performing precise tasks, it is difficult to do more than one thing well at a time. • The checklist defines the review steps in the order suggested for performing them. • By checking off each item, you are more likely to perform it properly. • Establish a personal checklist that is customized to your defect experience.

19. Using Checklists • Follow the order on the checklist. • Do one item at a time. • Do each item completely. • Check each item when complete. • Do them the same way every time. • Complete the entire checklist for each unit/module separately.

20. Using Checklists (cont.) • Do we include coding standards as part of checklist? • No, we have an automated style checker. • Yes, the style checker doesn't catch all coding standard violations. • Consider separating readability standards from semantic ones • indentation, layout, etc, vs if (constant == variable) • Consider separate defect category for readability since that affects maintenance

21. Using Checklists (cont.) • Take frequent breaks. The work is tedious, breaks help you stay alert. • Don’t rush (don’t confuse speed with progress) • Suggest limit of 200 LOC/hr. • Consider your Review Objectives /motivations.

22. Code review before or after compile? • Which do you think is best? • Why? • (Pro/Con arguments on pp197-199)

23. Building a personal checklist • Based on defect log of your actual defects • Create a pareto distribution (sorted by frequency) • Build a checklist to focus on most prevalent defects first • Review periodically • add items that you are missing in reviews • retain items that you are finding.