Smells de diseño:
cuando el código huele mal

¿Qué es un smell de diseño?

Un smell no es un bug. El código con un smell compila, pasa las pruebas y funciona correctamente hoy. El problema es lo que pasará mañana: será difícil de entender, costoso de cambiar y frágil ante nuevos requisitos.

Martin Fowler, en Refactoring (1999), describió los smells como "ciertas estructuras en el código que sugieren —a veces gritan— la posibilidad de refactorizar". No son reglas absolutas. Son señales que invitan a hacer una pregunta: ¿este diseño me va a costar caro cuando cambie algo?

Tip de lectura: Tocá, enfocá o pasá el mouse sobre las líneas marcadas para ver por qué huelen mal.

Smell 1 — Método largo

Un método que hace demasiadas cosas. El síntoma más visible es la longitud, pero el problema real es que el método tiene múltiples razones para cambiar. Si cambia la lógica de validación de stock, el método cambia. Si cambia el cálculo de descuentos, el mismo método cambia. Si cambia la verificación de dirección, de nuevo el mismo método.

Cada responsabilidad adicional es una razón adicional para tocar el código. Y cada vez que se toca código por una razón, se arriesga romper las otras responsabilidades que conviven ahí.

// Ejemplo ilustrativo — sistema de pedidos ficticio
class Order
{
    public function process(): void
    {
        // Validar cliente (15 líneas)
        if (!$this->customer) {
            throw new \Exception('Customer not found.');
        }
        // ... más validaciones del cliente

        // Validar stock ítem por ítem (20 líneas)
        foreach ($this->items as $item) { /* ... */ }

        // Calcular descuentos (25 líneas)
        $total = 0;
        // ... descuentos por volumen, vendor, membresía

        // Notificar (10 líneas)
        $this->sendConfirmationEmail();
        $this->sendSms();
    }
}

La señal práctica: si necesitás un comentario de sección ("// Validate customer", "// Calculate discounts") es porque el método tiene más de una responsabilidad y cada sección podría ser un método separado.

Smell 2 — Clase dios

Una clase que sabe demasiado y hace demasiado. Acumula responsabilidades que deberían pertenecer a otras clases. El síntoma más claro: tiene métodos que usan principalmente los datos de otras clases, no los propios.

En aplicaciones web, la variante más frecuente es el fat controller: un controller que además de coordinar peticiones HTTP, contiene lógica de negocio, reglas de autorización, transformaciones de datos y decisiones de persistencia.

// Ejemplo ilustrativo — controller con demasiada responsabilidad
class OrderController extends Controller
{
    public function index(Request $request)
    {
        $user = $request->user();

        if ($user->role === 'customer') {
            $orders = Order::where('customer_id', $user->customer->id)->get();
        } elseif ($user->role === 'seller') {
            $orders = Order::where('seller_id', $user->seller->id)->get();
        } elseif ($user->role === 'courier') {
            $orders = Order::where('courier_id', $user->courier->id)->get();
        } elseif ($user->role === 'admin') {
            $orders = Order::paginate(20);
        }

        return response()->json($orders);
    }
}

El problema no es el largo: son cuatro if/elseif, nada dramático. El problema es que agregar un nuevo rol requiere modificar el controller. Esa lógica de "quién ve qué" debería vivir en otro lugar.

Smell 3 — Switch de estados

Cuando el comportamiento de un objeto depende de su estado y ese comportamiento se implementa con un switch o cadena de if/elseif, el código tiende a crecer de formas difíciles de controlar: el mismo switch aparece duplicado en varios métodos, y agregar un estado nuevo obliga a revisar todas las ramas de todos los switches.

// Ejemplo ilustrativo — máquina de estados con switch
class Order {
    public function transitionTo(string $next): void {
        switch ($this->status) {
            case 'created':
                if (!in_array($next, ['paid', 'cancelled'])) {
                    throw new \Exception("Invalid transition");
                }
                break;
            case 'paid':
                // ... más reglas
        }
        $this->status = $next;
    }

    public function canBeEdited(): bool {
        switch ($this->status) {
            case 'created':  return true;
            case 'paid':     return false;
        }
    }
}

La señal: el mismo switch aparece en dos métodos. Cuando llegue el estado 'disputed', habrá que actualizar ambos. Y probablemente haya un tercero en algún otro lugar del sistema.

Smell 4 — Envidia de características

Ocurre cuando un método de una clase usa intensamente los datos o métodos de otra clase, más que los propios. Es una señal de que ese método quizás pertenece a la clase que tanto "envidia".

// Ejemplo ilustrativo — procesador de pagos con Feature Envy
class PaymentController extends Controller {
    public function process(Request $request, Order $order) {
        $provider = $request->provider;

        if ($provider === 'providerA') {
            $handler = new ProviderAHandler();
            $result = $handler->charge($order->total, 'USD', [
                'reference'   => "ORDER-{$order->id}",
                'description' => "Payment for order #{$order->id}",
            ]);
            $success = $result['state'] === 'APPROVED';
        } elseif ($provider === 'providerB') {
            $handler = new ProviderBHandler();
            $result = $handler->makePayment([
                'amount'   => (int) ($order->total * 100),
                'currency' => 'USD',
            ]);
            $success = $result['status'] === 'success';
        }
    }
}

El controller sabe que ProviderA devuelve 'state' y ProviderB devuelve 'status'. Ese conocimiento no le pertenece. Si ProviderA cambia su API, el controller cambia. Si se agrega un tercer proveedor, el controller crece.

Smell 5 — Intimidad inapropiada

Ocurre cuando una clase conoce demasiados detalles internos de otra. El síntoma concreto: para entender o cambiar una clase, tenés que abrir otra. Las dos clases están acopladas por conocimiento, no por colaboración.

La variante más frecuente en aplicaciones web es instanciar dependencias con new directamente dentro de un método de negocio. La clase que llama sabe exactamente qué clase concreta usar, cómo construirla y cómo interpretar su respuesta interna.

// Del legacy del curso — sistema de pedidos ESEN
class Customer
{
    public function placeOrder(Order $order): void
    {
        $email = new EmailService();
        $sms   = new SMSService();

        $wompi  = new WompiGateway();
        $result = $wompi->charge($order->total());

        if ($result['estado'] !== 'APROBADO') {
            throw new PaymentException('Pago rechazado');
        }

        $order->confirm($result['id_transaccion']);
        $email->sendConfirmation($order);
        $sms->notify($order);
    }
}

Customer::placeOrder() conoce el nombre del campo de estado de Wompi, el nombre del campo de transacción, y cuáles son los dos servicios de notificación que existen. Ese conocimiento no le pertenece. Si Wompi actualiza su API, si se agrega WhatsApp como canal, o si se quiere testear el flujo sin cobrar realmente — en los tres casos hay que abrir Customer.

Smell 6 — Obsesión por primitivos

Consiste en usar tipos primitivos —string, int, bool— para representar conceptos del dominio que merecen su propio tipo. El síntoma más frecuente: un valor como el status de una orden viaja por todo el sistema como una cadena de texto sin ninguna garantía de que sea válida.

El costo concreto: cualquier parte del código puede inventar un valor ('delivered', 'Delivered', 'shipped') y el sistema no tiene forma de detectar el error hasta que algo explota en tiempo de ejecución, quizás en producción.

// Problema: status viaja como string suelto por todo el sistema
class Order {
    public string $status;

    public function isDelivered(): bool {
        return $this->status === 'delivered';
    }
}

// Cada desarrollador escribe el string a mano:
$order->status = 'cancelled';
$order->status = 'Cancelled';
$order->status = 'shipped';

// El filtro falla silenciosamente:
$cancelled = Order::where('status', 'cancelled')->get();

La solución clásica es un Value Object o, en PHP moderno, un enum: un tipo que solo puede tomar los valores que el dominio permite. El compilador —o el motor de PHP— rechaza cualquier otro valor antes de que llegue a producción.

Para discutir el martes

No hay nada que entregar. Llévalas pensadas — serán el punto de partida de la discusión del martes.

1. ¿Cuál de los seis smells te parece más difícil de detectar en código que no escribiste vos? ¿Por qué ese y no los otros?
2. En el ejemplo de Intimidad inapropiada, Customer::placeOrder() lee $result['estado'] y $result['id_transaccion'] directamente. Si Wompi cambia esos nombres en su API, ¿cuántas clases del sistema tendrían que cambiar? ¿Qué abstracción introduciría para aislar ese conocimiento?
3. Pensá en un proyecto propio o de trabajo. ¿Reconocés alguno de estos smells en ese código? ¿Cuál fue el costo concreto que tuvo —o que anticipás que tendrá— ese smell?
4. ¿Es siempre malo un método largo? Argumentá con un ejemplo donde un método de 60 líneas sea aceptable y otro donde 20 líneas sean demasiadas.
5. ¿Qué relación ves entre la Obsesión por primitivos y el Switch de estados? ¿Podría un smell llevar al otro?

Para ir más lejos

No es lectura obligatoria, pero si tenés tiempo:

• Fowler, M. (1999). Refactoring: Improving the Design of Existing Code. Capítulo 3: "Bad Smells in Code" — el catálogo original de 22 smells.
• Misko Hevery (2008). "Singletons are Pathological Liars" — blog post del equipo de testing de Google. Lo veremos como lectura obligatoria más adelante.