Code Review Guidelines for COMP 477

Code reviews are an essential practice in software development, serving multiple purposes: improving code quality, sharing knowledge, catching bugs early, and ensuring consistency across a project. In this course, we'll conduct in-person code reviews to foster collaborative learning and develop critical evaluation skills. This guide covers what both presenters and reviewers should focus on during the review process.

For the Code Presenter

Before the Review

  • Prepare Your Code: Ensure your code compiles and runs correctly. Fix obvious bugs and clean up debug code. The review should focus on design and logic, not basic syntax errors.
  • Write Clear Commit Messages: Your commit history should tell the story of your changes. Each commit should represent a logical unit of work with a descriptive message.
  • Self-Review First: Go through your code as if you're reviewing someone else's work. Look for areas that might be confusing, inefficient, or inconsistent with project conventions.
  • Prepare Context: Be ready to explain the problem you're solving, your approach, any trade-offs you considered, and how you tested your code. Think about alternative solutions you rejected and why.

During the Review

  • Start with the Big Picture: Begin by explaining the overall goal and your high-level approach. Help reviewers understand the context before diving into implementation details.
  • Walk Through Key Decisions: Highlight important design choices, especially where you considered multiple approaches. Explain why you chose your current solution.
  • Point Out Areas of Concern: Be honest about parts of your code you're uncertain about or areas where you'd appreciate specific feedback.
  • Discuss Testing: Describe your strategy and approach to testing. Provide a candid assessment of its weaknesses and limitations.
  • Be Open to Feedback: Listen actively and avoid becoming defensive. Remember that critiques of your code are not critiques of you personally. Ask clarifying questions if feedback isn't clear.
  • Take Notes: Write down suggestions and action items. You won't remember everything discussed, especially in longer reviews.
Example Opening: "I'm implementing a concurrent hash table for our key-value store. I chose to use reader-writer locks for each bucket to balance performance with safety. I'm particularly interested in feedback on my approach to handling resize operations and whether there are better alternatives to the locking strategy I've used."

For Code Reviewers

Mindset and Approach

  • Be Constructive: The goal is to improve the code and help the author learn, not to demonstrate your superiority. Frame feedback in terms of improvement opportunities.
  • Focus on the Code, Not the Person: Say "this function could be more efficient" rather than "you wrote an inefficient function."
  • Ask Questions: When something seems wrong, start by asking "Can you help me understand why..." rather than making definitive statements.
  • Acknowledge Good Work: Point out clever solutions, clean code, and good design decisions. Positive feedback is valuable for learning and morale.

What to Look For

1. Correctness and Logic

  • Does the code actually solve the intended problem?
  • Are there edge cases that aren't handled?
  • Are there potential race conditions or thread safety issues?
  • Does error handling cover all failure modes?

2. Code Quality and Style

  • Is the code readable and well-organized?
  • Are variable and function names descriptive?
  • Is the code consistent with project conventions?
  • Are functions and modules appropriately sized and focused?

3. Performance and Efficiency

  • Are there obvious performance bottlenecks?
  • Is memory being used efficiently?
  • Could algorithms or data structures be improved?
  • Are system resources (files, connections) properly managed?

4. Design and Architecture

  • Does the solution fit well with the existing codebase?
  • Are abstractions appropriate and well-defined?
  • Is the code maintainable and extensible?
  • Are dependencies and coupling reasonable?

5. Testing and Documentation

  • Are there adequate tests for the new functionality?
  • Is complex logic documented appropriately?
  • Are public interfaces well-documented?
  • Do comments explain the "why" rather than just the "what"?

Providing Effective Feedback

  • Be Specific: Instead of "this is confusing," say "this variable name doesn't clearly indicate what it stores."
  • Suggest Solutions: When pointing out problems, offer concrete suggestions for improvement when possible.
  • Prioritize Issues: Distinguish between critical bugs, important design issues, and minor style preferences. Focus the discussion on the most important items first.
  • Explain Your Reasoning: When suggesting changes, explain why the change would be beneficial. Help the author understand the principles behind your feedback.
Good Feedback Example: "I notice this loop allocates a new vector on each iteration. Since the size is known in advance, you could allocate once with the right capacity outside the loop. This would reduce garbage collection pressure and improve performance for large datasets."

Best Practices for Everyone

  1. Keep Reviews Focused: Limit review sessions to manageable chunks of code (typically 200-400 lines). Larger reviews become less effective and more prone to missing issues.
  2. Review Regularly: Frequent, smaller reviews are more effective than infrequent, large reviews. They're easier to digest and feedback is more actionable.
  3. Use a Checklist: Develop a mental (or written) checklist of common issues to look for. This ensures consistency and completeness in reviews.
  4. Follow Up: After feedback is given, verify that important issues are addressed in subsequent iterations.
  5. Learn from Each Review: Both presenters and reviewers should reflect on lessons learned and apply them to future work. Consider keeping a personal log of common issues and good practices discovered.
  6. Respect Time Constraints: Come prepared and stay focused. If a review is taking too long, consider breaking it into multiple sessions or focusing on the most critical issues.

Common Pitfalls to Avoid

  • Nitpicking: Don't get bogged down in minor style issues at the expense of important design discussions. Use automated tools for style consistency when possible.
  • Ego-Driven Reviews: Avoid showing off or being overly critical. The goal is collaborative improvement, not demonstrating superiority.
  • Rubber Stamping: Don't just approve code without actually reviewing it. Even code from experienced developers benefits from fresh eyes and different perspectives.
  • Analysis Paralysis: While thoroughness is good, don't let perfect be the enemy of good. Focus on significant improvements rather than endless micro-optimizations.

Remember that code review is a skill that improves with practice. Be patient with yourself and others as you develop these abilities. The investment in good review practices pays dividends in code quality, team knowledge sharing, and reduced bugs in production.