Code Review in C#: What to Check and How to Give Feedback in Practice

Code Review in C#: What to Check and How to Give Feedback in Practice

Code review is not just a formal procedure before merging a pull request. It is a powerful tool for improving code quality, spreading knowledge within the team, and preventing bugs. This is especially relevant for C# and the .NET ecosystem, which has many nuances: from memory management and asynchrony to strong typing and LINQ.

In this article, we will break down exactly what to pay attention to when reviewing C# code, and how to give feedback so that people listen to you, not hate you.

1. Architecture and Logic: "Why are we doing this?"

Before looking at formatting and variable names, make sure the task being solved actually makes sense. This is the most important, yet often skipped, step.

\n
• Requirements compliance: Does the code solve exactly the business task described in the ticket? Is there any unnecessary functionality (YAGNI)?
\n
• Violation of SOLID principles: Has the class turned into a "god object"? Is there a clear separation of responsibilities? For example, is business logic mixed with data access logic?
\n
• Unnecessary complexity: Can the task be solved more simply? Sometimes developers write complex patterns where a simple if-else or a simple loop would suffice.
\n

// ❌ Bad: Class does too much\npublic class UserService\n{\n    public void RegisterUser(string email, string password)\n    {\n        // Validation\n        if (!email.Contains("@"))\n            throw new Exception("Invalid email");\n\n        // Password hashing\n        var hash = BCrypt.HashPassword(password);\n\n        // Saving to DB\n        using var db = new AppDbContext();\n        db.Users.Add(new User { Email = email, PasswordHash = hash });\n        db.SaveChanges();\n\n        // Sending email\n        SmtpClient.Send("welcome@site.com", email, "Welcome!");\n    }\n}\n\n// ✅ Good: Separation into layers (SRP)\npublic class UserRegistrationService\n{\n    private readonly IUserRepository _repository;\n    private readonly IEmailService _emailService;\n\n    public UserRegistrationService(IUserRepository repository, IEmailService emailService)\n    {\n        _repository = repository;\n        _emailService = emailService;\n    }\n\n    public async Task<Result> RegisterAsync(string email, string password)\n    {\n        if (!EmailValidator.IsValid(email))\n            return Result.Failure("Invalid email");\n\n        var user = User.Create(email, password);\n        await _repository.AddAsync(user);\n        await _emailService.SendWelcomeAsync(email);\n\n        return Result.Success();\n    }\n}

2. Performance and Security: "Will this kill the server?"

.NET is a powerful framework, but it's easy to misuse. Here's what to check in every review.

\n
• Asynchrony (async/await): Are there any blocking calls (.Result, .Wait()) in asynchronous code? This kills performance. Use ConfigureAwait(false) in libraries.
\n
• Memory leaks: Were IDisposable resources not released? Is using used for Stream, SqlConnection, etc.?
\n
• SQL injection: Are queries built using string concatenation? In modern C# with Entity Framework or Dapper this is rare, but checking is mandatory.
\n
• LINQ and collections: Are there multiple iterations over the same collection? Is Any() used instead of Count() > 0 to check for the presence of elements?
\n

// ❌ Bad: Blocking call and multiple iterations\nvar task = GetDataAsync();\nvar data = task.Result; // Thread blocking!\n\nif (data.Count() > 0) // First it