На что обращаешь внимание во время code review?

Принципы и фокус code review

Во время code review я рассматриваю процесс не как поиск ошибок, а как коллаборативное улучшение кода и передачу знаний. Моё внимание распределяется по нескольким ключевым направлениям.

1. Корректность и функциональность

Первичная проверка — выполняет ли код свою задачу без ошибок и edge cases.

• Логика алгоритмов: Проверяю корректность, особенно в условиях цикла и ветвления.
• Обработка ошибок и исключений: Есть ли try-catch блоки для операций, которые могут завершиться неудачей (работа с файлами, API, БД)? Не подавляются ли исключения без веской причины?
• Пограничные случаи: Как код ведёт себя с пустыми массивами, null значениями, нулевыми или отрицательными числами? Учтены ли все варианты из требований?

// Плохо: Нет проверки на пустоту
$total = array_sum($userIds);

// Лучше: Явная обработка edge case
if (empty($userIds)) {
    throw new InvalidArgumentException('User IDs array cannot be empty');
}
$total = array_sum($userIds);

2. Безопасность

Это абсолютный приоритет, особенно для бэкенда.

• SQL-инъекции: Весь SQL-код должен использовать подготовленные выражения (prepared statements) через PDO или Query Builder.
• XSS и санитизация данных: Проверяю, экранируются ли данные при выводе в HTML (используется ли htmlspecialchars или шаблонизатор, который делает это по умолчанию?).
• Валидация входных данных: Проверяются ли данные на уровне контроллера/сервиса перед использованием? Не доверяем данным из $_GET, $_POST, $_COOKIE.
• Конфиденциальные данные: Не попадают ли в логи или вывод пароли, токены, персональные данные.

3. Читаемость и поддержка кода

Код читается в 10 раз чаще, чем пишется.

• Имена переменных и функций: Они должны быть говорящими ($userRepository вместо $repo, calculateOrderTotal() вместо calc()).
• Сложность методов: Длинные функции (более 20-30 строк) и высокие цикломатические сложности — сигнал к рефакторингу.
• Комментарии: Комментарии должны объяснять "почему", а не "что". Избыточные комментарии так же вредны, как и их отсутствие.

// Плохо: "Что" и так очевидно
$discount = $total * 0.1; // Рассчитываем скидку 10%

// Лучше: Объясняет "почему" (бизнес-правило)
$discount = $total * 0.1; // Скидка для постоянных клиентов уровня "Silver"

4. Архитектура и дизайн

Смотрю на то, как изменения вписываются в общую картину.

• Принципы SOLID: Не нарушаются ли? Например, не берет ли класс на себя слишком много ответственности (нарушение Single Responsibility).
• Связность и зацепление (Coupling & Cohesion): Классы должны быть слабо связанными и высоко связными.
• Паттерны: Применение уместных паттернов (Repository, Service, Factory) или, наоборот, избыточное усложнение.
• Тестируемость: Можно ли легко написать юнит-тест для этого кода? Нет ли жестких зависимостей (hard-coded new Class()), которые нельзя подменить?

5. Производительность и эффективность

• Оптимизация запросов к БД: Нет ли N+1 проблемы в циклах? Используется ли жадная загрузка (with() в Laravel) или оптимизированные JOIN?
• Сложность алгоритмов: Не используются ли неоптимальные алгоритмы (например, поиск в массиве вместо хеш-таблицы) на потенциально больших данных?
• Память: Не создаются ли в памяти огромные массивы, когда можно использовать генераторы (yield)?

6. Следование стандартам проекта

• Стиль кода (PSR-12): Соблюдаются ли отступы, именование, длина строк? (Проверяю автоматически через PHP CS Fixer, но обращаю внимание на сложные случаи).
• Архитектурные соглашения: Следует ли код принятым в проекте слоям (Controller -> Service -> Repository)?
• Документация: Обновлены ли @param, @return в PHPDoc? Изменения в публичном API (методы сервиса) требуют обновления документации.

7. Тестирование

• Покрытие тестами: Поощряю написание или обновление юнит- и функциональных тестов для нового кода.
• Качество тестов: Сами тесты должны быть чистыми, изолированными и проверять именно то, что заявлено.

Мой подход: Я всегда начинаю review с позитивного подкрепления — отмечаю удачные решения. Критику формулирую в виде вопросов: "Как ты думаешь, что произойдет, если этот массив окажется пустым?" или "Рассматривали ли мы вариант вынести эту логику в отдельный сервис, чтобы избежать дублирования в другом месте?". Цель — не указать на ошибку, а вместе прийти к лучшему, более надежному и понятному решению, попутно обучая и обучаясь.