PHP Code Review: 10 Mistakes That Drive Seniors Crazy

Online Python Trainer for Beginners

Learn Python easily without overwhelming theory. Solve practical tasks with automatic checking, get hints in Russian, and write code directly in your browser — no installation required.

Start Course

Introduction: Why Is Your PHP Code Being Rewritten?

\n

You submitted a Pull Request, full of confidence that you wrote perfect code. An hour passes, and you see 15 comments from a senior developer. Sound familiar? Code review in PHP is not just a formality, but a real test of maturity. Seniors look at code not as a set of instructions, but as a system that will live and evolve.

\n

In this article, we will break down the most frequent comments you receive during reviews. We won't teach you syntax — you already know how to declare a variable. We'll talk about code culture, architecture, and pitfalls that turn working code into a maintenance nightmare. Get ready: it will be painful, but useful.

\n\n

1. Magic Numbers and Strings: "What Does 42 Mean?"

\n

The most common and most annoying mistake. A senior sees if ($status == 3) in the code and immediately deducts points. The number 3 is a magic constant. In a month, you won't remember what it means yourself. And a new developer will guess: is it the "Published" status, "Draft", or "Deleted"?

\n\n

Bad:

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

Good:

\n
class OrderStatus {\n    const PENDING = 1;\n    const CONFIRMED = 2;\n    const SHIPPED = 3;\n    const DELIVERED = 4;\n}\n\nif ($order->getStatus() === OrderStatus::SHIPPED) {\n    $order->sendNotification();\n}
\n

Using named constants makes the code self-documenting. You're not just writing code — you're telling a story. This is one of the key php best practices that juniors often ignore.

\n\n

2. God Objects and 500-Line Monsters

\n

"This class does everything!" — a phrase that makes a senior shudder. When one class is responsible for sending emails, generating PDFs, and working with the database, it violates the Single Responsibility Principle (SRP). Such a class is impossible to test, difficult to change, and is guaranteed to break at the slightest refactoring.

\n\n

Signs of a God Object:

\n
    \n
  • The class contains more than 200-300 lines
  • \n
  • A method has more than 20-30 lines
  • The class name ends with "Manager" or "Helper"
  • Methods accept 5+ parameters
\n

The solution is simple: split it up! Extract logic into services, repositories, handlers. PHP refactoring starts small — make a class that does only one thing, but does it well.

\n\n

3. "Bare" SQL Work via PDO

\n

Yes, PDO protects against injections if you use prepared statements. But a senior wants to see an abstraction, not raw SQL in the controller. ORMs like Eloquent or Doctrine are the standard. If the project is small, use at least a Query Builder.

\n\n

Antipattern:

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

Correct Approach:

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

Working through a repository allows you to easily change the data source (e.g., switch to Redis cache) without rewriting the entire project. This is PHP code quality that is worth its weight in gold.

\n\n

4. Ignoring strict_types

\n

PHP is a dynamically typed language, but that's no reason to write code like it's 2005. Declaring declare(strict_types=1); at the beginning of each file is a sign of professionalism. A senior hates it when a function expects an int but receives the string "42" and silently converts it.

\n\n
declare(strict_types=1);\n\nfunction calculateTotal(int $price, int $quantity): int {\n    return $price * $quantity;\n}\n\n// This will throw a TypeError, not a silent conversion\necho calculateTotal('100', 2); /                

Blogs

Book Recommendations