Ревью кода на PHP: 10 ошибок, которые бесят senior-ов

Онлайн-тренажер Python для начинающих

Изучайте Python легко и без перегрузки теорией. Решайте практические задачи с автоматической проверкой, получайте подсказки на русском языке и пишите код прямо в браузере — без необходимости что-либо устанавливать.

Начать курс

Введение: Почему ваш PHP-код заставляют переписывать?

Вы отправили Pull Request, полный уверенности, что написали идеальный код. Проходит час, и вы видите 15 комментариев от senior-разработчика. Знакомо? Code review на PHP — это не просто формальность, а настоящий экзамен на зрелость. Senior-ы смотрят на код не как на набор инструкций, а как на систему, которая будет жить и развиваться.

В этой статье мы разберем самые частые замечания, которые вы получаете на ревью. Мы не будем учить вас синтаксису — вы и так знаете, как объявить переменную. Мы поговорим о культуре кода, архитектуре и подводных камнях, которые превращают рабочий код в кошмар для поддержки. Готовьтесь: будет больно, но полезно.

1. Магические числа и строки: "Что значит 42?"

Самый частый и самый раздражающий косяк. Senior видит в коде if ($status == 3) и сразу ставит минус. Число 3 — это магическая константа. Через месяц вы сами не вспомните, что оно значит. А новый разработчик будет гадать: это статус "Опубликовано", "В черновике" или "Удалено"?

Плохо:

if ($order->getStatus() == 3) {
    $order->sendNotification();
}

Хорошо:

class OrderStatus {
    const PENDING = 1;
    const CONFIRMED = 2;
    const SHIPPED = 3;
    const DELIVERED = 4;
}

if ($order->getStatus() === OrderStatus::SHIPPED) {
    $order->sendNotification();
}

Использование именованных констант делает код самодокументируемым. Вы не просто пишете код — вы рассказываете историю. Это одна из главных php best practices, которую junior-ы часто игнорируют.

2. God-объекты и монстры на 500 строк

"Этот класс делает всё!" — фраза, от которой senior-а бросает в дрожь. Когда один класс отвечает и за отправку email, и за генерацию PDF, и за работу с базой — это нарушение принципа единственной ответственности (SRP). Такой класс невозможно тестировать, сложно изменять, и он гарантированно сломается при малейшем рефакторинге.

Признаки God-объекта:

Класс содержит больше 200-300 строк
В методе больше 20-30 строк
Имя класса заканчивается на "Manager" или "Helper"
Методы принимают 5+ параметров

Решение простое: дробите! Выносите логику в сервисы, репозитории, хендлеры. Рефакторинг PHP начинается с малого — сделайте класс, который делает только одну вещь, но делает её хорошо.

3. "Голая" работа с SQL через PDO

Да, PDO защищает от инъекций, если вы используете prepared statements. Но senior хочет видеть не сырой SQL в контроллере, а абстракцию. ORM вроде Eloquent или Doctrine — это стандарт. Если проект маленький, используйте хотя бы Query Builder.

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

$pdo = new PDO('mysql:host=localhost;dbname=test', 'user', 'pass');
$stmt = $pdo->prepare('SELECT * FROM users WHERE email = ?');
$stmt->execute([$email]);
$user = $stmt->fetch();

Правильный подход:

class UserRepository {
    public function findByEmail(string $email): ?User {
        return User::where('email', $email)->first();
    }
}

Работа через репозиторий позволяет легко менять источник данных (например, перейти на Redis кеш), не переписывая весь проект. Это качество кода PHP, которое ценится на вес золота.

4. Игнорирование strict_types

PHP — язык с динамической типизацией, но это не повод писать код как в 2005 году. Объявление declare(strict_types=1); в начале каждого файла — признак профессионализма. Senior ненавидит, когда функция ожидает int, а получает строку "42" и молча конвертирует её.

declare(strict_types=1);

function calculateTotal(int $price, int $quantity): int {
    return $price * $quantity;
}

// Это вызовет TypeError, а не тихую конвертацию
echo calculateTotal('100', 2); // Fatal error

Строгая типизация спасает от багов, которые могут жить в продакшене годами. Это один из ключевых советов PHP разработчикам от опытных коллег.

5. Комментарии, которые врут

"Этот код исправляет баг #12345" — бесполезный комментарий. Senior-ы читают код, а не комментарии. Если комментарий не объясняет "почему" (а не "что"), он не нужен. Код должен быть настолько чистым, чтобы комментарии были излишними.

Плохой комментарий:

// Увеличиваем счетчик на 1
$counter++;

Хороший комментарий (объяснение бизнес-логики):

// Скидка применяется только для новых пользователей (регистрация до 30 дней)
if ($user->getAgeInDays() < 30) {
    $price = $price * 0.9;
}

Лучший код — это код, который не требует комментариев. Используйте говорящие имена переменных и методов. PHP антипаттерны часто связаны с перекомментированием очевидных вещей.

6. Отсутствие обработки ошибок

Senior замечает, что у вас нет try-catch или вы глушите все исключения пустым блоком. Ошибки — это часть жизни приложения. Игнорировать их — значит подставлять пользователей и команду поддержки.

// Так делать нельзя
try {
    $data = $api->fetchData();
} catch (Exception $e) {
    // Ничего не делаем
}

// Правильный подход
try {
    $data = $api->fetchData();
} catch (ConnectionException $e) {
    Log::error('API недоступен: ' . $e->getMessage());
    throw new ServiceUnavailableException('Попробуйте позже');
}

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

7. Жесткая связность и new в конструкторе

Когда вы пишете $logger = new FileLogger(); прямо внутри класса, вы прибиваете его гвоздями к конкретной реализации. Senior сразу предложит Dependency Injection (DI).

Плохо:

class OrderProcessor {
    private $logger;
    
    public function __construct() {
        $this->logger = new FileLogger(); // Жесткая связь
    }
}

Хорошо:

class OrderProcessor {
    public function __construct(private LoggerInterface $logger) {}
}

// Внедрение зависимости
$processor = new OrderProcessor(new DatabaseLogger());

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