Refactorización en pasos pequeños

Código en:JavaC#PythonJavaScript

En resumen: La refactorización cambia la estructura del código sin cambiar su comportamiento, en pasos pequeños con nombre como Rename y Extract Method, con las pruebas en verde después de cada uno.

Problema

La factura que nadie quiere tocar

La tienda online imprime las facturas con el método Invoices.Inv. Lleva dos años funcionando:

public static string Inv(string n, string street, string city, string zip, List<Item> items, string type)
{
    decimal t = 0;
    foreach (Item i in items)
    {
        t = t + i.Price * i.Qty;
    }
    decimal d = 0;
    if (type == "gold")
    {
        d = t * 0.05m;
    }
    else if (type == "company")
    {
        if (t >= 500)
        {
            d = t * 0.10m;
        }
    }
    string s = "Factura: " + n + "\n";
    s = s + street + ", " + zip + " " + city + "\n";
    s = s + $"Importe: {t:F2} €\n";
    s = s + $"Descuento: {d:F2} €\n";
    s = s + $"Total con IVA: {AfterDiscount(t, d) * 1.20m:F2} €";
    return s;
}

static decimal AfterDiscount(decimal t, decimal d)
{
    return t - d;
}

¿Qué es t? ¿Y d? Seis parámetros seguidos, cuatro de ellos texto: quien cambie de sitio la ciudad y el código postal obtiene una factura con «Madrid 28013» en el lugar equivocado, y el compilador no dice nada. La cadena de if según el tipo de cliente sigue creciendo: el mes que viene llegan los socios con un 3 %. Y AfterDiscount hace una sola resta, pero te obliga a saltar a otro sitio para verla.

Quieres ordenarla. Pero las facturas van a contabilidad, y un error en ellas es un error en los impuestos.

La idea

Refactorización: la forma cambia, el comportamiento no

Refactorizar es cambiar la forma del código manteniendo el comportamiento: las mismas entradas dan las mismas salidas. No es «reescribir». Tampoco es «ya que estaba, arreglé un error». Esos son otros cambios, que se hacen por separado.

Se hace en pasos pequeños con nombre. Cada paso es tan pequeño que apenas puede salir mal, y después de cada uno ejecutas las pruebas (la red de seguridad de la lección anterior). Los nombres vienen del catálogo de Martin Fowler y son los mismos en cualquier lenguaje y cualquier IDE: Rename, Extract Method, Inline, Introduce Parameter Object, Replace Conditional with Polymorphism.

Primero va la red. Cuatro pruebas fijan los cuatro tipos de factura que la tienda imprime hoy: un cliente normal, un cliente oro, una empresa por encima y por debajo de 500 €.

public class InvoiceTest
{
    List<Item> coffee = [new("Café en grano", 12.50m, 2), new("Taza", 8.00m, 1)];
    List<Item> moreCoffee = [new("Café en grano", 12.50m, 4)];
    List<Item> flour = [new("Harina 25 kg", 30.00m, 20)];

    [Fact]
    public void Regular()
    {
        Assert.Equal("Factura: María Torres\nCalle Mayor 5, 28013 Madrid\nImporte: 33.00 €\nDescuento: 0.00 €\nTotal con IVA: 39.60 €",
            Invoices.Inv("María Torres", "Calle Mayor 5", "Madrid", "28013", coffee, "regular"));
    }

    [Fact]
    public void Gold()
    {
        Assert.Equal("Factura: Lucía Navarro\nCalle Alcalá 12, 41001 Sevilla\nImporte: 50.00 €\nDescuento: 2.50 €\nTotal con IVA: 57.00 €",
            Invoices.Inv("Lucía Navarro", "Calle Alcalá 12", "Sevilla", "41001", moreCoffee, "gold"));
    }

    [Fact]
    public void CompanyFrom500()
    {
        Assert.Equal("Factura: Panadería Amanecer S.L.\nAvenida del Puerto 80, 46001 Valencia\nImporte: 600.00 €\nDescuento: 60.00 €\nTotal con IVA: 648.00 €",
            Invoices.Inv("Panadería Amanecer S.L.", "Avenida del Puerto 80", "Valencia", "46001", flour, "company"));
    }

    [Fact]
    public void CompanyUnder500()
    {
        Assert.Equal("Factura: Panadería Amanecer S.L.\nAvenida del Puerto 80, 46001 Valencia\nImporte: 33.00 €\nDescuento: 0.00 €\nTotal con IVA: 39.60 €",
            Invoices.Inv("Panadería Amanecer S.L.", "Avenida del Puerto 80", "Valencia", "46001", coffee, "company"));
    }
}

Resultado

OK    CompanyFrom500
OK    CompanyUnder500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 4, fallaron: 0

Míralo

Paso 1: Rename

Los nombres de una letra pasan a ser palabras: t es subtotal, d es discount, s es text, Inv es Invoice. zip pasa a ser postcode, y type pasa a ser customerType.

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
    decimal subtotal = 0;
    foreach (Item item in items)
    {
        subtotal = subtotal + item.Price * item.Qty;
    }
    decimal discount = 0;
    if (customerType == "gold")
    {
        discount = subtotal * 0.05m;
    }
    else if (customerType == "company")
    {
        if (subtotal >= 500)
        {
            discount = subtotal * 0.10m;
        }
    }
    string text = "Factura: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {AfterDiscount(subtotal, discount) * 1.20m:F2} €";
    return text;
}

Las pruebas

OK    CompanyFrom500
OK    CompanyUnder500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 4, fallaron: 0

Rename también cambia todos los sitios que llaman a el método, incluidas las pruebas. Las facturas esperadas no se tocan.

En Visual Studio es Ctrl+R, Ctrl+R (o F2), en Rider es Mayús+F6. El compilador sabe qué Inv es cuál, así que cambia exactamente ese, no cada trozo de texto Inv.

Míralo

Paso 2: Extract Method

El método hace dos trabajos distintos que no tienen nada que ver con el texto de la factura: el total de las líneas y el descuento. Cada uno pasa a un método propio, con un nombre que dice qué devuelve. Eso es Extract Method:

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
    decimal subtotal = ItemsTotal(items);
    decimal discount = DiscountFor(customerType, subtotal);
    string text = "Factura: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {AfterDiscount(subtotal, discount) * 1.20m:F2} €";
    return text;
}

static decimal ItemsTotal(List<Item> items)
{
    decimal sum = 0;
    foreach (Item item in items)
    {
        sum = sum + item.Price * item.Qty;
    }
    return sum;
}

static decimal DiscountFor(string customerType, decimal subtotal)
{
    decimal discount = 0;
    if (customerType == "gold")
    {
        discount = subtotal * 0.05m;
    }
    else if (customerType == "company")
    {
        if (subtotal >= 500)
        {
            discount = subtotal * 0.10m;
        }
    }
    return discount;
}

Las pruebas

OK    CompanyFrom500
OK    CompanyUnder500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 4, fallaron: 0

Ahora Invoice se lee de arriba abajo como una lista: cuánto es, cuánto es el descuento, cuál es el texto. Los detalles están a un salto de distancia, solo si los necesitas.

En Visual Studio: selecciona las líneas y pulsa Ctrl+R, Ctrl+M (o Ctrl+. y «Extract method»). En Rider: Ctrl+Alt+M.

El IDE mira qué variables entran en las líneas seleccionadas y cuáles salen, y crea los parámetros y el valor de retorno. Si haces el paso a mano, es justo ahí donde se tuerce.

Pruébalo

Predice la salida

Alguien hace Extract Method a mano. El descuento pasa a un método que lo guarda en su propio parámetro en lugar de devolverlo:

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
    decimal subtotal = ItemsTotal(items);
    decimal discount = 0;
    ApplyDiscount(customerType, subtotal, discount);
    string text = "Factura: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {AfterDiscount(subtotal, discount) * 1.20m:F2} €";
    return text;
}

static void ApplyDiscount(string customerType, decimal subtotal, decimal discount)
{
    if (customerType == "gold")
    {
        discount = subtotal * 0.05m;
    }
    else if (customerType == "company")
    {
        if (subtotal >= 500)
        {
            discount = subtotal * 0.10m;
        }
    }
}

Respuesta

Fallan Gold y CompanyFrom500

Pasan las cuatro

Solo falla CompanyFrom500

Comprobar

Más precisamente, para curiosos

Resultado

FALLA CompanyFrom500
FALLA Gold
OK    CompanyUnder500
OK    Regular
Pruebas: 4, pasaron: 2, fallaron: 2

discount dentro de ApplyDiscount es una copia del valor. El valor nuevo se queda allí dentro y desaparece cuando termina el método, y fuera discount sigue siendo 0. Donde el descuento ya es 0 de todos modos (un cliente normal, una empresa por debajo de 500 €), las pruebas pasan. Por eso la red tiene una prueba para cada rama del if. El código extraído tiene que devolver el valor, como lo hace el IDE.

Míralo

Paso 3: Inline

Lo contrario de Extract Method. AfterDiscount hace una resta, y su nombre no dice nada más que subtotal - discount. Se usa en un solo sitio. El cuerpo va donde está la llamada, y el método desaparece. Eso es Inline:

static decimal AfterDiscount(decimal subtotal, decimal discount)
{
    return subtotal - discount;
}

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
    decimal subtotal = ItemsTotal(items);
    decimal discount = DiscountFor(customerType, subtotal);
    string text = "Factura: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {(subtotal - discount) * 1.20m:F2} €";
    return text;
}

Las pruebas

OK    CompanyFrom500
OK    CompanyUnder500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 4, fallaron: 0

Extract Method e Inline no se contradicen. Extraes cuando el nombre explica algo que el código no dice (ItemsTotal). Lo devuelves a su sitio cuando el nombre es solo un salto (AfterDiscount). En Rider es Ctrl+Alt+N, en Visual Studio es Ctrl+. y «Inline method».

Míralo

Paso 4: Introduce Parameter Object

El nombre, la calle, la ciudad y el código postal siempre viajan juntos, y sin embargo son cuatro parámetros seguidos. Un intercambio no se nota:

Invoices.Invoice("María Torres", "Calle Mayor 5", "28013", "Madrid", coffee, "regular")

Salida

Factura: María Torres
Calle Mayor 5, Madrid 28013
Importe: 33.00 €
Descuento: 0.00 €
Total con IVA: 39.60 €

Los cuatro pasan a ser un objeto: el cliente. Eso es Introduce Parameter Object.

Un record escribe las propiedades y el constructor:

public record Customer(string Name, string Street, string City, string Postcode);

public static string Invoice(Customer customer, List<Item> items, string customerType)
{
    decimal subtotal = ItemsTotal(items);
    decimal discount = DiscountFor(customerType, subtotal);
    string text = "Factura: " + customer.Name + "\n";
    text = text + customer.Street + ", " + customer.Postcode + " " + customer.City + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {(subtotal - discount) * 1.20m:F2} €";
    return text;
}

Las pruebas

OK    CompanyFrom500
OK    CompanyUnder500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 4, fallaron: 0

Las llamadas en las pruebas cambian con la forma, y las facturas esperadas se quedan igual. Aquí está una llamada a través de los pasos:

Invoices.Inv("Lucía Navarro", "Calle Alcalá 12", "Sevilla", "41001", moreCoffee, "gold")
Invoices.Invoice("Lucía Navarro", "Calle Alcalá 12", "Sevilla", "41001", moreCoffee, "gold")
Invoices.Invoice(new Customer("Lucía Navarro", "Calle Alcalá 12", "Sevilla", "41001"), moreCoffee, "gold")
Invoices.Invoice(new Customer("Lucía Navarro", "Calle Alcalá 12", "Sevilla", "41001"), moreCoffee, new Gold())

En C# el constructor también se puede llamar con nombres: new Customer(Name: "...", City: "...", ...). Entonces el orden no importa.

Míralo

Paso 5: Replace Conditional with Polymorphism

Lo que queda es la cadena de if según el tipo de cliente en DiscountFor. Cada tipo nuevo es una rama nueva aquí, y mañana también en otros sitios (el correo, el informe). Cada tipo pasa a ser una clase que implementa una interfaz común, y conoce su propio descuento:

public interface ICustomerType
{
    decimal Discount(decimal subtotal);
}

public class Regular : ICustomerType
{
    public decimal Discount(decimal subtotal)
    {
        return 0;
    }
}

public class Gold : ICustomerType
{
    public decimal Discount(decimal subtotal)
    {
        return subtotal * 0.05m;
    }
}

public class Company : ICustomerType
{
    public decimal Discount(decimal subtotal)
    {
        if (subtotal >= 500)
        {
            return subtotal * 0.10m;
        }
        return 0;
    }
}

public static string Invoice(Customer customer, List<Item> items, ICustomerType customerType)
{
    decimal subtotal = ItemsTotal(items);
    decimal discount = customerType.Discount(subtotal);
    string text = "Factura: " + customer.Name + "\n";
    text = text + customer.Street + ", " + customer.Postcode + " " + customer.City + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {(subtotal - discount) * 1.20m:F2} €";
    return text;
}

Las pruebas

OK    CompanyFrom500
OK    CompanyUnder500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 4, fallaron: 0

Esto es Strategy del módulo de patrones: la regla es un objeto aparte y la factura solo pregunta. Si el tipo de cliente cambia mientras el programa se ejecuta (pasa a ser oro tras el décimo pedido), el mismo movimiento lleva a State. El texto "gold" de la base de datos se convierte en un objeto en un solo sitio, a la entrada, con una tabla como en la lección de Strategy.

public record Customer(string Name, string Street, string City, string Postcode);

public interface ICustomerType
{
    decimal Discount(decimal subtotal);
}

public static string Invoice(Customer customer, List<Item> items, ICustomerType customerType)
{
    decimal subtotal = ItemsTotal(items);
    decimal discount = customerType.Discount(subtotal);
    string text = "Factura: " + customer.Name + "\n";
    text = text + customer.Street + ", " + customer.Postcode + " " + customer.City + "\n";
    text = text + $"Importe: {subtotal:F2} €\n";
    text = text + $"Descuento: {discount:F2} €\n";
    text = text + $"Total con IVA: {(subtotal - discount) * 1.20m:F2} €";
    return text;
}

Paso a paso, en palabras

1. La función antes del primer paso: 23 líneas en un solo bloque. Abajo están las cuatro pruebas y los totales de las cuatro facturas.
2. Rename: t pasa a ser subtotal, d pasa a ser discount, s pasa a ser text. Las pruebas están en verde, los totales son los mismos.
3. Extract Method: el bucle va a ItemsTotal, la cadena de if a DiscountFor. En su lugar queda una línea para cada uno.
4. Inline: AfterDiscount solo restaba. Su cuerpo vuelve a la línea y la función desaparece.
5. Introduce Parameter Object: nombre, calle, ciudad y código postal pasan a ser un solo cliente.
6. Replace Conditional with Polymorphism: el if desaparece. Cada tipo de cliente es una clase detrás de ICustomerType, y la función llama a customerType.Discount(subtotal).
7. Cinco pasos. La forma de Invoice es otra, pero las cuatro pruebas y las cuatro facturas son las mismas que al principio.

El programa que imprime las cuatro facturas se ejecutó con el código de antes del primer paso y con el código después del quinto. La salida es la misma:

Salida antes y después

Factura: María Torres
Calle Mayor 5, 28013 Madrid
Importe: 33.00 €
Descuento: 0.00 €
Total con IVA: 39.60 €

Factura: Lucía Navarro
Calle Alcalá 12, 41001 Sevilla
Importe: 50.00 €
Descuento: 2.50 €
Total con IVA: 57.00 €

Factura: Panadería Amanecer S.L.
Avenida del Puerto 80, 46001 Valencia
Importe: 600.00 €
Descuento: 60.00 €
Total con IVA: 648.00 €

Factura: Panadería Amanecer S.L.
Avenida del Puerto 80, 46001 Valencia
Importe: 33.00 €
Descuento: 0.00 €
Total con IVA: 39.60 €

Pruébalo

Encuentra el error

Alguien escribe así la clase para una empresa:

public class Company : ICustomerType
{
    public decimal Discount(decimal subtotal)
    {
        return subtotal * 0.10m;
    }
}

Una empresa pide café por 33 €:

Invoices.Invoice(new Customer("Panadería Amanecer S.L.", "Avenida del Puerto 80", "Valencia", "46001"), coffee, new Company())

Salida

Factura: Panadería Amanecer S.L.
Avenida del Puerto 80, 46001 Valencia
Importe: 33.00 €
Descuento: 3.30 €
Total con IVA: 35.64 €

¿En qué línea está el error?

Elige la línea con el error.

1public class Company : ICustomerType2{3 public decimal Discount(decimal subtotal)4 {5 return subtotal * 0.10m;6 }7}

Comprobar

Más precisamente, para curiosos

Las pruebas

FALLA CompanyUnder500
OK    CompanyFrom500
OK    Gold
OK    Regular
Pruebas: 4, pasaron: 3, fallaron: 1

La idea

Cuándo parar

La refactorización no tiene fin si no le pones uno. Paras cuando el código es lo bastante claro para el cambio que viene. ¿La tarea es añadir socios con un 3 %? El paso 5 lo convierte en una clase nueva. ¿La tarea es otra? Quizá no necesites el paso 5.

No merece la pena cuando:

• nadie toca ni lee el código. El código feo que funciona y no cambia no es urgente;
• hay dos variantes y no van a crecer. Un if es más claro que una interfaz y dos clases;
• no hay red debajo del código. Entonces el primer paso es la red, no la refactorización.

Y una regla: no mezclar. Cambias la forma o el comportamiento, nunca los dos en un mismo paso. Kent Beck lo llama «los dos sombreros»: con uno reorganizas y las pruebas no se tocan, con el otro añades y escribes una prueba nueva. Cambias de sombrero a propósito.

Ejercicio

Socios con un 3 %

Resuelve la tarea en tu editor.

Socios. La tienda añade el tipo de cliente «socio» con un 3 % de descuento en cada pedido. Empieza con el código después del paso 5. Primero escribe una prueba para un socio con el pedido de 33 €, luego la clase Partner. Las demás clases y Invoice no se tocan.

Salida

Factura: María Torres
Calle Mayor 5, 28013 Madrid
Importe: 33.00 €
Descuento: 0.99 €
Total con IVA: 38.41 €

Más tareas

• Extract Method para el texto. Extrae la línea de la dirección a un método AddressLine(Customer). Ejecuta las pruebas después del paso.
• El otro orden. Haz los cinco pasos en otro orden: primero Introduce Parameter Object y luego Rename. ¿Pasan las pruebas después de cada uno? ¿Qué paso era más fácil antes?
• El tipo desde la base de datos. El tipo de cliente llega como texto ("gold"). Escribe la tabla que lo convierte en un objeto y decide qué pasa con un tipo desconocido. El código de hoy da un descuento de 0 para un tipo desconocido: mantenlo con una prueba.

Test

Ponte a prueba

1. ¿Cuál de estas cosas es refactorización?

La factura empieza a mostrar el IVA en su propia líneaEl bucle del subtotal pasa a un método aparte con el mismo resultadoEl descuento para un cliente oro pasa a ser del 7 %

2. Después de Introduce Parameter Object las pruebas no compilan, porque llaman al método con seis parámetros. ¿Qué haces?

Cambiar también las facturas esperadas, para que coincidanBorrar las pruebas y escribir otras nuevas al finalCambiar solo las llamadas en las pruebas; las facturas esperadas se quedan como están

3. ¿Cuándo es mejor Inline que Extract Method?

Nunca: cuantos más métodos, mejorCuando el método es largoCuando el método pequeño dice lo mismo que su cuerpo y solo añade un salto al leer

4. La cadena de if según el tipo de cliente se convierte en una interfaz y una clase por cada tipo. ¿Cómo se llama esto en el módulo de patrones?

ObserverStrategy (o State, si el tipo cambia mientras el programa se ejecuta)Singleton

5. Cinco pasos, pruebas después de cada uno. En el cuarto, una prueba falla. ¿Qué significa?

El cuarto paso cambió el comportamiento, y los tres primeros están bienTienes que volver al principioLa prueba está mal

Comprobar

Empezar la lección de nuevo

Lecciones relacionadas

• Se basa en: Nombres en el código · Funciones: una tarea, pocos parámetros, sin efectos secundarios ocultos · Code smells: método largo, clase grande, duplicación, primitivos en vez de tipos, cadenas largas de if · Las pruebas como red de seguridad
• Continúa en: SOLID sin dogma · Ejercicio: una clase desordenada, limpiada paso a paso
• Ejercicios sobre el tema: Código limpio y refactorización