Apuntes · Capítulo 13Diseño

Principios de diseño y refactoring

Principios de diseño y refactoring

Hasta la semana 12 aprendimos herramientas: clases, herencia, interfaces, excepciones, genéricos, colecciones, lambdas y streams. Con ellas se puede escribir casi cualquier programa. Pero dos programas que hacen exactamente lo mismo pueden ser muy distintos el día en que hay que cambiarlos: en uno, el cambio toca una clase; en el otro, diez, y rompe tres cosas que nadie esperaba. Esta semana empieza la última parte del curso, diseño, y la pregunta es justamente esa: ¿qué hace que un programa sea fácil de cambiar?

El capítulo tiene dos partes. En la primera vemos dos medidas de un diseño, la cohesión y el acoplamiento, y los cinco principios SOLID, cada uno con un ejemplo pequeño y con una advertencia sobre cuándo no aplicarlo. En la segunda tomamos un programa que funciona y tiene pruebas, pero está mal diseñado, y lo refactorizamos en cinco pasos pequeños, corriendo las mismas pruebas después de cada uno. Al final comparamos el antes y el después. Es lo que les toca hacer en P4 con su propio proyecto.

Lectura. Eck no trata SOLID ni refactoring. De Eck, relean la sección 4.1 Black Boxes and Procedural Abstraction: la idea de interfaz e implementación es la base del bajo acoplamiento. Para la primera parte, Robert C. Martin, The Single Responsibility Principle, The Open Closed Principle y Solid Relevance. Para la segunda, Martin Fowler, Definition Of Refactoring, Code Smell y las entradas del catálogo de refactoring que se citan en cada paso. El código del capítulo está en code/week-13 y se prueba con make test dentro del contenedor del curso (env/run.sh).

Ruta de lectura.

  • Antes de clase (obligatorio, 15–20 min): ¿Qué es un buen diseño?, con Cohesión y Acoplamiento, y Qué es y qué no de Refactoring.
  • En clase: en la sesión A, Los principios SOLID, de S: responsabilidad única a Cuándo no aplicar SOLID; en la sesión B, Code smells y el ejemplo del mostrador de la biblioteca, de Paso 0: characterization tests a Paso 5: Replace Conditional with Polymorphism.
  • Después de clase: Antes y después, Mezclar refactoring con cambios y Cuándo refactorizar, el Resumen y los Ejercicios. Para ampliar, las lecturas de la nota Lectura.

¿Qué es un buen diseño?

Un programa que funciona hoy y no va a cambiar nunca puede estar escrito de cualquier manera. Ese programa casi no existe. Bodega empezó en la semana 1 como una clase Product y ha cambiado en cada entrega: precios por peso, descuentos, ventas, excepciones, un Map en lugar de una lista. Cada cambio fue más fácil o más difícil según cómo estaba repartido el código que ya había.

Por eso, en este curso, diseño quiere decir: cómo se reparten las responsabilidades entre las clases y cómo dependen unas de otras. Un buen diseño hace que los cambios probables sean baratos. No hay un diseño correcto en abstracto; hay diseños que aguantan mejor o peor los cambios que de verdad llegan.

Cohesión

La cohesión (cohesion) mide cuánto pertenece junto lo que está dentro de una clase. Una clase es cohesiva cuando todos sus campos y métodos sirven a una misma idea: TransitCard (semana 1) guarda un saldo y ofrece las operaciones sobre ese saldo; nada sobra.

Una clase con baja cohesión mezcla ideas que no tienen que ver entre sí. Una señal práctica: si se pueden separar los métodos en dos grupos, y los del primer grupo nunca usan los campos que usan los del segundo, en realidad hay dos clases metidas en una. Otra señal: el nombre. Las clases que terminan en Manager, Utils o Helper suelen ser cajones donde cae lo que no se supo dónde poner.

Acoplamiento

El acoplamiento (coupling) mide cuánto tiene que saber una clase de otra para funcionar. Si A lee los campos de B, conoce su clase concreta, la crea con new y depende del orden en que B hace las cosas, cualquier cambio en B puede romper A. Si A solo conoce una interfaz pequeña de B, B puede cambiar por dentro todo lo que quiera.

Algo de acoplamiento es inevitable: si dos clases colaboran, una sabe algo de la otra. La pregunta es cuánto y de qué tipo. Depender de una interfaz es menos acoplamiento que depender de una clase concreta; depender de un método público es menos que depender de un campo.

Idea clave. El objetivo es alta cohesión y bajo acoplamiento: que lo que cambia junto esté junto, y que lo que cambia por separado sepa lo menos posible de lo demás. Es la misma idea de la caja negra de Eck (sección 4.1): una interfaz clara por fuera y libertad para cambiar por dentro. Los principios que siguen son formas más concretas de decir esto.

Los principios SOLID

SOLID es un acrónimo que reúne cinco principios de diseño orientado a objetos. Los reunió y popularizó Robert C. Martin a comienzos de los 2000 (el acrónimo lo propuso Michael Feathers), aunque varios son más antiguos: el de Liskov es de 1987. No son leyes: son heurísticas que ayudan a reconocer un diseño que va a doler, y a decidir cómo repartir el código. Vamos uno por uno.

S: responsabilidad única

Una clase debe tener una sola razón para cambiar.

Ese es el principio de responsabilidad única (single responsibility principle). La parte importante es razón para cambiar. Martin lo precisa en su artículo: las razones para cambiar son personas o grupos que piden cambios. Si dos áreas distintas de una organización pueden pedir cambios en la misma clase, esa clase tiene dos responsabilidades, y un cambio pedido por una puede romper lo que necesita la otra.

Un ejemplo: invoice/before/Invoice.java, una factura que guarda sus líneas, calcula el total con IGV, arma el texto e imprime. Los precios van en céntimos, como siempre:

public class Invoice {
    private record Line(String description, int quantity, int unitPrice) {
    }

    private final String customer;
    private final List<Line> lines = new ArrayList<>();

    // ... constructor y add(...) ...

    // Razón 1: las reglas del negocio (el IGV es 18 %).
    public int total() {
        int subtotal = 0;
        for (Line line : lines) {
            subtotal += line.quantity() * line.unitPrice();
        }
        return subtotal + (subtotal * 18 + 50) / 100;
    }

    // Razón 2: el diseño del documento.
    public String format() {
        StringBuilder text = new StringBuilder("Factura para " + customer + "\n");
        for (Line line : lines) {
            int amount = line.quantity() * line.unitPrice();
            text.append(String.format("  %2d x %-12s S/ %d.%02d\n",
                    line.quantity(), line.description(), amount / 100, amount % 100));
        }
        int total = total();
        text.append(String.format("Total con IGV: S/ %d.%02d\n", total / 100, total % 100));
        return text.toString();
    }

    // Razón 3: a dónde va la factura.
    public void print() {
        System.out.print(format());
    }
}

La clase es corta y se entiende. El problema es quién va a pedir cambios. El contador pide cambios en el cálculo cuando cambia una tasa. Quien diseña los documentos quiere otro formato, con el RUC arriba y las columnas alineadas. Operaciones quiere que las facturas se envíen por correo en lugar de imprimirse. Tres personas distintas, tres razones, una sola clase. Y no son cambios independientes: si el diseñador cambia format para que el total se muestre redondeado, puede terminar llamando a total() de otra forma y el contador ve un error en un cálculo que nadie tocó a propósito.

A la izquierda, tres razones para cambiar: contabilidad (cambia el IGV), diseño (cambia el formato) y operaciones (se envía por correo). Cada una apunta a un método dentro de una sola caja Invoice: total(), format() y print(). A la derecha, cada método se convierte con Extract Class en su propia clase: Invoice (líneas, subtotal, IGV, total), InvoiceFormatter (arma el texto) e InvoiceService (formatea y envía por un Channel)A la izquierda, tres razones para cambiar: contabilidad (cambia el IGV), diseño (cambia el formato) y operaciones (se envía por correo). Cada una apunta a un método dentro de una sola caja Invoice: total(), format() y print(). A la derecha, cada método se convierte con Extract Class en su propia clase: Invoice (líneas, subtotal, IGV, total), InvoiceFormatter (arma el texto) e InvoiceService (formatea y envía por un Channel)
Antes, tres razones para cambiar llegan a la misma clase. Después, cada razón toca una sola clase.

La versión de invoice/after parte la clase en tres. Invoice se queda con los datos y las reglas del cálculo, y ya no sabe nada de texto:

public class Invoice {
    public record Line(String description, int quantity, int unitPrice) {
        public int amount() {
            return quantity * unitPrice;
        }
    }

    private static final int TAX_PERCENT = 18;   // IGV
    // ... customer, lines, constructor, add(...), customer(), lines() ...

    public int subtotal() { ... }

    public int tax() {
        return (subtotal() * TAX_PERCENT + 50) / 100;   // redondeo al céntimo
    }

    public int total() {
        return subtotal() + tax();
    }
}

InvoiceFormatter arma el texto a partir de lo que Invoice ofrece, e InvoiceService junta las dos cosas y envía el documento por un canal (lo vemos con la D, más abajo). Ahora cada pedido de cambio tiene una dirección: el IGV, a Invoice; el diseño, a InvoiceFormatter; el canal, a otra clase que implemente Channel.

Error común. Leer “una sola responsabilidad” como “un solo método” o “una sola cosa pequeña”. Invoice después del cambio tiene seis métodos públicos y está bien: todos sirven a la misma razón para cambiar. Partir una clase cohesiva en cinco clases de un método no la mejora; reparte una idea en cinco archivos.

O: abierto/cerrado

Un módulo debe estar abierto a la extensión y cerrado a la modificación.

El principio abierto/cerrado (open/closed principle) dice que agregar comportamiento nuevo no debería obligar a editar código que ya funciona. La forma de lograrlo en Java ya la conocen: el polimorfismo de la semana 6. Si el precio de un producto lo calcula cada subclase, agregar un tipo de producto es escribir una clase nueva; el código que vende productos no se toca.

Lo contrario es un switch sobre el tipo que se repite por el programa:

int fine = switch (member.type()) {
    case STUDENT -> ...;
    case TEACHER -> ...;
    case VISITOR -> ...;
};

Un tipo de socio nuevo obliga a encontrar y editar cada uno de esos switch. En la segunda parte del capítulo eliminamos un switch así con Replace Conditional with Polymorphism.

Pero cuidado con la conclusión fácil de que “todo switch es malo”. En la semana 8 vimos el switch sobre un tipo sealed, que es exhaustivo: si se agrega un caso, el compilador marca cada switch que no lo cubre. Las dos formas están cerradas en direcciones opuestas:

Diseño Agregar un tipo nuevo Agregar una operación nueva
Jerarquía en la que cada subclase hace override (semana 6) Una clase nueva; no se toca nada Hay que editar todas las clases
sealed con switch exhaustivo (semana 8) Hay que editar cada switch (el compilador los señala) Un método nuevo con un switch; no se toca nada

Ningún diseño está cerrado a todo. Elegir uno es apostar por qué tipo de cambio va a llegar más seguido: en un catálogo de productos llegan tipos de producto nuevos; en un evaluador de expresiones (el Expr de la semana 8), los tipos de nodo son pocos y fijos, y lo que se agrega son operaciones.

L: sustitución de Liskov

Si S es subtipo de T, un programa que funciona con objetos T debe seguir funcionando cuando recibe objetos S.

El principio de sustitución de Liskov (Liskov substitution principle) lo estudiamos en la semana 7 (Barbara Liskov y Jeannette Wing lo formalizaron en A Behavioral Notion of Subtyping). Con lo de la semana 9 se puede decir con más precisión: un subtipo debe cumplir el contrato del supertipo. No puede exigir más en sus precondiciones, ni garantizar menos en sus postcondiciones, ni lanzar excepciones que el supertipo no anuncia.

Un ejemplo con el Channel de la factura:

// Viola Liskov: Channel promete enviar, y este canal se niega.
class ArchiveChannel implements Channel {
    @Override
    public void send(String recipient, String text) {
        throw new UnsupportedOperationException("solo lectura");
    }
}

Compila, porque UnsupportedOperationException es unchecked. Pero InvoiceService se escribió confiando en que send envía, y con este canal falla en ejecución. El compilador verifica que los métodos existan con los tipos correctos; el contrato lo tiene que respetar quien escribe la clase.

Idea clave. implements y extends son una promesa de comportamiento, no solo de firmas. La señal más frecuente de que se rompió: un override que lanza UnsupportedOperationException, o código cliente que pregunta if (x instanceof Subtipo) para esquivar un caso especial.

I: segregación de interfaces

Ninguna clase debe estar obligada a depender de métodos que no usa.

El principio de segregación de interfaces (interface segregation principle) pide interfaces pequeñas y específicas. Una interfaz grande obliga a cada implementación a escribir todos sus métodos, incluso los que no tienen sentido para ella:

interface OfficeMachine {
    void print(String document);
    String scan();
    void fax(String number, String document);
}

class BasicPrinter implements OfficeMachine {
    public void print(String document) { ... }
    public String scan() { throw new UnsupportedOperationException(); }
    public void fax(String number, String document) { throw new UnsupportedOperationException(); }
}

BasicPrinter termina violando también el principio de Liskov: promete scan y no lo cumple. Además, todo el código que solo quiere imprimir depende de una interfaz que incluye el fax, y cada vez que cambia fax hay que recompilarlo y volver a revisarlo. La solución es partir la interfaz según quién la usa:

interface Printer { void print(String document); }
interface DocumentScanner { String scan(); }
interface Fax { void fax(String number, String document); }

class BasicPrinter implements Printer { ... }
class AllInOne implements Printer, DocumentScanner, Fax { ... }

Una clase puede implementar varias interfaces (semana 7), así que partir no le quita nada a la impresora multifunción. Las interfaces funcionales de la semana 12 son el caso extremo: Predicate, Function y Comparator tienen un solo método abstracto, y por eso cualquier lambda puede implementarlas.

D: inversión de dependencias

Los módulos de alto nivel no deben depender de los de bajo nivel. Ambos deben depender de abstracciones.

El principio de inversión de dependencias (dependency inversion principle) habla de la dirección de las flechas. Alto nivel es el código que expresa las reglas del programa: emitir una factura. Bajo nivel son los detalles: escribir en la consola, en un archivo o mandar un correo. Lo natural es escribir el alto nivel usando directamente el detalle:

public class InvoiceService {
    private final ConsoleChannel channel = new ConsoleChannel();   // el detalle, adentro
    ...
}

Así, InvoiceService depende de ConsoleChannel. Para enviar por correo hay que editarlo, y para probarlo no hay forma de evitar que escriba en la consola. La inversión consiste en que el alto nivel defina la abstracción que necesita, una interfaz Channel, y que los detalles la implementen:

public interface Channel {
    void send(String recipient, String text);
}

public class InvoiceService {
    private final InvoiceFormatter formatter;
    private final Channel channel;

    public InvoiceService(InvoiceFormatter formatter, Channel channel) {
        this.formatter = formatter;
        this.channel = channel;
    }

    public void issue(Invoice invoice) {
        channel.send(invoice.customer(), formatter.format(invoice));
    }
}
Animación en dos pasos. Paso 1: arriba, en el alto nivel, InvoiceService; abajo, en el bajo nivel, ConsoleChannel; una flecha baja de InvoiceService a ConsoleChannel con el texto new ConsoleChannel(). Paso 2: la flecha directa desaparece; en el alto nivel aparece la interfaz Channel, y InvoiceService la usa; ConsoleChannel y SpyChannel, abajo, la implementan con flechas punteadas que suben hacia ChannelAnimación en dos pasos. Paso 1: arriba, en el alto nivel, InvoiceService; abajo, en el bajo nivel, ConsoleChannel; una flecha baja de InvoiceService a ConsoleChannel con el texto new ConsoleChannel(). Paso 2: la flecha directa desaparece; en el alto nivel aparece la interfaz Channel, y InvoiceService la usa; ConsoleChannel y SpyChannel, abajo, la implementan con flechas punteadas que suben hacia Channel
La inversión: la interfaz pertenece al alto nivel, y la flecha que sale del detalle ahora apunta hacia arriba.

La palabra inversión se refiere a esa flecha: antes, el código de alto nivel apuntaba hacia el detalle; ahora, el detalle apunta hacia arriba, hacia una interfaz que pertenece al alto nivel. La dependencia entra por el constructor (inyección de dependencias por constructor, dependency injection), y alguien de afuera decide qué canal usar. En el programa, ese alguien es main, en InvoiceDemo.java:

Invoice invoice = new Invoice("alice");
invoice.add("cuaderno", 3, 450);
invoice.add("lapicero", 2, 150);

InvoiceService service = new InvoiceService(new InvoiceFormatter(), new ConsoleChannel());
service.issue(invoice);
$ java -cp build invoice.after.InvoiceDemo
[para alice]
Factura para alice
   3 x cuaderno     S/ 13.50
   2 x lapicero     S/ 3.00
Total con IGV: S/ 19.47

En la prueba, el canal es un test double: un spy escrito a mano que recuerda lo que le pidieron enviar, igual que en el laboratorio de la semana 7 (SpyChannel.java y InvoiceServiceTest.java):

class SpyChannel implements Channel {
    final List<String> recipients = new ArrayList<>();
    final List<String> texts = new ArrayList<>();

    @Override
    public void send(String recipient, String text) {
        recipients.add(recipient);
        texts.add(text);
    }
}

@Test
void issueSendsOneDocumentToTheCustomer() {
    SpyChannel spy = new SpyChannel();
    InvoiceService service = new InvoiceService(new InvoiceFormatter(), spy);

    service.issue(sample());

    assertEquals(List.of("alice"), spy.recipients);
    assertEquals(new InvoiceFormatter().format(sample()), spy.texts.get(0));
}

En Java. Hay frameworks, como Spring, que crean los objetos y les pasan sus dependencias automáticamente (contenedores de inversión de control; Martin Fowler los describe en Inversion of Control Containers and the Dependency Injection pattern). En este curso no los usamos: inyectamos a mano, por el constructor, y armamos las piezas en main. Es exactamente la misma idea, sin magia.

Fíjense en que InvoiceService ya no hace new de ningún canal, pero InvoiceDemo sí: el new no desaparece, se muda al borde del programa. No todo new es un problema. StringBuilder, ArrayList o un record de datos se crean donde se necesitan sin culpa; lo que conviene inyectar son los colaboradores que hablan con el mundo exterior (consola, archivos, red, reloj) o que se quieren reemplazar en una prueba.

Cuándo no aplicar SOLID

Los principios responden a un dolor concreto: un cambio que toca muchos lugares, una prueba que no se puede escribir. Sin ese dolor, aplicarlos solo agrega piezas.

Error común. Aplicar SOLID a ciegas: un programa de cien líneas con diez interfaces de un método, cada una con una sola implementación, una factory para cada clase y tres capas para sumar dos números. Cada interfaz es una indirección más que leer. Channel se justifica porque hay dos implementaciones reales (la consola y el spy de la prueba) y una tercera probable (el correo). Una interfaz con una sola implementación y sin pruebas que la necesiten, normalmente, sobra.

Kent Beck resumió el diseño simple en cuatro reglas, en orden de prioridad (Martin Fowler las comenta en Beck Design Rules): el código pasa las pruebas, revela su intención, no tiene duplicación y tiene el menor número de elementos. La última es el contrapeso de SOLID: si una clase o una interfaz no ayuda con ninguna de las tres primeras, se quita.

Refactoring

Qué es y qué no

Refactoring es cambiar la estructura interna del código sin cambiar su comportamiento observable, para que sea más fácil de entender y de modificar. La definición de Fowler tiene dos partes y las dos importan:

  • Sin cambiar el comportamiento. Después de refactorizar, el programa hace exactamente lo mismo, hasta en sus rarezas. Arreglar un error no es refactoring; es un cambio de funcionalidad (que también hay que hacer, pero aparte).
  • En pasos pequeños. Cada paso es tan chico que es difícil equivocarse, y el programa compila y pasa las pruebas después de cada uno.

¿Cómo se sabe que el comportamiento no cambió? Con pruebas. Las pruebas fijan lo que el programa hace; si siguen pasando después del cambio, el comportamiento que verifican se mantuvo. Sin pruebas no hay forma de saberlo: el cambio puede seguir siendo un refactoring, pero nadie lo comprobó, y es una actividad de alto riesgo. Por eso, antes de refactorizar código sin pruebas, se agregan characterization tests (los vemos en el paso 0).

El ciclo de trabajo es:

  1. Correr las pruebas: todo en verde.
  2. Hacer un cambio pequeño.
  3. Correr las pruebas.
  4. Si pasan, hacer commit con un mensaje que nombre el refactoring. Si fallan, deshacer el cambio (git restore .) e intentarlo en un paso más pequeño, en lugar de depurar.

Deshacer parece un desperdicio, pero el paso perdido es de dos minutos. Depurar un cambio grande que rompió algo, sin saber cuál de sus veinte partes fue, puede tomar una tarde.

Code smells

Un code smell es una señal en la superficie del código que suele indicar un problema de diseño más profundo. No es un error: el programa funciona. Es una pista de que el próximo cambio va a costar más de lo que debería. Los más frecuentes, con el refactoring del catálogo de Fowler que suele corresponder:

Code smell Cómo se ve Refactoring habitual
Método largo Un método que hay que leer con el dedo; comentarios que separan “partes” Extract Function
Clase grande Muchos campos, métodos que no se usan entre sí Extract Class
Código duplicado El mismo bloque, con variaciones mínimas, en dos lugares Extract Function y llamar desde ambos
Lista larga de parámetros register(a, b, c, d, e, f, g) Agrupar los datos en un record
Feature envy Un método que usa más los datos de otra clase que los suyos Move Function
switch repetido por tipo El mismo switch sobre el tipo en varios métodos Replace Conditional with Polymorphism
Nombres que no dicen nada d, tmp, data2, proc() Rename

El ejemplo: el mostrador de la biblioteca

Una biblioteca cobra multas por devolver tarde. Las reglas, en céntimos:

  • Cada día de atraso cuesta 50 por un libro y 100 por un DVD.
  • Un estudiante tiene 2 días de gracia y su multa por préstamo tiene un tope de S/ 10.00.
  • Un docente no paga por libros; por DVD paga la tarifa normal.
  • Un visitante paga el doble, sin gracia ni tope.

El programa arma el estado de cuenta de un socio a partir de sus préstamos devueltos. Los datos son records y enums sin comportamiento, y todo el trabajo lo hace library/step0/LibraryDesk.java:

public enum MemberType { STUDENT, TEACHER, VISITOR }
public enum ItemKind { BOOK, DVD }
public record Member(String name, MemberType type) { }
public record Loan(String title, ItemKind kind, LocalDate due, LocalDate returned) { }
public class LibraryDesk {

    public String statement(Member m, List<Loan> l) {
        String s = "Multas de " + m.name() + " (" + m.type() + ")\n";
        int t = 0;
        for (Loan x : l) {
            int d = (int) ChronoUnit.DAYS.between(x.due(), x.returned());
            if (d < 0) {
                d = 0;
            }
            int a = 0;
            switch (m.type()) {
                case STUDENT:
                    if (d > 2) {
                        a = (d - 2) * (x.kind() == ItemKind.DVD ? 100 : 50);
                    }
                    if (a > 1000) {
                        a = 1000;
                    }
                    break;
                case TEACHER:
                    if (x.kind() == ItemKind.DVD) {
                        a = d * 100;
                    }
                    break;
                case VISITOR:
                    a = d * (x.kind() == ItemKind.DVD ? 100 : 50) * 2;
                    break;
            }
            t += a;
            s += String.format("  %-18s %3d días  S/ %d.%02d\n", x.title(), d, a / 100, a % 100);
        }
        s += String.format("Total: S/ %d.%02d\n", t / 100, t % 100);
        return s;
    }

    public int totalFine(Member m, List<Loan> l) {
        int t = 0;
        for (Loan x : l) {
            // ... las mismas 25 líneas de arriba, copiadas, sin el format ...
            t += a;
        }
        return t;
    }
}

(ChronoUnit.DAYS.between cuenta los días entre dos fechas de java.time; da un número negativo si la segunda es anterior.)

El programa funciona: tiene clientes contentos y pruebas que pasan. Pero antes de seguir leyendo, hagan la lista de code smells. La nuestra:

  • Método largo: statement lee, calcula días, aplica reglas, formatea y suma, todo en 35 líneas.
  • Código duplicado: totalFine repite el cálculo completo. Si cambia el tope de los estudiantes, hay que acordarse de cambiarlo en dos lugares.
  • Nombres que no dicen nada: s, t, x, d, a, m, l.
  • Feature envy: el cálculo de los días de atraso solo usa datos de Loan, y la tarifa por día solo depende de ItemKind; los dos viven en LibraryDesk.
  • switch repetido por tipo: dos switch sobre MemberType, y uno más en cada método nuevo que necesite las reglas.
  • Clase grande (en pequeño): LibraryDesk calcula y da formato, dos razones para cambiar.

Además, el switch es del estilo antiguo, con case ...: y break, en lugar del switch con flechas de la semana 8. Es normal encontrarse código así: se escribió hace años, en otra versión de Java, y nadie se atrevió a tocarlo.

El objetivo es llegar a un diseño sin esos code smells sin cambiar el comportamiento, en cinco pasos. Cada paso es un paquete aparte en code/week-13/library/ (step0 a step5), para que se puedan comparar; en un proyecto real serían cinco commits sobre los mismos archivos.

Paso 0: characterization tests

Antes de tocar nada, hacen falta pruebas que fijen lo que el código hace hoy. Si el código no las tiene, se escriben primero: son los characterization tests. La técnica es mecánica: se llama al código con un caso, se mira qué devuelve y se escribe una prueba que exija exactamente eso, aunque parezca raro. La prueba no dice qué es correcto; dice qué hace el programa ahora.

library/step0/LibraryDeskTest.java tiene siete pruebas. Una fija el estado de cuenta completo de una estudiante:

private static final List<Loan> ALICE_LOANS = List.of(
        loan("Clean Code", ItemKind.BOOK, "2026-10-01", "2026-10-06"),
        loan("Matrix", ItemKind.DVD, "2026-10-03", "2026-10-04"),
        loan("SICP", ItemKind.BOOK, "2026-10-10", "2026-10-08"));

@Test
void studentStatement() {
    Member alice = new Member("alice", MemberType.STUDENT);
    String expected = """
            Multas de alice (STUDENT)
              Clean Code           5 días  S/ 1.50
              Matrix               1 días  S/ 0.00
              SICP                 0 días  S/ 0.00
            Total: S/ 1.50
            """;
    assertEquals(expected, desk.statement(alice, ALICE_LOANS));
}

Las otras cubren cada regla: el tope por préstamo de los estudiantes, que los docentes solo pagan DVD, que los visitantes pagan el doble sin gracia ni tope, un estado de cuenta vacío y que totalFine coincide con el total del estado de cuenta. Los valores esperados se copiaron de la salida real.

Hay una rareza: 1 días. Es un error (debería decir 1 día), y la prueba lo exige igual. Si un paso del refactoring lo “arreglara de paso”, la prueba fallaría, y con razón: ese paso habría cambiado el comportamiento. El error se corrige después, en su propio commit, cambiando a la vez el código y la prueba.

Idea clave. Los characterization tests apuntan a la interfaz pública que no va a cambiar: aquí, statement y totalFine, y los constructores de Member y Loan. Si las pruebas dependieran de métodos internos, cada refactoring las rompería y dejarían de servir como red. En los cinco pasos que siguen, el archivo de pruebas es el mismo (solo cambia la línea package), y el Makefile lo comprueba.

Con las pruebas en verde, empieza el refactoring.

Animación: a la izquierda, la lista de pasos del 0 al 5 (characterization tests, Rename, Extract Function, Move Function, Extract Class, Replace Conditional with Polymorphism); el paso actual se resalta en granate. A la derecha, una barra de make test que se llena hasta decir 7 de 7 pruebas pasan en cada paso, y tres medidas del código: las líneas de LibraryDesk bajan de 75 a 26, los switch por tipo de socio bajan de 2 a 0 y las líneas de producción en total suben de 99 a 121Animación: a la izquierda, la lista de pasos del 0 al 5 (characterization tests, Rename, Extract Function, Move Function, Extract Class, Replace Conditional with Polymorphism); el paso actual se resalta en granate. A la derecha, una barra de make test que se llena hasta decir 7 de 7 pruebas pasan en cada paso, y tres medidas del código: las líneas de LibraryDesk bajan de 75 a 26, los switch por tipo de socio bajan de 2 a 0 y las líneas de producción en total suben de 99 a 121
Cada paso aplica un refactoring y vuelve a correr las mismas pruebas. LibraryDesk se achica; el total de líneas sube un poco: refactorizar no es acortar.

Paso 1: Rename

El primer paso es el más barato y el que más ayuda a leer: Rename Variable. En step1:

s → text      t → total     x → loan     d → daysLate
a → fine      m → member    l → loans
for (Loan loan : loans) {
    int daysLate = (int) ChronoUnit.DAYS.between(loan.due(), loan.returned());
    if (daysLate < 0) {
        daysLate = 0;
    }
    int fine = 0;
    switch (member.type()) {
        case STUDENT:
            if (daysLate > 2) {
                fine = (daysLate - 2) * (loan.kind() == ItemKind.DVD ? 100 : 50);
            }
            ...

Ninguna línea cambió de lugar. Pero ahora fine = (daysLate - 2) * ... se lee como la regla de la biblioteca. Un editor como IntelliJ o VS Code hace el renombrado de forma segura (cambia solo esa variable, no todas las d del archivo); a mano, conviene renombrar de una variable a la vez.

$ make test
...
== pruebas de library.step1: 7 tests successful
$ git commit -am "Rename: nombres descriptivos en LibraryDesk"

Paso 2: Extract Function

Extract Function toma un fragmento de un método, lo convierte en un método con nombre y reemplaza el fragmento por una llamada. Aquí se aplica tres veces en step2: el cálculo de la multa de un préstamo, los días de atraso y el formato del dinero.

public String statement(Member member, List<Loan> loans) {
    String text = "Multas de " + member.name() + " (" + member.type() + ")\n";
    for (Loan loan : loans) {
        text += String.format("  %-18s %3d días  %s\n",
                loan.title(), daysLate(loan), money(fineFor(member, loan)));
    }
    return text + "Total: " + money(totalFine(member, loans)) + "\n";
}

public int totalFine(Member member, List<Loan> loans) {
    int total = 0;
    for (Loan loan : loans) {
        total += fineFor(member, loan);
    }
    return total;
}

private int fineFor(Member member, Loan loan) { ... el switch, una sola vez ... }

private int daysLate(Loan loan) {
    int days = (int) ChronoUnit.DAYS.between(loan.due(), loan.returned());
    return Math.max(days, 0);
}

private static String money(int cents) {
    return String.format("S/ %d.%02d", cents / 100, cents % 100);
}

Con fineFor extraído, totalFine ya no necesita su copia: el código duplicado desapareció y queda un solo switch. Y statement pasa a usar totalFine para el total. LibraryDesk bajó de 75 a 59 líneas.

En Java. Math.max(days, 0) reemplaza al if (d < 0) d = 0; del original. ¿Es el mismo comportamiento? Para todo int, sí. Este tipo de reemplazo “obvio” es el que más conviene hacer con las pruebas corriendo: lo obvio a veces no lo es (por ejemplo, con double y NaN).

Paso 3: Move Function

Ahora miremos daysLate(Loan loan): no usa ningún campo de LibraryDesk, solo datos de Loan. Es feature envy: el método vive en una clase y trabaja con los datos de otra. Move Function lo muda a donde están sus datos. En step3, el record gana un método:

public record Loan(String title, ItemKind kind, LocalDate due, LocalDate returned) {

    /** Días de atraso; 0 si se devolvió a tiempo o antes. */
    public int daysLate() {
        int days = (int) ChronoUnit.DAYS.between(due, returned);
        return Math.max(days, 0);
    }
}

Lo mismo con la tarifa por día, que estaba escrita tres veces como loan.kind() == ItemKind.DVD ? 100 : 50. Es un dato de cada tipo de ítem, así que pasa a ser un campo del enum (ItemKind.java):

public enum ItemKind {
    BOOK(50), DVD(100);

    private final int dailyRate;

    ItemKind(int dailyRate) {
        this.dailyRate = dailyRate;
    }

    public int dailyRate() {
        return dailyRate;
    }
}

Y la multa, que decide según el tipo de socio, se muda a MemberType; de paso, el switch antiguo se convierte en un switch con flechas, que el compilador verifica exhaustivo (ya no hace falta un default):

public enum MemberType {
    STUDENT, TEACHER, VISITOR;

    int fineFor(Loan loan) {
        int daysLate = loan.daysLate();
        int rate = loan.kind().dailyRate();
        return switch (this) {
            case STUDENT -> Math.min(Math.max(daysLate - 2, 0) * rate, 1000);
            case TEACHER -> loan.kind() == ItemKind.DVD ? daysLate * rate : 0;
            case VISITOR -> daysLate * rate * 2;
        };
    }
}

Member ofrece fineFor(loan) y delega en su tipo, para que LibraryDesk pida la multa al socio (member.fineFor(loan)) y no tenga que meterse en sus partes (member.type().fineFor(loan)). LibraryDesk se queda en 30 líneas, sin ningún cálculo propio:

public String statement(Member member, List<Loan> loans) {
    String text = "Multas de " + member.name() + " (" + member.type() + ")\n";
    for (Loan loan : loans) {
        text += String.format("  %-18s %3d días  %s\n",
                loan.title(), loan.daysLate(), money(member.fineFor(loan)));
    }
    return text + "Total: " + money(totalFine(member, loans)) + "\n";
}

Este paso movió tres cosas. En un proyecto real serían tres commits, uno por movimiento, con make test entre cada uno; aquí van juntos para no tener siete paquetes. Fíjense en una línea en particular: Math.min(Math.max(daysLate - 2, 0) * rate, 1000) reemplaza a dos if. Es justo el tipo de reescritura donde es fácil equivocarse, y es la prueba studentFineIsCappedPerLoan la que confirma que el tope sigue funcionando.

Paso 4: Extract Class

LibraryDesk todavía tiene dos razones para cambiar: qué se cobra y cómo se ve el estado de cuenta. Extract Class saca la segunda a su propia clase, StatementFormatter:

class StatementFormatter {

    String header(Member member) {
        return "Multas de " + member.name() + " (" + member.type() + ")\n";
    }

    String line(Loan loan, int fine) {
        return String.format("  %-18s %3d días  %s\n", loan.title(), loan.daysLate(), money(fine));
    }

    String footer(int total) {
        return "Total: " + money(total) + "\n";
    }

    static String money(int cents) {
        return String.format("S/ %d.%02d", cents / 100, cents % 100);
    }
}
public class LibraryDesk {

    private final StatementFormatter formatter = new StatementFormatter();

    public String statement(Member member, List<Loan> loans) {
        StringBuilder text = new StringBuilder(formatter.header(member));
        for (Loan loan : loans) {
            text.append(formatter.line(loan, member.fineFor(loan)));
        }
        text.append(formatter.footer(totalFine(member, loans)));
        return text.toString();
    }
    // totalFine igual que antes
}

StatementFormatter no es pública: es un detalle del paquete. Y LibraryDesk la crea con new, sin inyectarla. ¿Contradice eso la inversión de dependencias? No: el formato no habla con el mundo exterior ni hace falta reemplazarlo en una prueba. Si un día hubiera dos formatos (texto y HTML), sería el momento de una interfaz y de pasarla por el constructor; hoy sería una pieza de más.

Paso 5: Replace Conditional with Polymorphism

Queda un switch sobre el tipo de socio. Hoy es uno solo, pero cada regla nueva que dependa del tipo (cuántos préstamos puede tener a la vez, cuántos días dura un préstamo) agregaría otro. Replace Conditional with Polymorphism reemplaza el switch por un método al que cada caso le hace override.

La forma clásica sería una interfaz MemberType con una clase por tipo de socio. Pero eso cambiaría la interfaz pública (MemberType.STUDENT dejaría de existir) y con ella las pruebas. Los enums de Java permiten algo más pequeño: cada constante puede tener su propio cuerpo de clase y hacer override de un método abstracto del enum (JLS §8.9.1). Así queda step5/MemberType.java:

public enum MemberType {
    STUDENT {
        @Override
        int fineFor(Loan loan) {
            int charged = Math.max(loan.daysLate() - GRACE_DAYS, 0);
            return Math.min(charged * loan.kind().dailyRate(), CAP);
        }
    },
    TEACHER {
        @Override
        int fineFor(Loan loan) {
            return loan.kind() == ItemKind.DVD ? loan.daysLate() * loan.kind().dailyRate() : 0;
        }
    },
    VISITOR {
        @Override
        int fineFor(Loan loan) {
            return loan.daysLate() * loan.kind().dailyRate() * 2;
        }
    };

    private static final int GRACE_DAYS = 2;
    private static final int CAP = 1000;     // tope por préstamo, en céntimos

    /** Multa de un préstamo para este tipo de socio, en céntimos. */
    abstract int fineFor(Loan loan);
}

Cada constante es, por dentro, una subclase anónima del enum, y member.type().fineFor(loan) se resuelve con el dynamic dispatch de la semana 6. Ya no hay switch: la regla de cada tipo de socio está junto a su nombre, y un tipo nuevo es una constante nueva con su método; si se olvida el método, el compilador se queja porque fineFor es abstracto. De paso, los números mágicos 2 y 1000 tienen nombre.

Idea clave. El paso 5 se pudo hacer sin tocar las pruebas porque eligió una forma de polimorfismo que respeta la interfaz pública. Cuando se refactoriza, la interfaz pública es un límite: lo de adentro se cambia con libertad; lo de afuera, solo como un cambio aparte y a propósito.

Antes y después

Diagrama de clases. A la izquierda, antes (step0): una caja LibraryDesk con statement(m, l), que lee, calcula, formatea y suma, y totalFine(m, l), que hace lo mismo copiado; debajo, la nota 2 switch por tipo de socio, y cuatro cajas grises de solo datos: los records Member (name, type) y Loan (title, kind, due, returned) y los enums MemberType y ItemKind. A la derecha, después (step5): LibraryDesk con statement() y totalFine() usa a StatementFormatter (header, line, footer) y a Member (fineFor); Member usa a MemberType, en dorado, con fineFor abstracto, que a su vez usa a Loan (daysLate); Loan usa a ItemKind (dailyRate). Debajo de MemberType, tres cajas STUDENT, TEACHER y VISITOR: cada constante hace override de fineForDiagrama de clases. A la izquierda, antes (step0): una caja LibraryDesk con statement(m, l), que lee, calcula, formatea y suma, y totalFine(m, l), que hace lo mismo copiado; debajo, la nota 2 switch por tipo de socio, y cuatro cajas grises de solo datos: los records Member (name, type) y Loan (title, kind, due, returned) y los enums MemberType y ItemKind. A la derecha, después (step5): LibraryDesk con statement() y totalFine() usa a StatementFormatter (header, line, footer) y a Member (fineFor); Member usa a MemberType, en dorado, con fineFor abstracto, que a su vez usa a Loan (daysLate); Loan usa a ItemKind (dailyRate). Debajo de MemberType, tres cajas STUDENT, TEACHER y VISITOR: cada constante hace override de fineFor
Antes, una clase hace todo y las demás son solo datos. Después, cada cálculo vive con los datos que usa, el formato tiene su clase y no queda ningún switch por tipo.

Las clases son las mismas, más una. Lo que cambió es dónde está cada decisión:

Pregunta Antes (step0) Después (step5)
¿Cuántos días de atraso? LibraryDesk, dos veces Loan.daysLate()
¿Cuánto cuesta un día de DVD? Un ? 100 : 50 en tres lugares ItemKind.dailyRate()
¿Qué regla tiene un estudiante? Un case en dos switch La constante STUDENT
¿Cómo se ve el estado de cuenta? Mezclado con los cálculos StatementFormatter
¿Qué hace LibraryDesk? Todo (75 líneas) Coordina (26 líneas)

El código de producción en total pasó de 99 a 121 líneas. Refactorizar no es acortar: hay más métodos, cada uno con su nombre y su comentario. Lo que se redujo es lo que hay que leer para hacer un cambio: para agregar un tipo de socio, antes había que entender LibraryDesk entera; ahora basta con MemberType.

LibraryDemo.java imprime el mismo estado de cuenta con el código de antes y el de después:

$ java -cp build library.LibraryDemo
== step0
Multas de alice (STUDENT)
  Clean Code           5 días  S/ 1.50
  Matrix              30 días  S/ 10.00
Total: S/ 11.50
== step5
Multas de alice (STUDENT)
  Clean Code           5 días  S/ 1.50
  Matrix              30 días  S/ 10.00
Total: S/ 11.50

Treinta días de atraso de un DVD serían S/ 28.00 para una estudiante (28 días cobrables), pero el tope los deja en S/ 10.00, antes y después.

Mezclar refactoring con cambios

Error común. Aprovechar un paso de refactoring para cambiar también algo de la funcionalidad. Supongamos que, mientras mudan la multa a MemberType en el paso 3, alguien recuerda que la biblioteca quería subir el tope de los estudiantes a S/ 15.00 y cambia 1000 por 1500 en la misma línea. Las pruebas avisan:

│  └─ LibraryDeskTest ✔
│     ├─ studentFineIsCappedPerLoan() ✘ expected: <1100> but was: <1600>
│     ├─ visitorPaysDoubleWithoutGrace() ✔
...
[         6 tests successful      ]
[         1 tests failed          ]

Pero ¿qué falló? ¿El movimiento a MemberType (por ejemplo, el Math.min(Math.max(...)) que reemplazó a dos if) o el tope nuevo? Con los dos cambios en el mismo paso no se puede saber sin depurar. Hecho en dos commits, la respuesta es inmediata: el refactoring pasa las pruebas sin tocarlas, y el cambio del tope rompe exactamente la prueba del tope, que se actualiza a propósito en ese mismo commit.

Kent Beck lo resumió en una frase de 2012 que se cita mucho: primero hagan que el cambio sea fácil (esto puede ser difícil), y después hagan el cambio fácil. Refactorizar es la primera mitad; nunca al mismo tiempo que la segunda.

Cuándo refactorizar

  • Antes de agregar algo, cuando el diseño actual hace que el cambio sea difícil. Es el momento más rentable: el refactoring tiene un propósito concreto.
  • Cuando se entiende algo que costó entender. Un buen nombre o un método extraído guarda esa comprensión para el siguiente que lea.
  • A la tercera repetición. La primera vez se escribe; la segunda, se tolera la copia; la tercera, se extrae. Antes de eso, todavía no se sabe qué partes son de verdad iguales.

Y cuándo no: cuando no hay pruebas (primero se escriben los characterization tests), cuando el código no se va a volver a tocar, y en medio de un cambio de funcionalidad a medio terminar.

El código de la semana y make test

El código está en code/week-13:

Carpeta Qué tiene
invoice/before La factura con tres responsabilidades
invoice/after Invoice, InvoiceFormatter, Channel, ConsoleChannel, InvoiceService, el spy y sus pruebas
library/step0 … step5 El mostrador de la biblioteca en cada paso del refactoring, cada uno con la misma LibraryDeskTest
library/LibraryDemo.java El mismo estado de cuenta antes y después
lab El código inicial del laboratorio

Como hay paquetes, el Makefile busca los .java con find y ejecuta las demostraciones por su nombre completo (library.LibraryDemo). Además, antes de correr las pruebas comprueba que las de step1 a step5 son idénticas a las de step0, salvo la línea package, y prueba cada paso por separado:

$ make test
mkdir -p build
javac -Xlint:all -Werror -cp /opt/junit/junit.jar -d build invoice/before/Invoice.java ...
touch build/.stamp
== las pruebas de step1..step5 son idénticas a las de step0
== invoice.after.InvoiceDemo
...
== pruebas de library.step0: 7 tests successful
== pruebas de library.step1: 7 tests successful
== pruebas de library.step2: 7 tests successful
== pruebas de library.step3: 7 tests successful
== pruebas de library.step4: 7 tests successful
== pruebas de library.step5: 7 tests successful
╷
├─ JUnit Platform Suite ✔
├─ JUnit Jupiter ✔
│  ├─ InvoiceServiceTest ✔
│  │  ├─ totalIncludesTax() ✔
│  │  ├─ formatterBuildsTheText() ✔
│  │  └─ issueSendsOneDocumentToTheCustomer() ✔
│  └─ GradeBookTest ✔
│     ├─ halfRoundsUp() ✔
│     ├─ report() ✔
│     └─ finalGradeBySystem() ✔
└─ JUnit Vintage ✔
...
[         6 tests successful      ]
[         0 tests failed          ]

El laboratorio parte de lab/GradeBook.java, un registro de notas que funciona, tiene pruebas y está mal diseñado a propósito. Encontrar sus code smells es el primer ejercicio del laboratorio, así que aquí no los listamos.

Resumen

  • Diseño es cómo se reparten las responsabilidades y cómo dependen las clases entre sí. Un buen diseño hace baratos los cambios probables.
  • Alta cohesión: lo que está en una clase pertenece junto. Bajo acoplamiento: cada clase sabe lo menos posible de las demás.
  • S: una clase, una razón para cambiar (Invoice partida en tres). O: agregar comportamiento con código nuevo, no editando un switch; ningún diseño está cerrado a todo. L: un subtipo cumple el contrato del supertipo. I: interfaces pequeñas, según quién las usa. D: el alto nivel define interfaces y los detalles las implementan; las dependencias entran por el constructor, y eso permite usar test doubles.
  • Los principios se aplican cuando hay un cambio o una prueba concreta que los pide; si no, agregan piezas.
  • Refactoring: cambiar la estructura sin cambiar el comportamiento, en pasos pequeños, con las pruebas en verde después de cada uno. Si un paso falla, se deshace.
  • Refactorizar sin pruebas es de alto riesgo: primero se escriben characterization tests contra la interfaz pública, que fijan lo que el código hace hoy, rarezas incluidas.
  • Code smells frecuentes y su refactoring: método largo (Extract Function), código duplicado (Extract Function), nombres pobres (Rename), feature envy (Move Function), clase con dos responsabilidades (Extract Class), switch por tipo (Replace Conditional with Polymorphism).
  • Nunca se mezcla un refactoring con un cambio de funcionalidad en el mismo paso.

Ejercicios

  1. Elijan una clase de su proyecto Bodega y escriban, para cada método público, quién podría pedir que cambie (contabilidad, el dueño de la tienda, quien diseña los reportes…). ¿Cuántas razones para cambiar tiene la clase? Si tiene más de una, propongan cómo partirla, sin escribir el código todavía.
  2. En invoice/after, el IGV pasa a depender del cliente: los clientes con exoneración no lo pagan. ¿Qué clases tienen que cambiar? ¿Y si lo que cambia es que el total se muestre también en dólares? Comparen con lo que habría que cambiar en invoice/before.
  3. Escriban una clase FileChannel implements Channel que agregue cada documento al final de un archivo de texto (con lo que vieron de archivos, o simplemente acumulándolos en un StringBuilder si todavía no lo vieron). ¿Qué clases existentes tuvieron que modificar para usarla en InvoiceDemo?
  4. Sin ejecutarlo, digan qué pasa si se pasa un ArchiveChannel (el que lanza UnsupportedOperationException) a InvoiceService y se llama a issue. ¿Lo detecta el compilador? ¿Qué principio se viola? Propongan un diseño de interfaces en el que un canal de solo lectura no tenga que mentir.
  5. Den un ejemplo, de su proyecto o inventado, en que aplicar el principio abierto/cerrado sería un exceso. ¿Qué cambio tendría que aparecer para que valiera la pena?
  6. En step0, ¿por qué statement y totalFine pueden llegar a dar totales distintos si alguien cambia solo uno de los dos? Escriban una prueba que lo detectaría. ¿Hay ya una así en LibraryDeskTest?
  7. Corrijan el error de 1 días en step5 (que diga 1 día) como un cambio de funcionalidad aparte: primero cambien la prueba, véanla fallar, y después cambien el código. ¿Qué pruebas tuvieron que cambiar y por qué?
  8. Agreguen un tipo de socio STAFF (personal de la biblioteca: no paga multas, salvo un DVD con más de 7 días de atraso, que paga la tarifa normal desde el primer día) en step0 y en step5, con pruebas. ¿Cuántos lugares tuvieron que editar en cada versión?
  9. En step3, cambien a propósito Math.max(daysLate - 2, 0) por daysLate - 2 y corran make test. ¿Qué prueba falla y con qué mensaje? ¿Qué caso de la vida real no cubre la versión con el error? Devuelvan el código a su estado original.
  10. Un compañero propone el paso 5 con una sealed interface MemberType permits Student, Teacher, Visitor y tres records. Escriban cómo cambiaría la creación de un socio en las pruebas. ¿Por qué ese cambio ya no es solo refactoring? ¿Cuándo valdría la pena hacerlo igual?
  11. Lean las cuatro reglas del diseño simple en Beck Design Rules. Para cada uno de los cinco pasos del refactoring de la biblioteca, digan qué regla lo justifica.
  12. Tomen un método de más de 30 líneas de su propio proyecto. Escriban primero characterization tests para él y después apliquen Extract Function al menos dos veces, corriendo las pruebas después de cada paso y haciendo un commit por paso. Entreguen el git log --oneline de esos commits.