Explain your code review process and what you look for.

4 minintermediatebehavioralcode-reviewcollaboration

Quick Answer

An effective code review checks correctness and edge cases, readability and maintainability, adherence to conventions and architecture, adequate tests, security, and performance implications. The reviewer gives specific, kind, actionable feedback and distinguishes blocking issues from suggestions; the author keeps PRs small and self-reviews first. The aim is improving code quality and sharing knowledge, not gatekeeping.

Detailed Answer

Code Review Process:

1. Pre-Review Checklist (Author):

Before submitting a pull request:

  • Code compiles without warnings
  • All tests pass locally
  • New tests added for new functionality
  • Code coverage meets project standards (typically 80%+)
  • Self-review completed
  • PR description includes context, changes, and testing notes
  • Branch is up to date with target branch
  • Automated checks pass (CI/CD pipeline)

2. Review Structure:

I follow a tiered approach:

First Pass - High-Level Review (5-10 minutes):

  • Understand the purpose and context
  • Check if the solution aligns with requirements
  • Verify architectural patterns are followed
  • Ensure the scope is appropriate (not too large)

Second Pass - Detailed Review (15-30 minutes):

  • Line-by-line code examination
  • Check for code quality issues
  • Verify test coverage and quality
  • Look for potential bugs or edge cases

Third Pass - Final Check (5 minutes):

  • Review conversation and addressed comments
  • Ensure all concerns are resolved
  • Final approval or request changes

3. What I Look For:

Correctness:

  • Does the code do what it's supposed to do?
  • Are edge cases handled?
  • Are there potential null reference exceptions?
  • Is error handling appropriate?
// Look for proper null handling
public async Task GetUserAsync(int userId)
{
    var user = await _context.Users.FindAsync(userId);
    
    // Good - explicit null check
    if (user == null)
        throw new UserNotFoundException(userId);
    
    return _mapper.Map(user);
}

Code Quality:

  • SOLID principles adherence
  • DRY (Don't Repeat Yourself)
  • Clear and descriptive naming
  • Appropriate abstraction levels
  • Single Responsibility Principle
// Prefer specific, intention-revealing names
// Bad
public void Process(List data) { }

// Good
public void CalculateMonthlyRevenue(List orders) { }

Performance:

  • N+1 query problems
  • Unnecessary allocations
  • Inefficient algorithms
  • Proper async/await usage
  • Caching opportunities
// Check for N+1 queries
// Bad
var orders = await _context.Orders.ToListAsync();
foreach (var order in orders)
{
    // N+1: Separate query for each order
    order.Customer = await _context.Customers.FindAsync(order.CustomerId);
}

// Good
var orders = await _context.Orders
    .Include(o => o.Customer)
    .ToListAsync();

Security:

  • SQL injection prevention (parameterized queries)
  • XSS protection
  • Authentication and authorization
  • Sensitive data handling
  • Input validation
// Look for SQL injection vulnerabilities
// Bad
var sql = $"SELECT * FROM Users WHERE Username = '{username}'";

// Good
var user = await _context.Users
    .FirstOrDefaultAsync(u => u.Username == username);

Testing:

  • Unit tests for business logic
  • Integration tests for critical paths
  • Test naming and structure
  • Test coverage of edge cases
  • Proper use of mocks and stubs
[Fact]
public async Task GetUserAsync_WhenUserNotFound_ThrowsUserNotFoundException()
{
    // Arrange
    var userId = 999;
    _mockRepository
        .Setup(r => r.GetByIdAsync(userId))
        .ReturnsAsync((User)null);
    
    // Act & Assert
    await Assert.ThrowsAsync(
        () => _userService.GetUserAsync(userId));
}

Maintainability:

  • Code comments where necessary (why, not what)
  • Consistent formatting and style
  • Appropriate use of design patterns
  • Modular and loosely coupled design
  • Configuration vs. hard-coded values

API Design:

  • RESTful conventions
  • Appropriate HTTP status codes
  • Consistent response formats
  • API versioning strategy
  • Proper use of DTOs

4. Feedback Style:

  • Be respectful and constructive
  • Ask questions rather than make demands
  • Provide reasoning for suggestions
  • Acknowledge good solutions
  • Distinguish between critical issues and suggestions
// Good feedback examples:
"Consider using FirstOrDefaultAsync here instead of SingleOrDefaultAsync 
 to improve performance when we expect unique results."

"Great use of the strategy pattern here! This makes the code much more testable."

"This could throw a NullReferenceException if user.Address is null. 
 Should we add a null check or use the null-conditional operator?"

5. Review Efficiency:

  • Limit PR size (300-400 lines max)
  • Use code review checklists
  • Leverage automated tools (linters, static analysis)
  • Timely reviews (within 24 hours)
  • Avoid nitpicking on style (use automated formatters)

6. Follow-Up:

  • Verify that feedback is addressed appropriately
  • Approve when all critical issues are resolved
  • Mark minor suggestions as non-blocking
  • Document patterns for team learning