Ревью кода на C#: 7 вещей, которые бесят senior-разработчиков

Введение: Почему ваш код вызывает у сеньора тик?

Вы отправили пул-реквест. Прошло пять минут. В чате прилетает: «А давай перепишем вот этот блок?» и десяток комментариев под каждой второй строкой. Знакомо? Если вы пишете на C# и работаете в команде, code review — это не просто галочка в Jira. Это момент истины.

Я собрал типичные замечания, которые senior-ы оставляют на ревью кода C#. Это не про синтаксис — это про мышление. Про то, как превратить «просто рабочий код» в код, который не хочется переписать в первую же пятницу.

Поехали разбирать антипаттерны C#, которые бесят даже самых спокойных тимлидов.

1. Магические числа и строки: «А что это значит?»

Самый частый комментарий на ревью C# кода — это вопрос к числам. Вы пишете if (status == 3), и senior сразу спрашивает: «Что за 3? Это статус “В обработке” или “Удален”?».

Плохо:

if (order.Status == 3 && days > 7)
{
    CancelOrder(order);
}

Хорошо:

private const int OrderStatusCancelled = 3;
private const int MaxDaysBeforeAutoCancel = 7;

if (order.Status == OrderStatusCancelled && days > MaxDaysBeforeAutoCancel)
{
    CancelOrder(order);
}

Ещё лучше — вынести в enum. Senior-ы обожают читаемый код, где не нужно лезть в базу данных, чтобы понять бизнес-логику. Это базовое правило code review C#: код должен говорить сам за себя.

2. Async void — тихая убийца приложений

Если вы пишете async void где-то, кроме обработчиков событий (например, в WPF или WinForms), готовьтесь к жирному минусу. Async void нельзя отследить — исключение упадет в никуда и положит процесс.

Плохо:

public async void LoadData()
{
    var data = await _repository.GetDataAsync();
    // Если тут упадет — приложение упадет без сообщения об ошибке
}

Хорошо:

public async Task LoadDataAsync()
{
    var data = await _repository.GetDataAsync();
}

На ревью кода C# сеньор обязательно ткнет в этот момент. Запомните: async Task — ваш друг, async void — враг (кроме ивентов).

3. Годзилла-классы: Single Responsibility Principle

Вы открываете файл, а там 2000 строк. Один класс делает всё: ходит в базу, шлет email, генерирует отчет и еще кофе варит. Это классический антипаттерн C# — God Object.

Senior на code review C# скажет: «Давай разобьем на несколько классов». И будет прав.

Пример нарушения:

public class OrderService
{
    public void ProcessOrder(Order order)
    {
        // 1. Валидация
        // 2. Сохранение в БД
        // 3. Отправка email
        // 4. Логирование
    }
}

Как надо:

public class OrderProcessor
{
    private readonly IOrderValidator _validator;
    private readonly IOrderRepository _repository;
    private readonly INotificationService _notificationService;

    public OrderProcessor(
        IOrderValidator validator,
        IOrderRepository repository,
        INotificationService notificationService)
    {
        _validator = validator;
        _repository = repository;
        _notificationService = notificationService;
    }

    public async Task ProcessOrderAsync(Order order)
    {
        _validator.Validate(order);
        await _repository.SaveAsync(order);
        await _notificationService.SendConfirmationAsync(order);
    }
}

Это не просто «красиво». Это тестируемо. Вы можете замокать репозиторий и проверить только логику обработки. Рефакторинг C# в таком стиле — must have для любого проекта.

4. Исключения как управляющие конструкции

Еще одна боль на ревью кода C# — использование try-catch вместо нормальной проверки условий. Исключения — это дорого. Они ломают стек и убивают производительность.

Плохо:

try
{
    var user = users[userId];
}
catch (IndexOutOfRangeException)
{
    // Не нашли пользователя
}

Хорошо:

if (users.TryGetValue(userId, out var user))
{
    // Работаем с user
}
else
{
    // Логика "не нашли"
}

Используйте исключения только для действительно исключительных ситуаций. Не для потока управления. Это один из главных советов senior разработчика C#.

5. Null-коалиция и защитное программирование

Вы когда-нибудь видели код, где каждый метод начинается с десяти if (something == null)? Senior назовет это «защитным программированием от собственной лени».

Современный C# предлагает элегантные решения.

Плохо:

string name = null;
if (user != null && user.Profile != null)
{
    name = user.Profile.Name;
}

Хорошо:

string name = user?.Profile?.Name ?? "Гость";

Операторы ?. и ?? — это не фишки для «покрасоваться». Это способ сделать код короче и безопаснее. На code review C# примеры с такими конструкциями проходят без вопросов.

6. Комментарии-извинения и мертвый код

«// TODO: переписать этот ужас», «// fixme: работает, но не трогай», «// это сделано для клиента, не удалять». Знакомо? Senior на ревью кода C# такие комментарии не пропускает.

Плохой комментарий — тот, который объясняет почему код плох, а не что он делает. Хороший код не нуждается в комментариях. Если вам хочется написать «// костыль, потому что…», значит, надо переписать код, а не комментировать его.

То же самое касается закомментированных блоков. Вы их не удаляете «на всякий случай»? Senior удалит. Или попросит вас это сделать. Мертвый код — это шум. Типичные ошибки C# включают в себя и этот пункт.

7. Отсутствие using для IDisposable

Казалось бы, база. Но сколько раз senior-ы видят на ревью C# кода такое:

var connection = new SqlConnection(connectionString);
connection.Open();
// работаем
connection.Close(); // А если исключение?

Закрытие вру