Revue de code PHP : les remarques d'un senior pour éviter la honte

Introduction : Pourquoi votre code est-il systématiquement réécrit ?

Vous avez écrit du code. Il fonctionne. Vous êtes fier de vous. Mais la code review arrive, et le développeur senior laisse 15 commentaires, dont la moitié commence par « Et si... » ou « On réécrit ça avec... ». Ça vous parle ?

La revue de code PHP ne consiste pas à chercher la petite bête. C'est une question de survie du projet. Les seniors ne regardent pas simplement si « ça marche ou pas », mais comment le code se comportera sous charge, comment un autre développeur le maintiendra, et combien de temps vous passerez à chercher un bug dans six mois.

Dans cet article, nous allons décortiquer les remarques les plus fréquentes que vous entendrez en code review de la part d'un développeur PHP expérimenté. Je vous montrerai des exemples de code réels et vous expliquerai comment les corriger. Après cette lecture, vous serez capable d'anticiper 80 % des critiques et d'écrire un code dont vous n'aurez pas honte.

1. « Nombres magiques » et chaînes : le code doit parler de lui-même

La première chose qu'un senior remarquera, ce sont les nombres et les chaînes incompréhensibles disséminés dans le code. Les nombres magiques sont un antipattern qui tue la lisibilité.

Mauvais exemple

if ($user->getAge() > 18) {\n    $discount = $price * 0.1;\n}

Que dira le senior ?

\n
• Pourquoi 18 ? Est-ce la majorité légale ou un seuil de réduction ?
\n
• Pourquoi 0.1 ? C'est 10 % ? Et si la réduction change ?
\n

Bon exemple

const ADULT_AGE = 18;\nconst DISCOUNT_RATE = 0.1; // 10%\n\nif ($user->getAge() > ADULT_AGE) {\n    $discount = $price * DISCOUNT_RATE;\n}

Maintenant, le code raconte une histoire. Le senior approuvera : les constantes sont réutilisables, modifiables en un seul endroit, et ne nécessitent aucun commentaire.

2. Absence de typage : PHP 8 n'excuse pas le typage dynamique

PHP est un langage à typage faible, mais ce n'est pas une raison pour coder comme en 2005. Le typage strict évite des bugs difficiles à détecter.

Mauvais exemple

function calculateTotal($items, $tax) {\n    $total = 0;\n    foreach ($items as $item) {\n        $total += $item['price'] * $item['quantity'];\n    }\n    return $total * (1 + $tax);\n}

Remarques du senior

\n
• Qu'est-ce que $items ? Un tableau ? Un objet ? Une collection ?
\n
• Quelle est la structure d'un élément ? Pourquoi pas un DTO ?
\n
• Et si on passe $tax sous forme de chaîne "0.2" ? Ça fera une concaténation ?
\n

Bon exemple

declare(strict_types=1);\n\nclass OrderItem {\n    public function __construct(\n        public readonly float $price,\n        public readonly int $quantity\n    ) {}\n}\n\nfunction calculateTotal(array $items, float $taxRate): float {\n    $total = 0.0;\n    foreach ($items as $item) {\n        if (!$item instanceof OrderItem) {\n            throw new InvalidArgumentException('Expected OrderItem');\n        }\n        $total += $item->price * $item->quantity;\n    }\n    return $total * (1 + $taxRate);\n}

Le senior dira : « Bien joué, tu utilises un DTO, des propriétés readonly et le typage strict. Le code est prévisible. » C'est une bonne pratique PHP 8+.

3. Imbrications profondes et retour anticipé (Early Return)

Quand vous voyez 5 niveaux de if...else dans une seule méthode, le senior se prend la tête. Ce code est impossible à lire, tester et déboguer.

Mauvais exemple

public function processOrder($order) {\n    if ($order->isPaid()) {\n        if ($order->hasStock()) {\n            if ($order->isShippable()) {\n                // 50 lignes de logique\n                return $this->ship($order);\n            } else {\n                throw new \\Exception('Order cannot be shipped');\n            }\n        } else {\n            throw new \\Exception('Out of stock');\n        }\n    } else {\n        throw new \\Exception('Order not paid');\n    }\