Code Review Best Practices: How to Review Code Effectively

Code review is one of the most important practices in software development. Done well, it catches bugs, improves code quality, and helps teams learn. Done poorly, it creates friction and slows development. After reviewing thousands of pull requests, I've learned what makes reviews effective.

Why Code Review Matters

Benefits

• Catch bugs early - Before they reach production
• Improve code quality - Better design, cleaner code
• Knowledge sharing - Team learns from each other
• Consistency - Maintain coding standards
• Security - Catch vulnerabilities
• Documentation - Code is self-documenting

The Code Review Mindset

It's About the Code, Not the Person

// Bad: Personal attack
"This is terrible. You clearly don't understand how this works."

// Good: Focus on code
"I see what you're trying to do here. There's a pattern we use for this situation that might be cleaner."

Be Constructive

// Bad: Just criticism
"This is wrong."

// Good: Explain and suggest
"This approach might have edge cases. Consider using [alternative] which handles [specific case]."

Ask Questions

// Good: Encourage discussion
"I'm curious about this approach. What was your reasoning for [decision]?"

What to Review

1. Functionality

• Does it work?
• Does it handle edge cases?
• Are there bugs?

2. Code Quality

• Is it readable?
• Is it maintainable?
• Does it follow patterns?

3. Performance

• Are there performance issues?
• Could it be optimized?

4. Security

• Are there vulnerabilities?
• Is input validated?
• Are secrets handled properly?

5. Testing

• Are there tests?
• Do tests cover edge cases?
• Are tests maintainable?

Review Checklist

Functionality

• Code works as intended
• Handles edge cases
• Error handling is appropriate
• No obvious bugs

Code Quality

• Code is readable
• Follows project conventions
• No code duplication
• Functions are focused
• Naming is clear

Performance

• No obvious performance issues
• Database queries are efficient
• No unnecessary operations

Security

• Input is validated
• No SQL injection risks
• No XSS vulnerabilities
• Secrets not hardcoded
• Authentication/authorization correct

Testing

• Tests are included
• Tests are meaningful
• Edge cases are tested

Giving Feedback

The Feedback Sandwich

1. Positive - What's good
2. Improvement - What could be better
3. Positive - Reinforce ability

Example:

"Great job getting this feature working! I noticed the error handling could be more specific. But overall, the structure is really clean, and I can see you're thinking about maintainability."

Be Specific

// Bad: Vague
"This could be better."

// Good: Specific
"This function is doing three things: validation, transformation, and saving. Consider splitting it into three functions for better testability."

Explain Why

// Bad: Just say what
"Use async/await instead."

// Good: Explain why
"Using async/await here would make error handling clearer and the code more readable. The current promise chain is hard to follow."

Suggest Alternatives

// Bad: Just say it's wrong
"This is wrong."

// Good: Suggest alternative
"Instead of [current approach], consider [alternative] because [reason]. Here's an example: [code snippet]"

Common Review Comments

Code Style

// Comment: "Consider using const instead of let since this variable isn't reassigned."
let name = getUserName();
// Better:
const name = getUserName();

Error Handling

// Comment: "This might throw if user is null. Consider adding a null check."
const email = user.email;
// Better:
const email = user?.email;
if (!email) {
  throw new Error('User email not found');
}

Performance

// Comment: "This creates a new array on each iteration. Consider using map() instead."
const results = [];
items.forEach(item => {
  results.push(processItem(item));
});
// Better:
const results = items.map(item => processItem(item));

Security

// Comment: "This is vulnerable to SQL injection. Use parameterized queries."
const query = `SELECT * FROM users WHERE id = ${userId}`;
// Better:
const query = 'SELECT * FROM users WHERE id = $1';
db.query(query, [userId]);

Reviewing Different Types of Changes

New Features

• Does it solve the problem?
• Is the design sound?
• Are edge cases handled?
• Are there tests?

Bug Fixes

• Does it fix the bug?
• Does it introduce new bugs?
• Is the root cause addressed?
• Are there regression tests?

Refactoring

• Is the code better?
• Are tests updated?
• Is behavior unchanged?
• Is it easier to understand?

Performance Improvements

• Is it actually faster?
• Is it still correct?
• Are there trade-offs?
• Is it measurable?

Handling Disagreements

Focus on Goals

// Instead of arguing about style
// Discuss: "What are we trying to achieve?"
// Then: "Which approach better achieves that?"

Use Data

// Instead of opinions
// Use: Benchmarks, examples, patterns

Defer to Owner

// If no clear winner
// Defer to the person who will maintain it

Real-World Example

Pull Request: Add user authentication

Review Comments:

1. Functionality: "The login endpoint handles invalid credentials correctly. Good!"

2. Security: "Consider rate limiting the login endpoint to prevent brute force attacks."

3. Code Quality: "The validateUser function is doing both validation and authentication. Consider splitting into validateCredentials and authenticateUser for better testability."

4. Testing: "The tests cover happy path well. Could we add a test for the case where the database connection fails?"

5. Documentation: "The API endpoint is clear, but could we add JSDoc comments explaining the expected request/response format?"

Result:

• Code quality improved
• Security enhanced
• Tests more comprehensive
• Team learned from discussion

Best Practices

1. Review promptly - Don't let PRs sit
2. Be respectful - Focus on code, not person
3. Be specific - Vague comments aren't helpful
4. Explain why - Help others learn
5. Suggest alternatives - Don't just criticize
6. Approve when ready - Don't block unnecessarily
7. Learn from reviews - Apply feedback to your code
8. Keep it small - Smaller PRs are easier to review

Common Mistakes

1. Nitpicking

Problem: Commenting on minor style issues

Solution: Focus on important issues, let style slide

2. Being Too Harsh

Problem: Demeaning comments

Solution: Be constructive and respectful

3. Not Explaining

Problem: Just saying "fix this"

Solution: Explain why and suggest how

4. Blocking on Preferences

Problem: Blocking PRs for style preferences

Solution: Discuss, but don't block

Conclusion

Code review is a skill that improves with practice. The key is to:

• Focus on code - Not the person
• Be constructive - Help improve, don't just criticize
• Explain why - Help others learn
• Be respectful - Create positive culture

Remember: The goal of code review is to improve code quality and help the team learn. Keep that in mind, and your reviews will be effective.

What code review practices have worked best for your team? What challenges have you faced?