Code Review Mind Map

I’m working on implementing code reviews for a client, and dug up this old mind map of what I consider important.

Let me know if you have any suggestions on improving it.

Code Review Mind Map

Click to expand

  • How to do a code review
    • Select the code to review
      • Ensure all developers eventually have some of their code reviewed
      • Indicators of good code to review
        • Code that already has problems
          • Bugs
          • Performance issues
        • Crucial code
        • Complicated code
          • High cyclomatic complexity
          • Long classes and methods
          • Large amount of class coupling
        • Code that could be re-used in a library
      • Review limits
        • Review approximately 300 lines of code
        • Select one class, or a few related classes
        • Group review meeting is one hour long (or less, if done before one hour)
    • Who should attend reviews?
      • Author of code being reviewed
      • Moderator
        • Not the code author
        • Ensures review stays on track
        • Writes down changes to make
      • A mix of junior and senior developers (to help spread knowledge)
    • Author goes through the code before sending to reviewers
      • Add a short description of what the code does
      • Also gives them a chance to personally review the code once more
    • Send code to reviewers before the group review
      • Two or three days before the group review
      • Do individual, quick pre-reviews before the group review
        • Make notes for the group review
    • During the group review
      • Go through [roughly] 10 lines, or one method, at a time
        • If printing source code, include line numbers
      • No personal attacks on the author are allowed
      • Don’t make changes based on other programmer’s personal preferences
      • Moderator records changes the team agrees on
        • To the source code
        • To the coding standards (if applicable)
        • To the continuous integration process (automated checks)
    • After the group review
      • Ensure changes are made and checked in
  • Benefits you’ll receive
    • Defects are found before going to QA (or production)
    • Generally, more defects are found here, than in QA
    • Code reviews are more likely to find “edge case” problems
    • More developers are familiar with the code
    • Developers share “best practices” with each other during the review
  • Potential problems
    • Some developers use reviews to show their “superiority” over other developers
    • Reviewers lose focus and start discussing larger, or unrelated, issues
    • Reviewers state personal preferences as “the best way”
    • Suggested changes are never made
    • Some reviewers don’t contribute any suggestions
  • Potential problem solutions
    • Have phrases to stop discussion problems
      • “Offline”
      • “Personal Preference”
  • Modify the process
    • If you find recurring problems, can you add automated checks for them?
    • Would you do better with fewer, or more, reviewers in each group review?
    • Do the individual pre-reviews lead to better group reviews?
  • What NOT to do
    • Use reviews to evaluate developers
      • Decreases desire to participate
      • Less participation = fewer bugs found
  • Types of defects
    • Bugs/errors
    • Poorly named classes/properties/methods/variables
    • Duplicated code
    • Code that will cause performance problems
    • Code that doesn’t match coding standards