Ejercicio: una clase desordenada, limpiada paso a paso

Código en:JavaC#PythonJavaScript

En resumen: Un ejercicio de refactoring: una clase de alquiler de patinetes se limpia en cinco pasos pequeños (nombres, números mágicos, dinero, consulta separada del comando, planes) mientras los tests siguen en verde.

Problema

Los patinetes que nadie se atreve a tocar

Una empresa que alquila patinetes eléctricos cobra cada viaje con la clase RideMgr. Funciona desde hace tres años. El método calc calcula el precio según el plan del cliente (pago por uso, estudiante o abono mensual), quita la tarifa de desbloqueo a partir del cuarto viaje del día, añade un recargo nocturno, suma los ingresos y envía un recibo:

class RideMgr {
    private final Map<String, Integer> c = new HashMap<>();
    private double s = 0;
    private final List<String> out = new ArrayList<>();

    // El precio: 1 € por desbloquear y 0,15 € por minuto.
    // De noche el precio sube un 50 %.
    public double calc(String to, String t, int m, boolean f) {
        c.put(to, c.getOrDefault(to, 0) + 1);
        double p = 0;
        if (t.equals("payg")) {
            p = 1.0 + m * 0.2;
        } else if (t.equals("student")) {
            p = (1.0 + m * 0.2) * 0.8;
        } else if (t.equals("monthly")) {
            if (m > 45) {
                p = (m - 45) * 0.2;
            }
        }
        if (c.get(to) > 3 && !t.equals("monthly")) {
            p = p - 1.0;
        }
        if (f) {
            p = p + 0.5;
        }
        s = s + p;
        out.add(to + ": " + m + " min, " + String.format("%.2f", p) + " €");
        return p;
    }

    public List<String> sent() {
        return out;
    }

    public double day() {
        return s;
    }
}

El mes que viene llegan dos cambios:

• un plan de empresa: sin tarifa de desbloqueo, 0,18 € por minuto, sin descuento en el cuarto viaje;
• el precio antes del final: la app muestra lo que costará el viaje mientras todavía está en curso.

El segundo no se puede hacer con esta clase. Esto es lo que pasa si la app llama a calc para mostrar el precio, y luego otra vez para cobrar:

RideMgr rides = new RideMgr();
double shown = rides.calc("ana@example.com", "payg", 10, false);
System.out.println("Precio antes del final: " + String.format("%.2f €", shown));
double charged = rides.calc("ana@example.com", "payg", 10, false);
System.out.println("Cobrado: " + String.format("%.2f €", charged));
System.out.println("Recibos: " + rides.sent().size());
System.out.println("Ingresos: " + String.format("%.2f €", rides.day()));

Salida

Precio antes del final: 3.00 €
Cobrado: 3.00 €
Recibos: 2
Ingresos: 6.00 €

Un viaje, dos recibos y 6 € de ingresos. calc suena a pregunta, pero cuenta, suma y envía.

En esta lección no hay ninguna idea nueva. Es un ejercicio: la clase pasa por las herramientas del módulo una por una, en pasos pequeños, y después de cada paso ejecutas los tests.

Míralo

Paso 0: la red de seguridad

Primero la red, como en la lección sobre tests. Nueve tests fijan lo que la clase hace hoy: el precio de cada plan, la noche, el cuarto viaje, el recibo y los ingresos.

import java.util.List;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;

class RideMgrTest {
    static final String EMAIL = "ana@example.com";

    RideMgr rides = new RideMgr();

    String ride(String plan, int minutes, boolean night) {
        return money(rides.calc(EMAIL, plan, minutes, night));
    }

    String money(double amount) {
        return String.format("%.2f", amount);
    }

    @Test
    void payAsYouGoRide() {
        assertEquals("3.00", ride("payg", 10, false));
    }

    @Test
    void studentRide() {
        assertEquals("2.40", ride("student", 10, false));
    }

    @Test
    void monthlyWithinFreeMinutes() {
        assertEquals("0.00", ride("monthly", 30, false));
    }

    @Test
    void monthlyOverFreeMinutes() {
        assertEquals("3.00", ride("monthly", 60, false));
    }

    @Test
    void nightSurcharge() {
        assertEquals("3.50", ride("payg", 10, true));
    }

    @Test
    void fourthRideUnlockIsFree() {
        ride("payg", 10, false);
        ride("payg", 10, false);
        ride("payg", 10, false);
        assertEquals("2.00", ride("payg", 10, false));
    }

    @Test
    void studentFourthRide() {
        // Así es hoy: el descuento es el 1 € entero, no 0,80 €.
        ride("student", 10, false);
        ride("student", 10, false);
        ride("student", 10, false);
        assertEquals("1.40", ride("student", 10, false));
    }

    @Test
    void receipt() {
        ride("payg", 10, false);
        assertEquals(List.of("ana@example.com: 10 min, 3.00 €"), rides.sent());
    }

    @Test
    void revenue() {
        ride("payg", 10, false);
        ride("student", 10, false);
        assertEquals("5.40", money(rides.day()));
    }
}

Resultado

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

Los tests llaman a la clase solo a través del auxiliar ride. Cuando la forma de la clase cambie en los próximos pasos, cambiarán el auxiliar y las llamadas, y nunca los valores esperados.

Uno de los tests es raro. Un estudiante paga el 80 % del precio, así que desbloquear le cuesta 0,80 €. En el cuarto viaje, sin embargo, la clase quita el 1 € entero, y el estudiante paga 1,40 € en vez de 1,60 €. ¿Es un error o un regalo? No lo decides tú, y no lo decides ahora. El test fija lo que pasa hoy, con un comentario, y la pregunta va al product owner. Mientras limpias, el comportamiento no cambia, aunque parezca equivocado.

Más precisamente, para curiosos

Los importes se comparan como texto con dos decimales ("2.40"), tal como los ve el cliente. Comparar números tropezaría con los pequeños errores binarios de double:

System.out.println((1.0 + 10 * 0.2) * 0.8);

Salida

2.4000000000000004

Y en el paso 3 cambiará el tipo de los importes. El texto con dos decimales es el mismo antes y después.

Míralo

Paso 1: los nombres y la bandera

t, m, f, p, c y s pasan a ser palabras (nombres). El nombre más importante es el del método: no calcula, termina el viaje. El nombre honesto es finishRide, y ahora el efecto secundario se ve en el nombre. La clase pasa a llamarse RideBilling.

class RideBilling {
    private final Map<String, Integer> ridesToday = new HashMap<>();
    private double revenue = 0;
    private final List<String> receipts = new ArrayList<>();

    // El precio: 1 € por desbloquear y 0,15 € por minuto.
    // De noche el precio sube un 50 %.
    public double finishRide(String email, String plan, int minutes, boolean night) {
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        double price = 0;
        if (plan.equals("payg")) {
            price = 1.0 + minutes * 0.2;
        } else if (plan.equals("student")) {
            price = (1.0 + minutes * 0.2) * 0.8;
        } else if (plan.equals("monthly")) {
            if (minutes > 45) {
                price = (minutes - 45) * 0.2;
            }
        }
        if (ridesToday.get(email) > 3 && !plan.equals("monthly")) {
            price = price - 1.0;
        }
        if (night) {
            price = price + 0.5;
        }
        revenue = revenue + price;
        receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public double revenue() {
        return revenue;
    }
}

La bandera se queda por ahora (funciones). Un true en la llamada no dice nada. La noche es un dato del viaje, no la elección de otro trabajo, así que no se convierte en dos funciones. La llamada tiene que decir lo que significa:

Java no tiene argumentos con nombre. En los tests los valores son constantes con nombre, DAY y NIGHT, y el auxiliar las pasa.

Antes

String ride(String plan, int minutes, boolean night) {
    return money(rides.calc(EMAIL, plan, minutes, night));
}

@Test
void nightSurcharge() {
    assertEquals("3.50", ride("payg", 10, true));
}

Después

static final boolean DAY = false;
static final boolean NIGHT = true;

String ride(String plan, int minutes, boolean night) {
    return money(billing.finishRide(EMAIL, plan, minutes, night));
}

@Test
void nightSurcharge() {
    assertEquals("3.50", ride("payg", 10, NIGHT));
}

Los tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

En IntelliJ IDEA, renombrar es Shift+F6: cambia también las llamadas de los tests.

Míralo

Paso 2: los números y el comentario que miente

1.0, 0.2, 0.8, 45, 3 y 0.5 pasan a ser constantes con nombre (número mágico → constante).

El comentario sobre el método dice 0,15 € por minuto y un 50 % de recargo nocturno. El código cobra 0,20 € y añade 0,50 €. ¿Qué es verdad? Los tests fijaron lo que hace el código, y los clientes llevan años pagando según él. El comentario miente (comentarios) y se borra. Las constantes dicen lo mismo y no pueden quedarse anticuadas por sí solas.

class RideBilling {
    private static final double UNLOCK_FEE = 1.00;
    private static final double PER_MINUTE = 0.20;
    private static final double STUDENT_SHARE = 0.80;
    private static final int MONTHLY_FREE_MINUTES = 45;
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final double NIGHT_SURCHARGE = 0.50;

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private double revenue = 0;
    private final List<String> receipts = new ArrayList<>();

    public double finishRide(String email, String plan, int minutes, boolean night) {
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        double price = 0;
        if (plan.equals("payg")) {
            price = UNLOCK_FEE + minutes * PER_MINUTE;
        } else if (plan.equals("student")) {
            price = (UNLOCK_FEE + minutes * PER_MINUTE) * STUDENT_SHARE;
        } else if (plan.equals("monthly")) {
            if (minutes > MONTHLY_FREE_MINUTES) {
                price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE;
            }
        }
        if (ridesToday.get(email) > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
            price = price - UNLOCK_FEE;
        }
        if (night) {
            price = price + NIGHT_SURCHARGE;
        }
        revenue = revenue + price;
        receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public double revenue() {
        return revenue;
    }
}

Los tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

Los tests no se tocan: ni el nombre ni las llamadas han cambiado.

Pruébalo

Encuentra el error

Un compañero hace el mismo paso, pero se fía del comentario y lo deja:

Elige la línea con el error.

1// El precio: 1 € por desbloquear y 0,15 € por minuto.2// De noche el precio sube un 50 %.3private static final double UNLOCK_FEE = 1.00;4private static final double PER_MINUTE = 0.15;5private static final double STUDENT_SHARE = 0.80;6private static final int MONTHLY_FREE_MINUTES = 45;7private static final int PAID_UNLOCKS_PER_DAY = 3;8private static final double NIGHT_SURCHARGE = 0.50;

Comprobar

Más precisamente, para curiosos

Los tests

FALLA fourthRideUnlockIsFree()
FALLA monthlyOverFreeMinutes()
FALLA nightSurcharge()
FALLA payAsYouGoRide()
FALLA receipt()
FALLA revenue()
FALLA studentFourthRide()
FALLA studentRide()
OK    monthlyWithinFreeMinutes()
Pruebas: 9, pasaron: 1, fallaron: 8

Míralo

Paso 3: el dinero

El dinero en un double es primitive obsession (code smells). Hoy no da un céntimo mal en ningún test, porque cada importe se redondea solo al imprimirlo. Por eso el paso es un refactoring: la salida es la misma. Pero los ingresos son una suma de miles de importes así, y ahí es donde se acumulan los errores pequeños.

double pasa a ser BigDecimal, creado a partir de un string. BigDecimal no tiene operadores: + y * pasan a ser add y multiply. El repetido PER_MINUTE.multiply(BigDecimal.valueOf(minutes)) pasa a ser un pequeño método perMinute.

class RideBilling {
    private static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    private static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final BigDecimal STUDENT_SHARE = new BigDecimal("0.80");
    private static final int MONTHLY_FREE_MINUTES = 45;
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final List<String> receipts = new ArrayList<>();

    private static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        BigDecimal price = BigDecimal.ZERO;
        if (plan.equals("payg")) {
            price = UNLOCK_FEE.add(perMinute(minutes));
        } else if (plan.equals("student")) {
            price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
        } else if (plan.equals("monthly")) {
            if (minutes > MONTHLY_FREE_MINUTES) {
                price = perMinute(minutes - MONTHLY_FREE_MINUTES);
            }
        }
        if (ridesToday.get(email) > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
            price = price.subtract(UNLOCK_FEE);
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        revenue = revenue.add(price);
        receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

En los tests solo cambia el auxiliar que convierte el importe en texto:

String money(BigDecimal amount) {
    return String.format("%.2f", amount);
}

Los tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

Pruébalo

Completa

El precio por minuto en el tipo para dinero del lenguaje:

private static final BigDecimal PER_MINUTE = new BigDecimal();

Comprobar

Míralo

Paso 4: la consulta separada del comando

Ahora el cambio que esperaba: el precio antes del final tiene que ser una consulta que no cambia nada (comando o consulta). El cálculo se extrae a un método price (Extract Method de la lección de refactoring). finishRide sigue siendo un comando: pide el precio, y luego cuenta, suma y envía.

Un compañero lo hace así. El contador se mueve detrás del precio, porque un viaje cuenta cuando ha terminado:

¿Qué dicen los nueve tests?

public BigDecimal price(String email, String plan, int minutes, boolean night) {
    BigDecimal price = BigDecimal.ZERO;
    if (plan.equals("payg")) {
        price = UNLOCK_FEE.add(perMinute(minutes));
    } else if (plan.equals("student")) {
        price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
    } else if (plan.equals("monthly")) {
        if (minutes > MONTHLY_FREE_MINUTES) {
            price = perMinute(minutes - MONTHLY_FREE_MINUTES);
        }
    }
    if (ridesToday.getOrDefault(email, 0) > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
        price = price.subtract(UNLOCK_FEE);
    }
    if (night) {
        price = price.add(NIGHT_SURCHARGE);
    }
    return price;
}

public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
    BigDecimal price = price(email, plan, minutes, night);
    ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
    revenue = revenue.add(price);
    receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
    return price;
}

Respuesta

Solo falla fourthRideUnlockIsFree()

fourthRideUnlockIsFree() y studentFourthRide() fallan

Pasan los nueve: el contador es el mismo, solo está en otro sitio

Comprobar

Más precisamente, para curiosos

Los tests

FALLA fourthRideUnlockIsFree(): expected: <2.00> but was: <3.00>
FALLA studentFourthRide(): expected: <1.40> but was: <2.40>
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentRide()
Pruebas: 9, pasaron: 7, fallaron: 2

Míralo

Paso 4, segundo intento

El rojo apunta al último paso. No buscas el error en toda la clase: deshaces el paso y lo repites. El orden de los efectos secundarios era una parte oculta del comportamiento, y solo los tests lo sabían.

price calcula qué viaje del día será este:

public BigDecimal price(String email, String plan, int minutes, boolean night) {
    int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
    BigDecimal price = BigDecimal.ZERO;
    if (plan.equals("payg")) {
        price = UNLOCK_FEE.add(perMinute(minutes));
    } else if (plan.equals("student")) {
        price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
    } else if (plan.equals("monthly")) {
        if (minutes > MONTHLY_FREE_MINUTES) {
            price = perMinute(minutes - MONTHLY_FREE_MINUTES);
        }
    }
    if (rideOfDay > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
        price = price.subtract(UNLOCK_FEE);
    }
    if (night) {
        price = price.add(NIGHT_SURCHARGE);
    }
    return price;
}

public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
    BigDecimal price = price(email, plan, minutes, night);
    ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
    revenue = revenue.add(price);
    receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
    return price;
}

Los tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

Los tests no se vuelven a tocar: llaman a finishRide, y este hace lo mismo que antes.

La clase entera después del paso 4

class RideBilling {
    private static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    private static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final BigDecimal STUDENT_SHARE = new BigDecimal("0.80");
    private static final int MONTHLY_FREE_MINUTES = 45;
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final List<String> receipts = new ArrayList<>();

    private static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal price(String email, String plan, int minutes, boolean night) {
        int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
        BigDecimal price = BigDecimal.ZERO;
        if (plan.equals("payg")) {
            price = UNLOCK_FEE.add(perMinute(minutes));
        } else if (plan.equals("student")) {
            price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
        } else if (plan.equals("monthly")) {
            if (minutes > MONTHLY_FREE_MINUTES) {
                price = perMinute(minutes - MONTHLY_FREE_MINUTES);
            }
        }
        if (rideOfDay > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
            price = price.subtract(UNLOCK_FEE);
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        return price;
    }

    public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
        BigDecimal price = price(email, plan, minutes, night);
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        revenue = revenue.add(price);
        receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

Míralo

Paso 5: los planes

Lo que queda es la cadena de if sobre el texto del plan (cadena larga de if). Cada plan nuevo es una rama nueva, y "monthly" se comprueba en dos sitios. Cada plan pasa a ser una clase que conoce su precio y su descuento del cuarto viaje. Esto es Replace Conditional with Polymorphism de la lección de refactoring:

interface Plan {
    BigDecimal price(int minutes);

    BigDecimal loyaltyDiscount();
}

class PayAsYouGo implements Plan {
    @Override
    public BigDecimal price(int minutes) {
        return RideBilling.UNLOCK_FEE.add(RideBilling.perMinute(minutes));
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        return RideBilling.UNLOCK_FEE;
    }
}

class Student implements Plan {
    private static final BigDecimal STUDENT_SHARE = new BigDecimal("0.80");

    @Override
    public BigDecimal price(int minutes) {
        return new PayAsYouGo().price(minutes).multiply(STUDENT_SHARE);
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        // Así es hoy: el 1 € entero, aunque desbloquear le cuesta 0,80 € a un estudiante.
        return RideBilling.UNLOCK_FEE;
    }
}

class Monthly implements Plan {
    private static final int FREE_MINUTES = 45;

    @Override
    public BigDecimal price(int minutes) {
        return RideBilling.perMinute(Math.max(0, minutes - FREE_MINUTES));
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        return BigDecimal.ZERO;
    }
}

La comprobación del abono mensual desaparece: simplemente devuelve un descuento de 0. El comportamiento raro del estudiante queda ahora en una línea, con un comentario que dice por qué es así (el buen comentario de la lección sobre comentarios).

class RideBilling {
    static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final List<String> receipts = new ArrayList<>();

    static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal price(String email, Plan plan, int minutes, boolean night) {
        int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
        BigDecimal price = plan.price(minutes);
        if (rideOfDay > PAID_UNLOCKS_PER_DAY) {
            price = price.subtract(plan.loyaltyDiscount());
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        return price;
    }

    public BigDecimal finishRide(String email, Plan plan, int minutes, boolean night) {
        BigDecimal price = price(email, plan, minutes, night);
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        revenue = revenue.add(price);
        receipts.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

La parte del estudiante y los minutos gratis pasan al plan que los usa. La tarifa de desbloqueo y el precio por minuto los comparten varios planes, así que se quedan en la clase de cobro y dejan de ser private.

En los tests las llamadas pasan un objeto en vez de texto:

assertEquals("3.50", ride("payg", 10, NIGHT));
assertEquals("3.50", ride(new PayAsYouGo(), 10, NIGHT));

Los tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

Esto es Strategy del módulo de patrones de diseño: la regla es un objeto aparte, y el cobro solo pregunta.

Míralo

Los dos cambios ahora

El plan de empresa es una clase nueva. La clase de cobro no se toca (abierto a la extensión):

class Business implements Plan {
    private static final BigDecimal PER_MINUTE = new BigDecimal("0.18");

    @Override
    public BigDecimal price(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        return BigDecimal.ZERO;
    }
}

El precio antes del final es la consulta price. Las veces que la llames, no se cuenta nada y no sale ningún recibo:

public class Main {
    static final boolean DAY = false;
    static final boolean NIGHT = true;

    public static void main(String[] args) {
        RideBilling billing = new RideBilling();
        Plan payg = new PayAsYouGo();
        billing.finishRide("ana@example.com", payg, 10, DAY);
        billing.finishRide("ana@example.com", payg, 5, DAY);
        billing.finishRide("ana@example.com", payg, 12, NIGHT);
        System.out.println("Precio antes del final: " + String.format("%.2f €", billing.price("ana@example.com", payg, 8, DAY)));
        System.out.println("Precio antes del final: " + String.format("%.2f €", billing.price("ana@example.com", payg, 8, DAY)));
        System.out.println("Cobrado: " + String.format("%.2f €", billing.finishRide("ana@example.com", payg, 8, DAY)));
        System.out.println("Empresa, 12 min: " + String.format("%.2f €", billing.finishRide("office@example.com", new Business(), 12, DAY)));
        System.out.println("Recibos: " + billing.receipts().size());
    }
}

Salida

Precio antes del final: 1.60 €
Precio antes del final: 1.60 €
Cobrado: 1.60 €
Empresa, 12 min: 2.16 €
Recibos: 5

Cinco recibos: tres viajes antes de la vista previa, el cuarto y el de la empresa. Las dos vistas previas no dejan rastro, y muestran exactamente lo que se cobra después.

La idea

Antes y después

Smell	Paso	Lección
nada protege el comportamiento	0: tests que fijan el comportamiento de hoy	la red de seguridad
nombres de una letra, calc que no dice que envía	1: Rename	nombres
true en la llamada	1: un argumento que dice lo que significa	funciones
números mágicos	2: constantes con nombre	nombres
un comentario que miente	2: borrado	comentarios
dinero en un double	3: un tipo para dinero	code smells
una consulta con un efecto secundario oculto, un método largo	4: Extract Method, comando y consulta	funciones, refactoring
una cadena de if sobre el plan como texto	5: Replace Conditional with Polymorphism	code smells, refactoring, SOLID

La clase se hizo más larga, no más corta. Pero cada regla está en un solo lugar y tiene nombre, y cada cambio del principio es una clase.

El día de viajes, ejecutado con la clase de antes del paso 1 y con la clase de después del paso 5, imprime lo mismo línea por línea. Aquí los dos programas se ejecutan en cada comprobación del sitio.

Antes

RideMgr rides = new RideMgr();
rides.calc("ana@example.com", "payg", 10, false);
rides.calc("ivan@example.com", "student", 25, false);
rides.calc("ana@example.com", "payg", 5, false);
rides.calc("maria@example.com", "monthly", 60, true);
rides.calc("ana@example.com", "payg", 12, true);
rides.calc("ana@example.com", "payg", 8, false);

for (String receipt : rides.sent()) {
    System.out.println(receipt);
}
System.out.println("Ingresos: " + String.format("%.2f €", rides.day()));

Después

public class Main {
    static final boolean DAY = false;
    static final boolean NIGHT = true;

    public static void main(String[] args) {
        RideBilling billing = new RideBilling();
        Plan payg = new PayAsYouGo();
        billing.finishRide("ana@example.com", payg, 10, DAY);
        billing.finishRide("ivan@example.com", new Student(), 25, DAY);
        billing.finishRide("ana@example.com", payg, 5, DAY);
        billing.finishRide("maria@example.com", new Monthly(), 60, NIGHT);
        billing.finishRide("ana@example.com", payg, 12, NIGHT);
        billing.finishRide("ana@example.com", payg, 8, DAY);

        for (String receipt : billing.receipts()) {
            System.out.println(receipt);
        }
        System.out.println("Ingresos: " + String.format("%.2f €", billing.revenue()));
    }
}

Salida

ana@example.com: 10 min, 3.00 €
ivan@example.com: 25 min, 4.80 €
ana@example.com: 5 min, 2.00 €
maria@example.com: 60 min, 3.50 €
ana@example.com: 12 min, 3.90 €
ana@example.com: 8 min, 1.60 €
Ingresos: 18.80 €

La idea

Dónde parar

Te detienes cuando los cambios que vienen se vuelven fáciles. Aquí los dos cambios son una clase y una línea. Por eso algunos pasos se dejan sin hacer a propósito:

• el recibo en su propia clase. El texto del recibo es una segunda razón para cambiar la clase (una sola razón para cambiar). Hoy no cambia. En cuanto marketing empiece a editarlo cada mes, pasa a ser una clase aparte (tarea 1);
• el envío detrás de una interfaz. Los recibos son una lista, no un correo de verdad. Cuando llegue el correo real, el test no debe enviarlo, y entonces llega la abstracción (depender de una abstracción);
• el comportamiento del estudiante. Es un cambio de comportamiento, no una limpieza. Se hace aparte, con su propio test, cuando el producto decida.

Y la regla de la lección de refactoring: cambias la forma o el comportamiento, nunca los dos en un mismo paso.

Ejercicio

Sigue tú solo

Resuelve las tareas en tu editor, partiendo de la clase después del paso 5. Después de cada paso, ejecuta los nueve tests.

1. El recibo en su propia clase. Mueve el texto del recibo y la lista de recibos enviados a una clase ReceiptBook. La clase de cobro solo le dice "send". Los tests no se tocan.

Una solución posible

class ReceiptBook {
    private final List<String> sent = new ArrayList<>();

    public void send(String email, int minutes, BigDecimal price) {
        sent.add(email + ": " + minutes + " min, " + String.format("%.2f", price) + " €");
    }

    public List<String> sent() {
        return sent;
    }
}

class RideBilling {
    static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final ReceiptBook receipts = new ReceiptBook();

    static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal price(String email, Plan plan, int minutes, boolean night) {
        int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
        BigDecimal price = plan.price(minutes);
        if (rideOfDay > PAID_UNLOCKS_PER_DAY) {
            price = price.subtract(plan.loyaltyDiscount());
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        return price;
    }

    public BigDecimal finishRide(String email, Plan plan, int minutes, boolean night) {
        BigDecimal price = price(email, plan, minutes, night);
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        revenue = revenue.add(price);
        receipts.send(email, minutes, price);
        return price;
    }

    public List<String> receipts() {
        return receipts.sent();
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

Los tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Pruebas: 9, pasaron: 9, fallaron: 0

El programa del día de la tarjeta "Antes y después" imprime la misma salida con esta clase.

2. El plan desde texto. El plan llega de la base de datos como texto ("payg"). Escribe la tabla que lo convierte en un objeto. ¿Qué hacía la clase vieja con un plan desconocido? Fíjalo con un test sobre la clase vieja antes de decidir qué debe hacer la nueva.

3. La noche desde la hora. La app ya no sabe cuándo es de noche; pasa la hora de inicio. La regla "de las 22:00 a las 6:00" tiene que vivir en un solo lugar. Escribe primero los tests para las 21:59, 22:00, 5:59 y 6:00.

4. El estudiante. El producto ha decidido: en el cuarto viaje un estudiante recibe 0,80 €, no 1 €. Es un cambio de comportamiento: primero cambia el valor esperado en el test y míralo en rojo, y luego cambia una línea del plan.

Test

Ponte a prueba

1. El test de un estudiante en el cuarto viaje espera 1,40 €, y la respuesta "correcta" parece ser 1,60 €. ¿Qué haces mientras limpias la clase?

Cambiar el valor esperado a 1,60 y arreglar el código en el mismo pasoDejar el test como está, con un comentario, y preguntar al product owner. Si decide otra cosa, eso es un cambio aparte con su propio testBorrar el test, porque fija un error

2. El comentario dice 0,15 € por minuto, y el código cobra 0,20 €. ¿A cuál le haces caso?

Al código: los tests fijaron lo que hace, y los clientes llevan años pagando según élAl comentario: es la documentaciónAl que sea más nuevo

3. ¿Por qué los tests llaman a la clase a través de un método auxiliar y no directamente?

Los hace más rápidosCuando cambia la forma de la clase, cambia un solo lugar, y los valores esperados siguen igualPorque el framework de tests lo exige

4. El primer intento del paso 4 puso dos tests en rojo. ¿Qué haces?

Deshacer el paso y repetirlo: el rojo apunta a él, porque antes todo estaba en verdeBuscar el error en toda la claseCambiar los valores esperados de los dos tests

5. Después de la limpieza, el plan de empresa es una clase nueva, y la clase de cobro no se toca. ¿Qué principio es este?

Depender de una abstracciónUna sola razón para cambiarAbierto a la extensión, cerrado a la modificación

Comprobar

Empezar la lección de nuevo

Lecciones relacionadas

• Se basa en: Code smells: método largo, clase grande, duplicación, primitivos en vez de tipos, cadenas largas de if · Las pruebas como red de seguridad · Refactorización en pasos pequeños · SOLID sin dogma
• Ejercicios sobre el tema: Código limpio y refactorización