Revisión de código PHP: qué dice un senior y cómo no quedar mal

Introducción: ¿Por qué te hacen reescribir tu código?

Escribiste el código. Funciona. Estás orgulloso. Pero llega el code review y el desarrollador senior te deja 15 comentarios, la mitad empezando con "¿Y qué pasa si...?" o "Mejor reescribamos esto con...". ¿Te suena?

La revisión de código en PHP no es cuestión de ser quisquilloso. Se trata de la supervivencia del proyecto. Los seniors no solo miran si "funciona o no", sino cómo se comportará el código bajo carga, cómo lo mantendrá otro desarrollador y cuánto tiempo perderás buscando un bug dentro de seis meses.

En este artículo analizaremos las observaciones más frecuentes que escucharás en un code review de un desarrollador PHP experimentado. Te mostraré ejemplos reales de código y te explicaré cómo corregirlos. Después de leerlo, podrás anticipar el 80% de las críticas y escribir código del que no te avergüences.

1. "Números mágicos" y cadenas: el código debe hablar por sí mismo

Lo primero que notará un senior son los números y cadenas sin explicación repartidos por el código. Los números mágicos son un antipatrón que mata la legibilidad.

Ejemplo malo

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

¿Qué dirá el senior?

\n
• ¿Por qué exactamente 18? ¿Es la mayoría de edad o el umbral de descuento?
\n
• ¿Por qué 0.1? ¿Es el 10%? ¿Y si el descuento cambia?
\n

Ejemplo bueno

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

Ahora el código cuenta su propia historia. El senior lo aprobará: las constantes se pueden reutilizar, cambiar en un solo lugar y no necesitan comentarios.

2. Falta de tipos: PHP 8 no es excusa para la tipificación dinámica

PHP es un lenguaje de tipado débil, pero eso no significa que debas escribir como en 2005. La tipificación estricta evita errores difíciles de detectar.

Ejemplo malo

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}

Observaciones del senior

\n
• ¿Qué es $items? ¿Un array? ¿Un objeto? ¿Una colección?
\n
• ¿Qué estructura tiene el elemento? ¿Por qué no hay un DTO?
\n
• ¿Y si pasas $tax como cadena "0.2"? ¿Habrá concatenación?
\n

Ejemplo bueno

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}

El senior dirá: "Bien hecho, usas DTO, propiedades readonly y tipificación estricta. El código es predecible". Esto es una buena práctica de PHP 8+.

3. Anidamientos profundos y retorno temprano (Early Return)

Cuando ves 5 niveles de if...else dentro de un mismo método, el senior se agarra la cabeza. Ese código es imposible de leer, probar y depurar.

Ejemplo malo

public function processOrder($order) {\n    if ($order->isPaid()) {\n        if ($order->hasStock()) {\n            if ($order->isShippable()) {\n                // 50 líneas de lógica\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    }\n