Code Review на C#: что проверять, чтобы код не болел

Code Review на C#: не просто галочка, а спасение прода

Вы когда-нибудь видели код, который работает, но от которого хочется закрыть глаза? «Магические числа», циклы в циклах, async void и null там, где его быть не должно. Code review на C# — это не бюрократическая процедура, а последний рубеж обороны перед тем, как баг уйдёт в прод. Или, что ещё хуже, перед тем, как код начнут поддерживать ваши коллеги, проклиная автора.

В этой статье мы разберём, на что реально обращать внимание при ревью C# кода. Никакой воды — только конкретные кейсы, антипаттерны и примеры. Если вы джуниор — это ваш чек-лист. Если сеньор — освежите память. Поехали.

1. Null — главный враг C# разработчика

Знаменитый NullReferenceException — причина миллионов бессонных ночей. Первое, что проверяем на code review: защиту от null.

Что должно насторожить?

• Отсутствие проверки на null перед обращением к свойствам.
• Возврат null из методов, где ожидается коллекция.
• Игнорирование nullable-контекстов (#nullable enable).

Плохо:

public string GetUserName(User user)
{
    return user.Name.ToUpper(); // user может быть null!
}

Хорошо (C# 8+):

public string GetUserName(User? user)
{
    return user?.Name?.ToUpper() ?? "Unknown";
}

Совет для code review: требуйте включения Nullable в новых проектах. Это не просто модно, это спасает от глупых ошибок. Code review C# без проверки nullable — как езда без ремня безопасности.

2. Асинхронность: async/await без граблей

Асинхронное программирование в C# — мощный инструмент, но при неправильном использовании он превращается в мину замедленного действия. На ревью мы проверяем три главных греха.

Грех №1: async void

Методы async void нельзя отследить — исключение упадёт в финальный поток и убьёт процесс. Допустимо только для обработчиков событий (например, кнопки в WinForms).

Плохо:

public async void LoadDataAsync()
{
    var data = await _service.GetDataAsync(); // Ошибка убьёт процесс
}

Хорошо:

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

Грех №2: Блокировка асинхронного кода

.Result или .Wait() в асинхронном контексте — верный путь к дедлоку.

Плохо:

var result = _service.GetDataAsync().Result; // deadlock в UI-приложениях

Хорошо:

var result = await _service.GetDataAsync();

Грех №3: Отсутствие CancellationToken

Долгие операции должны быть отменяемы. Если в сигнатуре метода нет CancellationToken, задайте вопрос автору: «А что будет, если пользователь закроет вкладку?»

3. LINQ: красота или медленная смерть?

LINQ — это элегантно. Но code review часто вскрывает, что за элегантностью скрывается O(n²) или множественные проходы по коллекции.

Типичные ошибки при ревью C# кода с LINQ:

• Множественный обход: .Count() > 0 + .First() — два прохода. Используйте .Any() и .FirstOrDefault().
• Забытый ToList(): ленивая загрузка может привести к тому, что запрос к БД выполнится после того, как контекст DataReader уже закрыт.
• Where + Select = избыточность: если можно использовать один SelectMany — используйте.

Пример, который должен вызвать красный флаг:

var result = list.Where(x => x.IsActive).OrderBy(x => x.Name).ToList();
// Если list — это IQueryable из БД, то сортировка и фильтрация уйдут на сервер.
// Но если list — это уже материализованный List, то всё ок.
// Code review должен проверять тип источника!

4. Исключения: когда catch — это зло

Обработка ошибок — тема, где code review на C# часто проходит мимо. Основное правило: не глотайте исключения.

Антипаттерны:

• catch (Exception ex) { /* ничего */ } — молчаливое поглощение.
• catch (Exception ex) { throw ex; } — сбрасывает стек вызовов.
• Использование исключений для управления потоком (например, int.Parse в try-catch вместо int.TryParse).

Правильный паттерн:

public async Task<User> GetUserAsync(int id)
{
    try
    {
        return await _repository.GetByIdAsync(id);
    }
    catch (NotFoundException)
    {
        _logger.LogWarning($"User {id} not found");
        return null; // или бросить кастомное исключение
    }
}

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

5. Магические числа и строки

Если в коде встречается if (status == 3) или var timeout = 5000 без пояснения — это плохо. На code review мы требуем выносить такие константы в enum или const.

Плохо:

if (order.Status == 2) { /* отменён */ }

Хорошо:

if (order.Status == OrderStatus.Cancelled) { /* понятно без комментариев */ }

6. Dispose и using — утечка ресурсов

Работа с файлами, сетевыми соединениями, БД — всё это требует освобождения ресурсов. Проверяем:

• Есть ли using для SqlConnection, StreamReader, HttpClient?
• Не забыт ли Dispose() у IDisposable объектов?
• Используется ли await using для асинхронных потоков?

Пример из code review на C#:

// Пло