Un intento de optimizar un código funcional pero poco elegante terminó desencadenando un incidente en producción, provocando la duplicación de órdenes de envío en un sistema logístico. La raíz del problema fue una refactorización apresurada y la falta de pruebas exhaustivas, lo que llevó a la conclusión de que si un código opera correctamente, la modificación debe ser mínima o, de lo contrario, optar por la solución de menor impacto.

Contexto del problema

El objetivo era consolidar una lista de objetos sumando los valores de aquellos que compartían un mismo identificador. Para ilustrarlo, utilizaremos una clase de dominio Inmueble, donde se deben agrupar las propiedades que pertenecen al mismo propietario y acumular su valor.

public class Inmueble {
    private String propietario;
    private String tipo;
    private BigDecimal valor;
    private String direccion;

    public Inmueble() {}

    public Inmueble(String propietario, String tipo, BigDecimal valor, String direccion) {
        this.propietario = propietario;
        this.tipo = tipo;
        this.valor = valor;
        this.direccion = direccion;
    }
    // Getters y Setters...
}

Se parte de una lista inicial con varios inmuebles, algunos compartiendo propietario:

List<Inmueble> catastro = new ArrayList<>();
catastro.add(new Inmueble("Carlos", "Casa", new BigDecimal("500"), "Calle 1"));
catastro.add(new Inmueble("Ana", "Apto", new BigDecimal("200"), "Calle 2"));
catastro.add(new Inmueble("Beto", "Casa", new BigDecimal("350"), "Calle 3"));
catastro.add(new Inmueble("Ana", "Finca", new BigDecimal("800"), "Calle 4"));
catastro.add(new Inmueble("Beto", "Apto", new BigDecimal("150"), "Calle 5"));

Implemenatción original (Funcional)

El código inicial recorría la lista y acumulaba los valores en una colección auxiliar. Aunque carecía de elegancia y reusabilidad, operaba sin fallos.

List<Inmueble> unicos = new ArrayList<>();
for (Inmueble actual : catastro) {
    boolean encontrado = false;
    for (Inmueble unico : unicos) {
        if (unico.getPropietario().equals(actual.getPropietario())) {
            unico.setValor(unico.getValor().add(actual.getValor()));
            encontrado = true;
        }
    }
    if (!encontrado) {
        unicos.add(actual);
    }
}

Refactorización con bug introducido

Se decidió abstraer la lógica a un método genérico utilizando interfaces funcionales (BiPredicate y BiConsumer) para evaluar coincidencias y ejecutar la fusión.

public static <T> List<T> fusionarDuplicados(Collection<T> items, BiPredicate<T, T> comparador, BiConsumer<T, T> fusionador) {
    List<T> resultado = new ArrayList<>();
    for (T item : items) {
        boolean coincidencia = false;
        for (T res : resultado) {
            coincidencia = comparador.test(res, item);
            if (coincidencia) {
                fusionador.accept(res, item);
            }
        }
        if (!coincidencia) {
            resultado.add(item);
        }
    }
    return resultado;
}

// Ejecución
List<Inmueble> consolidado = fusionarDuplicados(
    catastro,
    (base, nuevo) -> base.getPropietario().equals(nuevo.getPropietario()),
    (base, nuevo) -> base.setValor(base.getValor().add(nuevo.getValor()))
);

Al ejecutar esta versión, el resultado contenía entradas adicoinales para "Ana" y "Beto" que no debían existir, ya que sus valores debía haberse sumado a la primera ocurrencia y omitido su inserción como nuevos elementos.

Análisis del fallo lógico

El defecto crítico reside en el bucle interno del método genérico:

for (T res : resultado) {
    coincidencia = comparador.test(res, item);
    if (coincidencia) {
        fusionador.accept(res, item);
    }
}

Cuando se encuentra una coincidencia, coincidencia se establece en true y se acumula el valor. Sin embargo, al no interrumpir la iteración, el bucle continúa evaluando los elementos restantes en resultado. Si un elemento posterior no coincide, la variable coincidencia se sobrescribe a false. Esto provoca que la validación exterior if (!coincidencia) evalúe incorrectamente que el elemento no fue procesado, añadiéndolo de nuevo a la lista de resultados.

Corrección del código

La solución directa es detener el flujo del bucle interno tan pronto como se detecte y procese la coincidencia, evitando así la sobrescritura del estado.

public static <T> List<T> fusionarDuplicados(Collection<T> items, BiPredicate<T, T> comparador, BiConsumer<T, T> fusionador) {
    List<T> resultado = new ArrayList<>();
    for (T item : items) {
        boolean coincidencia = false;
        for (T res : resultado) {
            if (comparador.test(res, item)) {
                fusionador.accept(res, item);
                coincidencia = true;
                break;
            }
        }
        if (!coincidencia) {
            resultado.add(item);
        }
    }
    return resultado;
}