Code review в C#: что проверять и как давать обратную связь на практике

Code review в C#: что проверять и как давать обратную связь на практике

Code review — это не просто формальная процедура перед мержем пул-реквеста. Это мощный инструмент для повышения качества кода, распространения знаний в команде и предотвращения багов. Особенно это актуально для C# и экосистемы .NET, где есть множество нюансов: от работы с памятью и асинхронностью до строгой типизации и LINQ.

В этой статье мы разберем, на что конкретно обращать внимание при ревью C# кода, и как давать обратную связь так, чтобы вас слушали, а не ненавидели.

1. Архитектура и логика: "А зачем мы это делаем?"

Прежде чем смотреть на форматирование и названия переменных, убедитесь, что решаемая задача вообще имеет смысл. Это самый важный, но часто пропускаемый этап.

• Соответствие требованиям: Решает ли код именно ту бизнес-задачу, которая описана в тикете? Нет ли лишней функциональности (YAGNI)?
• Нарушение принципов SOLID: Не превратился ли класс в "божественный объект"? Есть ли четкое разделение ответственности? Например, не смешивается ли бизнес-логика с логикой доступа к данным?
• Избыточная сложность: Можно ли решить задачу проще? Иногда разработчики пишут сложные паттерны там, где достаточно обычного if-else или простого цикла.

// ❌ Плохо: Класс делает слишком много
public class UserService
{
    public void RegisterUser(string email, string password)
    {
        // Валидация
        if (!email.Contains("@"))
            throw new Exception("Invalid email");

        // Хэширование пароля
        var hash = BCrypt.HashPassword(password);

        // Сохранение в БД
        using var db = new AppDbContext();
        db.Users.Add(new User { Email = email, PasswordHash = hash });
        db.SaveChanges();

        // Отправка email
        SmtpClient.Send("welcome@site.com", email, "Welcome!");
    }
}

// ✅ Хорошо: Разделение на слои (SRP)
public class UserRegistrationService
{
    private readonly IUserRepository _repository;
    private readonly IEmailService _emailService;

    public UserRegistrationService(IUserRepository repository, IEmailService emailService)
    {
        _repository = repository;
        _emailService = emailService;
    }

    public async Task<Result> RegisterAsync(string email, string password)
    {
        if (!EmailValidator.IsValid(email))
            return Result.Failure("Invalid email");

        var user = User.Create(email, password);
        await _repository.AddAsync(user);
        await _emailService.SendWelcomeAsync(email);

        return Result.Success();
    }
}

2. Производительность и безопасность: "Не убьет ли это сервер?"

.NET — мощный фреймворк, но его легко использовать неправильно. Вот что стоит проверять в каждом ревью.

• Асинхронность (async/await): Нет ли блокирующих вызовов (.Result, .Wait()) в асинхронном коде? Это убивает производительность. Используйте ConfigureAwait(false) в библиотеках.
• Утечки памяти: Не забыли ли освободить IDisposable ресурсы? Используется ли using для Stream, SqlConnection и т.д.?
• SQL-инъекции: Строятся ли запросы через конкатенацию строк? В современном C# с Entity Framework или Dapper это редкость, но проверка обязательна.
• LINQ и коллекции: Нет ли множественных проходов по одной и той же коллекции? Используется ли Any() вместо Count() > 0 для проверки наличия элементов?

// ❌ Плохо: Блокирующий вызов и множественный проход
var task = GetDataAsync();
var data = task.Result; // Блокировка потока!

if (data.Count() > 0) // Первый проход
{
    var first = data.First(); // Второй проход
    Console.WriteLine(first);
}

// ✅ Хорошо: Асинхронность и один проход
var data = await GetDataAsync(); // Асинхронно

if (data.Any()) // Быстрая проверка
{
    var first = data.First();
    Console.WriteLine(first);
}

3. Читаемость и стиль кода: "Поймет ли это стажер?"

Код пишется один раз, а читается десятки раз. Хороший ревьюер — это адвокат читателя кода.

• Именование: Названия переменных, методов и классов должны говорить сами за себя. Избегайте сокращений (tmp, data, GetIt).
• Магические числа: Все числовые константы должны быть вынесены в именованные константы или enum.
• Слишком длинные методы: Если метод занимает больше 20-30 строк, скорее всего, его нужно разбить на несколько более мелких.
• Комментарии: Комментарии должны объяснять почему сделано так, а не что делает код (это должен быть понятно из самого кода).

// ❌ Плохо: Непонятные имена и магические числа
public double Calc(int a, int b)
{
    return (a + b) * 0.15; // Что за 0.15? Магия!
}

// ✅ Хорошо: Самодокументируемый код
private const double TaxRate = 0.15;

public double CalculateTotalWithTax(double price, double quantity)
{
    var subtotal = price * quantity;
    return subtotal * TaxRate;
}

4. Как давать обратную связь: психология ревью

Даже идеальный код не поможет, если автор будет обижен или демотивирован после вашего комментария. Вот несколько правил конструктивного фидбека.

• Будьте конкретны: Вместо "Этот код плохой" напишите "Здесь нарушается принцип единственной ответственности. Предлагаю вынести отправку email в отдельный сервис".
• Используйте "Я-сообщения": "Мне кажется, что этот метод можно упростить" звучит лучше, чем "Ты написал слишком сложный код".
• Хвалите хорошее: Если вы видите элегантное решение или красивый паттерн, обязательно отметьте это. Это мотивирует.
• Раз