PHP-Code-Review: Was ein Senior sagt & wie du glänzt

Einleitung: Warum muss dein Code immer umgeschrieben werden?

Du hast Code geschrieben. Er funktioniert. Du bist stolz. Doch dann kommt das Code-Review, und der Senior-Entwickler hinterlässt 15 Kommentare – die Hälfte beginnt mit „Was ist, wenn...“ oder „Schreiben wir das lieber um mit...“. Kommt dir bekannt vor?

Ein PHP-Code-Review ist nicht dazu da, um kleinlich zu sein. Es geht um das Überleben des Projekts. Senioren schauen nicht nur auf „läuft/läuft nicht“, sondern darauf, wie sich der Code unter Last verhält, wie gut ihn andere Entwickler warten können und wie viel Zeit du in sechs Monaten für die Fehlersuche brauchst.

In diesem Artikel analysieren wir die häufigsten Kritikpunkte, die du im Code-Review von einem erfahrenen PHP-Entwickler hörst. Ich zeige dir echte Code-Beispiele und erkläre, wie du sie verbesserst. Nach der Lektüre kannst du 80 % der Anmerkungen vorwegnehmen und Code schreiben, den du gerne zeigst.

1. „Magische Zahlen“ und Strings: Code sollte für sich selbst sprechen

Das Erste, was einem Senior auffällt, sind unverständliche Zahlen und Strings, die im Code verstreut sind. Magische Zahlen sind ein Anti-Pattern, das die Lesbarkeit zerstört.

Schlechtes Beispiel

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

Was sagt der Senior?

\n
• Warum genau 18? Ist das das Erwachsenenalter oder die Rabattgrenze?
\n
• Warum 0.1? Sind das 10 %? Was, wenn der Rabatt sich ändert?
\n

Gutes Beispiel

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

Jetzt erzählt der Code seine eigene Geschichte. Der Senior wird zustimmen: Konstanten können wiederverwendet, an einer Stelle geändert werden und benötigen keine Kommentare.

2. Fehlende Typen: PHP 8 ist keine Ausrede für dynamische Typisierung

PHP ist eine schwach typisierte Sprache, aber das heißt nicht, dass du schreiben sollst wie im Jahr 2005. Strikte Typisierung schützt vor schwer auffindbaren Bugs.

Schlechtes Beispiel

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}

Anmerkungen des Seniors

\n
• Was ist $items? Ein Array? Ein Objekt? Eine Collection?
\n
• Welche Struktur hat ein Element? Warum kein DTO?
\n
• Was passiert, wenn $tax als String „0.2“ übergeben wird? Gibt es eine Verkettung?
\n

Gutes Beispiel

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}

Der Senior wird sagen: „Gut gemacht, du nutzt DTOs, readonly-Eigenschaften und strikte Typisierung. Der Code ist vorhersagbar.“ Das ist Best Practice für PHP 8+.

3. Tiefe Verschachtelungen und Early Return

Wenn du 5 Ebenen von if...else in einer einzigen Methode siehst, fasst sich der Senior an den Kopf. Solcher Code ist unmöglich zu lesen, zu testen und zu debuggen.

Schlechtes Beispiel

public function processOrder($order) {\n    if ($order->isPaid()) {\n        if ($order->hasStock()) {\n            if ($order->isShippable()) {\n                // 50 Zeilen Logik\n                return $this->ship($order);\n            } else {\n                throw new \\Exception('Bestellung kann nicht versendet werden');\n            }\n        } else {\n            throw new \\Exception('Nicht auf Lager');\n        }\n    } else {\n        throw new \\Exception('Bestellung nicht bezahlt');\n    }\