Code Reviews

Code Reviews

• 1.
  1 Establishing Code Review Processesfor yourTeam Phil Denoncourt III Lead Consultant PJ Denoncourt & Associates, LLC Track: Enterprise Architecture
• 2.
  2 Wireless Access 1. Connectto the “Cambridge” network 2. Open a Browser 3. In Logon page use the code: af609
• 3.
  “Software testing alonehas limited effectiveness -- the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for integration testing. In contrast, the average effectiveness of design and code inspections are 55 and 60 percent. Case studies of review results have been impressive:In a software-maintenance organization, 55 percent of one-line maintenance changes were in error before code reviews were introduced. After reviews were introduced, only 2 percent of the changes were in error. When all changes were considered, 95 percent were correct the first time after reviews were introduced. Before reviews were introduced, under 20 percent were correct the first time. In a group of 11 programs developed by the same group of people, the first 5 were developed without reviews. The remaining 6 were developed with reviews. After all the programs were released to production, the first 5 had an average of 4.5 errors per 100 lines of code. The 6 that had been inspected had an average of only 0.82 errors per 100. Reviews cut the errors by over 80 percent. The Aetna Insurance Company found 82 percent of the errors in a program by using inspections and was able to decrease its development resources by 20 percent. IBM's 500,000 line Orbit project used 11 levels of inspections. It was delivered early and had only about 1 percent of the errors that would normally be expected. A study of an organization at AT&T with more than 200 people reported a 14 percent increase in productivity and a 90 percent decrease in defects after the organization introduced reviews. Jet Propulsion Laboratories estimates that it saves about $25,000 per inspection by finding and fixing defects at an early stage.” Steve McConnell, Code Complete Code Reviews
• 4.
  • Bugs shouldbe spotted earlier • Bugs are cheaper to fix Fewer Defects • More eyes on sustainable factoring • Proves the code is understandable More Maintainable Benefits - Quality
• 5.
  Junior • Learn practicedtechniques from experienced developers Senior • Gather more breadth of system architecture Architect • Opportunity to promote principles • Gives you more visibility to suspect areas of code. Benefits - Coaching
• 6.
  • Understand code,not developer is being reviewed Collaborative Group • Done independently or in group? • How do you to notate results? • How to you verify corrections were made? Process • Set expectations by having coding standards documented • Code ReviewChecklist Standards Prequisites
• 7.
  Code is written Compiles Static Code Analysis Automated Tests Succeed Developer reviews code Code Review Corrections aremade Code is committed to central repository When to do reviews
• 8.
  • Promotes betterlearning • People likely to come unprepared Code Review Meeting • Not interrupting developer’s schedules • Easy to defer Offline / Async • Suggestions are emailed to a moderator before the review • Moderator goes through the list High Impact Inspection Methodology
• 9.
  • More bugswere not found in groups larger than 5 Small Group • Keep review to 200-400 lines of code Small Review Scope • After 60 minutes, fewer bugs were found Timeboxed effort Suggestions
• 10.
  • Shouldn’t beforced to have code reviewed that isn’t ready. Developer submits code • Managers don’t have much to contribute. • Encourages unnecessary defensiveness NonTechnical Attendees • Encourages a through review • Sets expectations for developersChecklists Suggestions
• 11.
  • Need toencourage egoless reviews • Continue to reinforce that code is being reviewedTone • Encourage atmosphere of proposing solutions. Not just pointing out problems. Fix it • Make sure non-traditional assets are reviewed • Database Schemas • Supporting Documentation • Installers Broad Suggestions
• 12.
  • Consider havingone person responsible for a single area • Security • Memory Usage • Performance Subject Matter Experts • If bug was not found in code review, determine root cause Feedback • Code Reviews are generally not the place to design the solution. • Keep changes focused on code. Revision Suggestions
• 13.
  • Code isre-reviewed until it passes • If it continues to need work, address the underlying issue Repeat • Best if testing staff can read code • A way of communicating how system is put together. Involve Testing Staff • Best to have a system that integrates with build systemAutomate Suggestions
• 14.
  Immediate code reviews •Albeit reviewing isn’t the focus Reviews tend to be non-objective Metrics are hard to gather Pair Programming
• 15.
  Threading Static variables areprotected Appropriate objects are threadsafe Locks are released in the correct order Critical Resources Handles are freed at the earliest point Handles are validated before usage Instrumentation Code logs important events Logging is not excessive Security Authorization checks are done Function Parameters are validated  Convention  Adequately Documented  Appropriate Headers  Error Handling  Assumptions in functions are asserted  Exceptions are documented  Resources are properly freed in exception situations  It is possible for the exception to occur  Loops  It will always be finite  Ending conditions are accurate  Tests  Coverage is sufficient  Purpose of test is understandable Review Checklist
• 16.
  • Issues foundper review • Expect a spike initially • As your team gets better at finding defects • Expect this to go downward Code Review Failure Rate • Number of big issues found in QA • Expect this to go down rapidly Severe QA Bugs • Number of lines committed as a result of code review • Expect this to be high initially, and have a downwards trend. Number of lines changed per review Measuring
• 17.
  • Best ifintegrates with defect system • As well as SCCIntegrate • Tracks concerns in code • Resolution of the concernsFeatures • Wide variety of vendors • Open SourceResearch Software
• 18.
  • Team won’tlearn from each other Minimal Learning • You won’t be present, so you don’t have insight • Seed code with known issues to measure Questionable Detail • Won’t be taking up team’s time • Project headcount is minimized Lower Commitment Outsourcing
• 19.
  • Takes timeto review existing code • Costs ½ person day to review 300 linesCommitment • Big shift from committing any code, to only reviewed code • Some devs are sensitive to feedback Culture • Many developers have had bad experiences • Some research suggests reviews aren’t worthwhile Reputation Struggles
• 20.
  Code Reviews improvequality Code Reviews improve team Reviews require planning and resources Wrapping up
• 22.
  22 ThankYou Sponsors ! {CodeRight}
• 23.
  Help make abetter event - Complete a Survey! http://ArchitectFactory.com/Survey.aspx