DEV Community: Fran Iglesias

Golden, librería de snapshot testing en Go

Fran Iglesias — Wed, 31 Jan 2024 00:00:00 +0000

Golden es una librería de snapshot testing que he creado para Go. Después de unas semanas de trabajo, ya está declarada estable y he publicado la versión v1.0.0.

En parte, como ejercicio de aprendizaje. En parte, porque no me acaban de encajar otras librerías disponibles.

Snapshot testing es una técnica bastante usada en desarrollo frontend que consiste en guardar el output de nuestro código y usarlo como criterio para ejecutar futuros tests. De este modo, creamos un test de regresión que nos asegure que mantenemos el comportamiento actual de una unidad de software.

En backend, el snapshot testing no es tan usado, pero hay muchos casos de uso para esta técnica: objetos complejos, generación de archivos de todo tipo (JSON, CSV, XML, etc.) para los que es costoso desarrollar un test basado en aserciones.

Además, esta técnica es muy potente usada con código legacy o, en general, con código que no tiene tests. Nos permite obtener una buena cobertura rápidamente, antes de intervenir en un código.

Golden , además de snapshot testing, nos permite trabajar con approval testing. En esta modalidad, lo que hacemos es mantener el test fallando a propósito hasta que el snapshot que se ha generado sea revisado por nosotras o por una experta del dominio que nos pueda decir si el output es correcto o no. Cuando nos satisface, “aprobamos” el snapshot y lo usamos como criterio en los tests futuros.

Approval testing es una técnica adecuada cuando estamos escribiendo código nuevo que genera objetos complejos o documentos.

Finalmente, Golden ofrece la posibilidad de realizar los tests combinatorios de la técnica Golden Master. Esta técnica consiste en bombardear el código a base de llamadas con distintas combinaciones de sus parámetros, de tal modo que lo forcemos a recorrer todos sus posibles flujos de ejecución.

Para ello, no tenemos más que indicarle a Golden listas de valores para cada parámetro de entrada de la unidad bajo test y generará todas las combinaciones posibles. Esta técnica puede ayudarnos a obtener una cobertura completa de un código existente sin tener que preocuparnos de entenderlo en profundidad. Una vez que hemos generado el “golden master” y estamos protegidas por el test, podemos empezar a aplicar técnicas de refactor para mejorar su diseño.

Golden: librería de snapshot testing en Go

Refactoring para quienes no refactorizan 3

Fran Iglesias — Sun, 03 Dec 2023 00:00:00 +0000

En las entregas anteriores hemos mencionado varias veces la necesidad de mantener el tiempo de refactoring bajo control, evitando la tentación de llevarlo demasiado lejos.

Esta precaución es necesaria porque corremos el riesgo de dar al código una forma que no sea adecuada a su evolución futura. Esto es, aunque podamos tener unas expectativas razonables sobre la evolución de nuestro negocio, en realidad no sabemos que nos deparará el futuro. Tomar decisiones sobre la estructura del código que no estén apoyadas por una necesidad puede llevarnos a mayores costes cuando tengamos que desandar ese camino.

Cuando un refactor llama a tu puerta

De todos modos, y teniendo esta advertencia en mente, muchas veces el propio código nos va a dar indicaciones de que necesita refactoring. Estas indicaciones suelen venir en forma de code smells, algunos de los cuales pueden gestionarse independientemente del significado del código. Más bien, esos smells apuntan a que la estructura de conocimiento está reflejada en el código, pero de una forma defectuosa.

Voy a intentar poner un ejemplo. Los siguientes métodos operan sobre un mismo objeto, el cual no es el objeto en el que están definidos.

    def standard_deviation(consumptions)
        Math.sqrt(variance(consumptions))
    end

    def variance(consumptions)
        sum = consumptions.sum(0.0) { |element| (element - average(consumptions)) ** 2 }
        sum / (consumptions.size - 1)
    end

    def average(consumptions)
        consumptions.sum(0.0) / consumptions.size
    end

Dicho de otra forma. Los comportamientos representados por estos métodos en realidad pertenecen a un objeto consumptions que aún no tenemos, pero del cual el código nos está diciendo que, al menos, deberíamos considerar su existencia.

Este consumptions es actualmente un array que agrega los consumos de una oficina. O, en general, representa una colección de consumos que nos interesa para hacer un análisis. Tratarlo como un array es propio de un enfoque procedural de la programación, pero en orientación a objetos, consumption debería ser un objeto con sus propios comportamientos. En este caso: agregar los consumos y proporcionarnos ciertos índices estadísticos que nos interesan y que se obtienen a partir de sus datos.

La señal que nos indica que hay una posibilidad de refactoring es bastante visible:

Hay un grupo de métodos de un objeto que no llaman a otros métodos del propio objeto, excepto a los que forman parte del mismo grupo. Por ejemplo, standard_deviation usa variance, el cual usa average, pero no usan otros métodos del objeto.
Esos métodos tienen un parámetro en común, que es sobre el que trabajan. Todos trabajan sobreconsumptions.

Ahora bien, si nos fijamos, consumptions solo guarda las lecturas de consumo. En el artículo anterior mencionamos que podría ser interesante guardar todo el objeto Consumption. En primer lugar, porque es un objeto y así mantenemos su integridad. En segundo lugar, nos aporta más información que podría llegar a ser útil en algún momento.

¿A dónde quiero llegar? Si hago este refactor ahora mismo, encapsulando el array consumptions en un objeto estoy tomando decisiones que pueden condicionar el desarrollo futuro del software. Ahora me parece muy claro que podría encapsular consumptions y beneficiarme de sus comportamientos. Pero ¿y si en el futuro lo que necesito es tener los objetos Consumption?

Suele ser preferible esperar a tener más contexto antes de proceder a un refactor, incluso aunque sea muy evidente. Por ejemplo, que tengamos una tarea que toca esa área.

Una fuente de datos alternativa

Ahora que hemos cambiado la forma de agregar los datos por oficina, nos dicen que algunas oficinas podrían obtener la información en formato JSON. Hay que tener en cuenta que a partir de ahora se nos proporcionarán varios archivos con los datos, pero en los distintos formatos. De hecho, es perfectamente posible que sean varias decenas de archivos si cada oficina nos enviase uno diferente.

Así que tenemos que hacer estos cambios:

Poder procesar varios archivos
Agrupar toda la información
Tener un procesador extra para Json
Elegir el procesador según el archivo

Los dos últimos puntos encajan con un patrón Strategy: necesitamos poder escoger entre varios algoritmos en tiempo de ejecución (un lector de CSV y un lector de JSON). Para ello, tenemos que disponer de esos distintos algoritmos y un mecanismo que sepa cuál escoger en cada caso.

La cuestión ahora es hacer un refactoring del código actual hasta introducir este patrón para una sola estrategia, que es la que tenemos ahora. Comprobamos que todo el comportamiento actual se mantiene y una vez consolidados los cambios, introducimos la estrategia para otros formatos de archivo. Y, como veremos, este último paso esta vez será muy sencillo y tendrá poco riesgo.

¿Por qué proceder así? La idea de no mezclar las fases de refactoring e introducción de nuevas features busca reducir el riesgo de mezclar regresiones en el comportamiento actual y la introducción de nuevos bugs.

El refactor preparatorio estaría protegido por los tests existentes, de modo que si provocamos una regresión la detectaremos y podremos corregirla, deshaciendo el cambio que la provocó o haciendo las modificaciones necesarias. Por otro lado, una vez consolidado el refactor, la introducción de código nuevo puede ser asegurada mediante TDD o, si no, con test posteriores. De este modo, las fuentes de posibles errores se mantienen separadas y son fáciles de identificar y corregir.

Extrayendo un objeto colaborador

Así que vamos a empezar. En OOP preferimos objetos pequeños con responsabilidades bien definidas que trabajan colaborando. En el ejemplo que tenemos, tendría sentido un objeto encargado de leer los datos de los archivos. Ahora mismo, eso ocurre en el método:

def obtain_consumptions(file_name)
    data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)
    data.map do |row|
        Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
    end
end

Fíjate que en el cuerpo del método no tenemos llamadas a otros métodos del objeto ConsumptionAnalyser lo que nos indicaría que tiene sentido extraerlo. Este refactor se llama Extract class y consiste básicamente en crear una clase nueva a la que se mueven los métodos deseados y usándolo donde la necesitemos.

Algunos IDE ofrecen una automatización de este refactor dependiendo del lenguaje. Pero esencialmente se hace asé:

Creamos la nueva clase
Copiamos y pegamos en ella los métodos escogidos
Adaptamos lo que sea necesario
Reemplazamos los métodos originales con llamadas a esta clase

La nueva clase se llamará CSVConsumptionsProvider.

class CsvConsumptionsProvider

end

Ahora, copiamos y pegamos el método obtain_consumptions:

class CsvConsumptionsProvider
    def obtain_consumptions(file_name)
        data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)
        data.map do |row|
            Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
        end
    end
end

Puede ser el momento de revisar el nombre del método y realizar otros ajustes que veamos necesarios. Por ejemplo:

class CsvConsumptionsProvider
    def from_file(file_name)
        data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)
        data.map do |row|
            Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
        end
    end
end

El último paso será introducir este objeto como colaborador de ConsumptionAnalyzer. Antes de eso, nos aseguramos de que los tests están pasando.

class ConsumptionAnalyzer
    def initialize(provider = CsvConsumptionsProvider.new)
        @provider = provider
    end

    CONSUMPTIONS_A_YEAR = 12

    def execute(file_name, deviation_factor = 1.4)
        normalized = @provider.from_file(file_name)
        offices = offices(normalized)
        outliers = outliers(deviation_factor, offices)

        puts outliers
        puts "Data sample #{normalized.size} rows"
        puts "Found #{outliers.size} outliers"
        puts "Found #{outliers.size / offices.size} per office"
    end

    # Code removed for clarity

end

Este cambio debería permitir que los tests pasen sin problema.

En este ejemplo, estamos haciendo que provider sea opcional, creando una instancia por defecto. En otros lenguajes, podemos hacer algo similar a lo que sigue:

def initialize(provider = nil)
    if provider.nil?
        @provider = CsvConsumptionsProvider.new
    else
        @provider = provider
    end
end

Preparándose para el patrón Strategy

Extraer funcionalidad a objetos colaboradores es una buena forma de darle estructura al código. Pero nuestro analizador todavía depende que la fuente de datos sea CSV.

Como no queremos depender directamente de una tecnología o formato específico, necesitamos introducir un Mediador. Un Mediador es un objeto que se introduce para romper la dependencia directa entre dos objetos. De este modo, uno puede evolucionar sin saber nada del otro. Ambos quedarán acoplados al mediador, pero es una dependencia más ligera.

Nuestro mediador representa la idea abstracta de un proveedor de consumos.

class ConsumptionProvider
    def from_file(file_name)
    end
end

Y en su primera implementación, simplemente hace uso del CSVConsumptionProvider.

class ConsumptionsProvider
    def initialize(provider = CsvConsumptionsProvider.new)
        @provider = provider
    end
    def from_file(file_name)
        @provider.from_file(file_name)
    end
end

Por supuesto, tenemos que cambiar la dependencia en ConsumptionAnalyzer:

class ConsumptionAnalyzer
    def initialize(provider = ConsumptionsProvider.new)
        @provider = provider
    end

    # Code removed for clarity
end

Todos estos cambios no alteran el comportamiento y los tests siguen pasando. Estamos casi terminando el refactor. El beneficio que hemos conseguido es que ahora, ConsumptionAnalyzer no tiene ni idea de que está leyendo datos de un archivo CSV, no hay ninguna referencia que haga pensar en ello.

Csv como Strategy

El siguiente paso sucede en ConsumptionsProvider y consiste en dejar de usar incondicionalmente CsvConsumptionsProvider. De momento, sabemos que el criterio para escoger un Provider concreto es el tipo de archivo, que podemos determinar por la extensión de su nombre. Eso es lo que vamos a introducir ahora:

class ConsumptionsProvider
    def initialize(provider = CsvConsumptionsProvider.new)
        @provider = provider
    end
    def from_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            return @provider.from_file(file_name)
        end
        raise NotImplementedError.new , "#{extension} file support not implemented"
    end
end

Esto puede parecer innecesario en este punto, ya que solo tenemos un tipo de proveedor. Sin embargo, creo que se puede entender por donde vamos. Este refactor nos ha dejado en una situación en la que introducir otro proveedor simplemente requeriría escribir una clase nueva y modificar la condición para que el programa lo reconozca.

Vamos a arreglar un poquito el código dado que sigue muy condicionado por tener una sola estrategia. Por ejemplo, así:

class ConsumptionsProvider
    def initialize

    end
    def from_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        raise NotImplementedError.new , "#{extension} file support not implemented"
    end
end

Esto nos permitirá crear un nuevo JsonConsumptionsProvider, por ejemplo, e incluirlo así:

class ConsumptionsProvider
    def initialize

    end
    def from_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        if extension == ".json"
            provider = JsonConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        raise NotImplementedError.new , "#{extension} file support not implemented"
    end
end

Pero no nos adelantemos. Primero necesitamos tener un proveedor y antes de eso hay que prepararse para otro comportamiento.

Agregar todos los resultados

Uno de los requisitos que nos piden es agregar todos los resultados, lo que significa que nos pasarán una lista de archivos de los que obtener datos y debemos proporcionar una salida única.

Así que tenemos que dar soporte a poder indicar varios archivos en ConsumptionAnalyzer.execute y en ConsumptionsProvider.from_file. Además, en este último, tenemos que obtener los datos y agregarlos antes de entregarlos.

Vamos por partes. Una forma fácil de permitir varios archivos en ConsumptionsProvider.from_file es cambiar el parámetro file_name con splat operator. De ese modo, podemos pasarle una lista de nombres de archivo y se comportará como un array.

Para eso, nos viene bien extraer el procesamiento de cada archivo individual en un método. Es decir. Ahora estamos así:

class ConsumptionsProvider
    def initialize

    end
    def from_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        raise NotImplementedError.new , "#{extension} file support not implemented"
    end
end

Y nos preparamos haciendo esto:

class ConsumptionsProvider
    def initialize

    end
    def from_file(file_name)
        read_file(file_name)
    end

    def read_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        raise NotImplementedError.new, "#{extension} file support not implemented"
    end
end

Este cambio, que es un Extract method, no afecta al comportamiento actual. Ahora vamos con from_file.

class ConsumptionsProvider

    def from_file(*file_names)
        data = []
        file_names.each do |file_name|
            data = read_file(file_name)
        end
        data
    end

    # Code removed for clarity
end

Ahora, from_file acepta cualquier número de parámetros y los empaqueta como un array. Sencillamente, recorremos el array de nombres de archivo y vamos leyendo cada uno. Este cambio es temporal, porque aún no hemos cambiado el Analizador para dar soporte a múltiples archivos.

En este ejemplo de código hemos asumido que solo se va a pasar un archivo, pero lo más adecuado sería hacer lo siguiente: Añadir cada conjunto de datos que leemos al array que vamos a devolver.

class ConsumptionsProvider
    def from_file(*file_names)
        data = []
        file_names.each do |file_name|
            data.push(*read_file(file_name))
        end
        data
    end

    # Code removed for clarity
end

Cambiar el método execute de ConsumptionAnalyzer va a ser un poco más complicado. En este caso, el operador splat requiere que cambiemos la signatura del método, puesto que este operador solo puede usarse en el último parámetro. Por esa razón, tendríamos que invertir el orden de los parámetros.

En lenguajes como Ruby este refactor puede no estar automatizado, debido a su naturaleza dinámica. En Java, nos bastaría con hacer sobrecarga del método, añadiendo otra signatura. En otros lenguajes, con el refactor automatizado no hay mucho de qué preocuparse.

Sin embargo, podemos hacer el refactor paso a paso. Dependiendo de los usos que tengamos actualmente del método puede ser más o menos complicado. De hecho, puede haber diferentes formas de hacerlo.

Lo primero que hacemos es verificar los usos que tenemos ahora. Básicamente son dos. En ambos casos pasamos los dos parámetros, incluyendo deviation_factor que es opcional. Podríamos eliminar la opcionalidad, ya que no estamos haciendo uso de ella.

def execute(file_name, deviation_factor)
    # Code removed for clarity
end

Lo siguiente sería introducir un parámetro extra a través del cual podamos pasar los nombres de archivo. El operador splat hace que el parámetro actual como si fuese opcional, permitiéndonos no pasar nada en su lugar. De este modo, se respeta el uso que estamos haciendo actualmente.

def execute(file_name, deviation_factor, *files)
    # Code removed for clarity
end

A continuación, voy a hacer un cambio temporal que nos prepare el camino para dejar de usar el primer parámetro. En caso de que no pasemos nada en files, se usa lo que venga en file_name y se pasa como un array deconstruido.

def execute(file_name, deviation_factor, *files)
    files.append(file_name) if files.size == 0
    normalized = @provider.from_file(*files)

    # Code removed for clarity
end

Ahora podríamos ir sustituyendo los usos de este método para que pasen los nombres de archivo por files, en lugar de por file_name. En nuestro ejemplo son dos usos, por lo que es algo que podemos hacer de inmediato. Gracias al último cambio, sabemos que estamos usando los archivos pasados a través de files.

a = ConsumptionAnalyzer.new
a.execute('../sample.csv', deviation, '../sample.csv')

El otro uso es el test:

RSpec.describe 'Consumer Analyzer' do
    context "Default behaviour" do
        it "should generate report" do
            a = ConsumptionAnalyzer.new
            result = capture_stdout {a.execute('sample.csv', 1.4, 'sample.csv')}
            expect(result).to match_snapshot('default_snapshot')
        end
    end
end

Finalmente, una vez comprobado que todo funciona correctamente, eliminamos el uso del parámetro file_name.

a = ConsumptionAnalyzer.new
a.execute(deviation, '../sample.csv')


RSpec.describe 'Consumer Analyzer' do
    context "Default behaviour" do
        it "should generate report" do
            a = ConsumptionAnalyzer.new
            result = capture_stdout {a.execute(1.4, 'sample.csv')}
            expect(result).to match_snapshot('default_snapshot')
        end
    end
end


    def execute(deviation_factor, *files)
        normalized = @provider.from_file(*files)

        # Code removed for clarity
    end

Ten en cuenta que todos estos pasos los hemos dado sin que en ningún momento los tests dejasen de funcionar. En un ejemplo tan pequeño como este, podríamos haberlo completado sin tanta ceremonia, pero en un proyecto medianamente grande, proceder paso a paso te garantiza que el refactor sea seguro, dando pequeños pasos que no tienen efectos negativos.

Crear una nueva estrategia

Para esta serie de artículos preparé un generador de datos aleatorios que ahora tendré que modificar para que guarde los archivos en json. Con esto, puedo generar un ejemplo sencillo. Me da igual el contenido porque solo necesito que se puedan leer los datos.

[
  {
    "office": 1,
    "year": 2023,
    "month": 1,
    "consumption": 8379097
  },
  {
    "office": 1,
    "year": 2023,
    "month": 2,
    "consumption": 9539936
  },
  {
    "office": 1,
    "year": 2023,
    "month": 3,
    "consumption": 2025802
  },
  {
    "office": 1,
    "year": 2023,
    "month": 4,
    "consumption": 1398801
  },
  {
    "office": 1,
    "year": 2023,
    "month": 5,
    "consumption": 6572861
  },
  {
    "office": 1,
    "year": 2023,
    "month": 6,
    "consumption": 7942753
  },
  {
    "office": 1,
    "year": 2023,
    "month": 7,
    "consumption": 2569213
  },
  {
    "office": 1,
    "year": 2023,
    "month": 8,
    "consumption": 4575579
  },
  {
    "office": 1,
    "year": 2023,
    "month": 9,
    "consumption": 5742751
  },
  {
    "office": 1,
    "year": 2023,
    "month": 10,
    "consumption": 6769903
  },
  {
    "office": 1,
    "year": 2023,
    "month": 11,
    "consumption": 6564423
  },
  {
    "office": 1,
    "year": 2023,
    "month": 12,
    "consumption": 2062790
  }
]

Para crear este provider, haré un test leyendo de este archivo que se llama example.json. Lo más correcto sería utilizar una librería como FakeFS, que nos permite trabajar en un sistema de archivos virtual, o aplicar alguna otra idea que nos evitase tener que tocar el sistema de archivos. Pero puesto que añade una complejidad que va más allá de los objetivos de estos artículos, prefiero usar el método más sencillo.

En principio sería un poco absurdo testear esto a base de conseguir leer el archivo, obtener el output en forma de array de Consumption y verificar que cada uno de los objetos se ha creado bien. Así que básicamente, lo que quiero es comprobar que se leen todos los registros del archivo y que se pueblan correctamente.

Este debería servir para probar el primer punto.

RSpec.describe JsonConsumptionProvider do
    it "should read all records in file" do
        provider = JsonConsumptionProvider.new
        consumptions = provider.from_file("example.json")
        expect(consumptions.size).to eq(12)
    end
end

Hagamos una implementación fake, solo para probar que el test funciona:

class JsonConsumptionProvider
    def from_file(filename)
        Array.new(12, Consumption.new)
    end
end

Ahora que tenemos una línea base de comportamiento, vamos introduciendo cambios. Primero queremos leer los datos del archivo. Si podemos abrir el archivo en modo de lectura, ya tenemos un paso.

class JsonConsumptionProvider
    def from_file(filename)
        f = File.new(filename, "r")
        Array.new(12, Consumption.new)
    end
end

Ahora, obtengamos los datos. Hasta aquí todo parece funcionar y data debería contener un array de hashes:

class JsonConsumptionProvider
    def from_file(filename)
        f = File.new(filename, "r")
        raw = f.read
        data = JSON.parse(raw)
        f.close
        Array.new(12, Consumption.new)
    end
end

Vamos a ver si son 12:

class JsonConsumptionProvider
    def from_file(filename)
        f = File.new(filename, "r")
        raw = f.read
        data = JSON.parse(raw)
        f.close
        Array.new(data.size, Consumption.new)
    end
end

Resulta que sí. La primera parte parece conseguida. Nuestro provider es capaz de leer datos del archivo y, aparentemente, logra leer los 12 registros. Introducimos tests para ver si los lee correctamente:

RSpec.describe JsonConsumptionProvider do
    it "should read all records in file" do
        provider = JsonConsumptionProvider.new
        consumptions = provider.from_file("example.json")
        expect(consumptions.size).to eq(12)
    end

    it "should read data in first record" do
        provider = JsonConsumptionProvider.new
        consumptions = provider.from_file("example.json")
        expected = Consumption.new
        expected.office = 1
        expected.year = 2023
        expected.month = 1
        expected.consumption = 8379097

        expect(consumptions[0]).to eq(expected)
    end
end

Como es de esperar, este test no va a pasar. Los datos los hemos copiado del archivo, porque el comportamiento que esperamos es que se generen los objetos Consumption con esos mismos datos.

Lo hacemos pasar con este código:

class JsonConsumptionProvider
    def from_file(filename)
        f = File.new(filename, "r")
        raw = f.read
        data = JSON.parse(raw)
        f.close

        consumptions = []

        data.each do |h|
            c = Consumption.new
            c.office = h["office"]
            c.year = h["year"]
            c.month = h["month"]
            c.consumption = h["consumption"]
            consumptions.append(c)
        end
        consumptions
    end
end

Este código es lo bastante general como para convertir correctamente todos los registros leídos. Podemos introducir otro test, pero no va a aportarnos información nueva:

RSpec.describe JsonConsumptionProvider do
    it "should read all records in file" do
        provider = JsonConsumptionProvider.new
        consumptions = provider.from_file("example.json")
        expect(consumptions.size).to eq(12)
    end

    it "should read data in first record" do
        provider = JsonConsumptionProvider.new
        consumptions = provider.from_file("example.json")
        expected = Consumption.new
        expected.office = 1
        expected.year = 2023
        expected.month = 1
        expected.consumption = 8379097

        expect(consumptions[0]).to eq(expected)
    end

    it "should read data in last record" do
        provider = JsonConsumptionProvider.new
        consumptions = provider.from_file("example.json")
        expected = Consumption.new
        expected.office = 1
        expected.year = 2023
        expected.month = 12
        expected.consumption = 2062790

        expect(consumptions[11]).to eq(expected)
    end
end

Con esto, ya tenemos un nuevo proveedor. Y sabemos que funciona correctamente.

Juntarlo todo

Ya casi estamos listas para unir todas las piezas. Todo el trabajo que nos quedaría lo podemos hacer aquí:

class ConsumptionsProvider
    # Code removed for clarity

    def read_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        raise NotImplementedError.new, "#{extension} file support not implemented"
    end
end

A primera vista, una opción posible es reproducir la estructura if para introducir el proveedor JSON.

def read_file(file_name)
    extension = File.extname(file_name)
    if extension == ".csv"
        provider = CsvConsumptionsProvider.new
        return provider.from_file(file_name)
    end
    if extension == ".json"
        provider = JsonConsumptionsProvider.new
        return provider.from_file(file_name)
    end
    raise NotImplementedError.new, "#{extension} file support not implemented"
end

Esto debería funcionar. De hecho, el snapshot test original sigue pasando. Lo adecuado sería introducir un nuevo test para verificar que todo funciona. En este caso, generaría un nuevo archivo de ejemplo en formato json.

Este es el nuevo test, donde se puede ver como paso los dos nombres de archivo con el diferente formato de datos. Por cierto, que me ha servido para corregir algunos errores de nombres por todo el código.

RSpec.describe 'Consumer Analyzer' do
    # Code removed for clarity

    context "Two sources" do
        it "should generate mixed report" do
            a = ConsumptionAnalyzer.new
            result = capture_stdout {a.execute( 1.4, 'sample.csv', 'sample_2.json')}
            expect(result).to match_snapshot('two_sources')
        end
    end
end

Dado que sabemos que el proveedor de Json lee correctamente los datos, por el test unitario, y que el análisis también funciona correctamente, por el primer test de snapshot, podemos confiar en que el comportamiento es correcto y este test nos vale.

Ya hemos desarrollado la funcionalidad deseada y, si te das cuenta, hemos pasado más trabajo refactorizando que implementando las nuevas capacidades. Podrías pensar que es un desperdicio pero ten en cuenta que:

El refactor nos ha garantizado que añadir la nueva funcionalidad no iba a perjudicar el comportamiento existente
El trabajo de crear el nuevo proveedor de datos ha sido muy sencillo
En el futuro, será igualmente sencillo añadir soporte para nuevos formatos de archivo

Pero es que incluso puede ser más sencillo si hacemos un poco de refactor a posteriori.

Rematando el trabajo con otro refactoring

Echemos un vistazo:

class ConsumptionsProvider
    def initialize

    end
    def from_file(*file_names)
        data = []
        file_names.each do |file_name|
            data.push(*read_file(file_name))
        end
        data
    end

    def read_file(file_name)
        extension = File.extname(file_name)
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        if extension == ".json"
            provider = JsonConsumptionsProvider.new
            return provider.from_file(file_name)
        end
        raise NotImplementedError.new, "#{extension} file support not implemented"
    end
end

Si quisiera añadir soporte para un nuevo tipo de archivo, por ejemplo, un XML, tengo que crear una clase Provider y modificar ConsumptionsProvider. Cierto que esta modificación está bastante controlada, pero imagina no tener que tocarla para nada. Veamos otra forma de organizar este código.

Empecemos por hacer un cambio en la forma en que tratamos el provider.

def read_file(file_name)
    extension = File.extname(file_name)
    provider = nil
    if extension == ".csv"
        provider = CsvConsumptionsProvider.new
        return provider.from_file(file_name)
    end
    if extension == ".json"
        provider = JsonConsumptionsProvider.new
        return provider.from_file(file_name)
    end
    if provider.nil?
        raise NotImplementedError.new, "#{extension} file support not implemented"
    end
end

Ahora, podemos mover el return, que es igual en todas las ramas fuera de la estructura condicional.

def read_file(file_name)
    extension = File.extname(file_name)
    provider = nil
    if extension == ".csv"
        provider = CsvConsumptionsProvider.new
    end
    if extension == ".json"
        provider = JsonConsumptionsProvider.new
    end
    if provider.nil?
        raise NotImplementedError.new, "#{extension} file support not implemented"
    end
    provider.from_file(file_name)
end

Al fin y al cabo, el método read_file hace dos cosas:

Decidir qué proveedor utilizar.
Ejecutar el proveedor.

Así que separemos ambas responsabilidades.

def read_file(file_name)
    provider = select_provider(file_name)
    provider.from_file(file_name)
end

def select_provider(file_name)
    extension = File.extname(file_name)
    provider = nil
    if extension == ".csv"
        provider = CsvConsumptionsProvider.new
    end
    if extension == ".json"
        provider = JsonConsumptionsProvider.new
    end
    if provider.nil?
        raise NotImplementedError.new, "#{extension} file support not implemented"
    end
    provider
end

El método select_provider es básicamente una factoría, la cual podríamos extraer a otro objeto. Ya hemos visto el refactor Extract class, por lo que te voy a mostrar el resultado:

class ProviderFactory
    def make_provider(file_name)
        extension = File.extname(file_name)
        provider = nil
        if extension == ".csv"
            provider = CsvConsumptionsProvider.new
        end
        if extension == ".json"
            provider = JsonConsumptionsProvider.new
        end
        if provider.nil?
            raise NotImplementedError.new, "#{extension} file support not implemented"
        end
        provider
    end
end

Y aquí su uso:

class ConsumptionsProvider
    def initialize(factory = ProviderFactory.new)
        @factory = factory
    end
    def from_file(*file_names)
        data = []
        file_names.each do |file_name|
            data.push(*read_file(file_name))
        end
        data
    end

    def read_file(file_name)
        provider = select_provider(file_name)
        provider.from_file(file_name)
    end

    def select_provider(file_name)
        @factory.make_provider(file_name)
    end
end

Los objetos factoría están, por decirlo así, en las fronteras del dominio, así que la vida allí es un poco más salvaje. Por ejemplo, es más tolerable modificar la factoría que modificar ConsumptionsProvider.

Podemos replantear un poco el código de ProviderFactory:

class ProviderFactory
    def initialize
        @providers = {
          ".csv": CsvConsumptionsProvider.new,
          ".json": JsonConsumptionsProvider.new,
        }
    end

    def make_provider(file_name)
        extension = File.extname(file_name).to_sym
        unless @providers.key? extension
            raise NotImplementedError.new, "#{extension} file support not implemented"
        end
        @providers[extension]
    end
end

Este cambio mantiene el mismo comportamiento del programa y simplifica enormemente su mantenimiento, ya que basta con añadir una entrada al diccionario de @providers.

Una posible mejora sería añadir un método register(extension, provider), que nos permitiría añadir nuevos proveedores sin tocar esta clase, manteniendo, o no, el soporte por defecto a los actuales .json y .csv. O un poco de meta-programación para añadirlos automáticamente.

class ProviderFactory
    def initialize
        @providers = {
          ".csv": CsvConsumptionsProvider.new,
          ".json": JsonConsumptionsProvider.new,
        }
    end

    def register(extension, provider)
        @providers[extension] = provider
    end

    def make_provider(file_name)
        extension = File.extname(file_name).to_sym
        unless @providers.key? extension
            raise NotImplementedError.new, "#{extension} file support not implemented"
        end
        @providers[extension]
    end
end

Se podría utilizar así:

factory = ProviderFactory.new
factory.register(".csv", CsvConsumptionsProvider.new)
factory.register(".json", JsonConsumptionsProvider.new)

provider = ConsumptionsProvider.new(factory)

a = ConsumptionAnalyzer.new(provider)
a.execute(deviation, '../sample.csv')

Para resumir, con este refactor dejamos todo preparado para que en el futuro, añadir un nuevo tipo de fuente de datos requiera solo añadir código.

Conclusiones

Al principio del artículo señalábamos una línea de refactor que resultó irrelevante para la feature que nos habían pedido desarrollar. Empezar a trabajar en esa línea hubiera supuesto una pérdida de tiempo, sin aportar valor.

La tentación de refactorizar un código que sabemos que no está muy bien diseñado es muy fuerte. Sin embargo, en equipos de trabajo orientados a producto, el foco debe estar puesto en las mejoras y corrección de errores.

Por tanto, el refactor debería estar supeditado a estas necesidades. No solo para priorizar la entrega de valor, sino para que el refactor contribuya a ella de manera efectiva.

Un refactor aplicado sin contexto puede llevarnos por un camino indeseable, que haga más cara la entrega de valor porque hemos aplicado criterios a ese refactor que no se han visto confirmados por la evolución del negocio.

Por su parte, son los cambios en nuestro conocimiento del negocio los que deberían guiar el refactor. Si el conocimiento que adquirimos nos apunta en una dirección, el refactor debería seguirla.

Por otro lado, las acciones de refactor pueden suponer una buena parte del tiempo de desarrollo, pero asumiendo que tenemos el código protegido por tests, debería ser un tiempo de trabajo seguro que nos facilite introducir la nueva funcionalidad.

El refactor a posteriori, por su parte, es una inversión para el futuro, ya que puede ahorrarnos tiempo cuando tengamos que tocar de nuevo en esa área.

Puedes ver el código de ejemplo en este repositorio.

Refactor para quienes no refactorizan 2

Fran Iglesias — Wed, 29 Nov 2023 00:00:00 +0000

Segunda entrega de esta serie de introducción al refactoring, si es que no refactorizas. O si quieres impulsar esta práctica en tu equipo.

En la entrada anterior he intentado caracterizar el refactoring como una práctica técnica, deliberada, sistemática y metódica. En esta entrega me gustaría hablar de la oportunidad de la misma.

También en la entrada anterior he mencionado que en muchos equipos se habla de refactoring como una tarea específica para escribir desde otras bases un proyecto o parte de él, ya sea porque no se ha tocado en mucho tiempo, ya porque se es consciente de que su diseño no es bueno o por la razón que sea.

Para ello se intenta conseguir momentos o espacios, paralizando a veces el desarrollo de nuevas prestaciones. En parte porque a lo mejor el deterioro del código es tan grande que resulta carísimo en esfuerzo, tiempo y riesgo, añadir esas prestaciones.

Pero hemos dicho también que no podemos considerar refactoring esa forma de trabajar. Llámalo reescritura, rediseño o como quieres. Si implica, por así decir, parar el ritmo de entrega de valor, no es refactoring. Es otra cosa. Y no te digo que no sea necesaria, pero es otra cosa.

Decíamos que el refactoring consiste en realizar cambios pequeños e inocuos en un código a fin de mejorar su diseño. Por supuesto, podrías utilizar las técnicas de refactoring para abordar esos trabajos de reescritura de proyectos. O, dicho de otra forma, podrías abordarlo como refactoring, realizando cambios pequeños a medida que los necesitas sin romper el comportamiento actual. Pero eso es otra historia.

Hablemos de cuando se debería hacer realmente el refactoring.

El refactoring oportunista

Se puede hacer refactoring cuando estamos estudiando un código y observamos algo que nos hace enarcar una ceja, leer varias veces un trozo de código o necesitar un tiempo para interpretar qué está pasando ahí. Idealmente, deberíamos poder leer un código y entender qué está haciendo y cómo lo está haciendo.

El refactoring oportunista ocurre cuando nos encontramos con un fragmento de código que muestra claramente un smell y corregirlo nos puede ayudar a que el código esté en mejor estado. Pero, como veremos, es importante resistir la tentación de seguir profundizando en la madriguera del conejo.

El contexto habitual es estar leyendo código por la razón que sea. Imagina el refactor como una nota que añades en el margen para ayudarte en la lectura, sobre todo en la lectura futura. Pero no estás buscando cosas que arreglar en el código. Simplemente te lo encuentras.

Lo malo es que, a veces, encuentras cosas graves.

Un ejemplo con números mágicos

Veamos esta línea:

next if consumptions.size < 12

Esta línea significa que vamos a la siguiente fila si el array de consumptions tiene menos de 12 elementos. Pero, ¿por qué 12? Vale: 12 consumos por oficina en un año. De nuevo, tenemos un ejemplo del code smell número mágico. Una solución es darle un nombre:

# This goes outside of the class
CONSUMPTIONS_A_YEAR = 12

next if consumptions.size < CONSUMPTIONS_A_YEAR

Por desgracia, esta línea nos indica la existencia de otros problemas más graves:

¿Qué pasa si una oficina tiene menos de 12 consumos en un año porque abrió sus puertas en el segundo o tercer trimestre?
¿Y si el archivo de datos está desordenado?

Pero eso lo trataremos más adelante. Quizá lo más interesante es que al hacer el código más legible es también más fácil identificar aquellos casos en que el código no refleja bien el conocimiento del negocio.

Un ejemplo con condiciones complejas

Otro ejemplo es esta línea:

next unless (consumption - average).abs > standard_deviation * deviation_factor

La línea se lee: pasar al siguiente consumo a menos que se cumpla la condición (consumption - average).abs > standard_deviation * deviation_factor, en cuyo caso seguimos en el bloque. Ahora bien, para entender la condición puede que tengas que pensar un ratito en qué significa.

La condición establece que si la diferencia de ese consumo con la media es mayor que la desviación típica multiplicada por un cierto factor entonces debemos considerar ese consumo como un outlier.

Las expresiones condicionales complejas no se suelen catalogar como code smells, pero teniendo en cuenta que introducen una dificultad para seguir el flujo o relato del código, suele ser recomendable esconder es complejidad bajo un nombre descriptivo. Para eso, podríamos extraer toda la expresión condicional a un método de la clase.

next unless consumption_is_outlier(average, consumption, deviation_factor, standard_deviation)

Claro que esta solución no es mucho mejor. Un método con cuatro parámetros es también un smell, especialmente si son parámetros posicionales. Un cambio de orden, puede generar un error muy difícil de depurar.

Podemos probar con un refactor un poco más sencillo: introducir variable para reducir la complejidad de la expresión. El lado izquierdo representa la diferencia de consumo con la media:

(consumption - average).abs

Mientras que el lado derecho representa el límite para considerar esa diferencia como demasiado grande:

standard_deviation * deviation_factor

Podríamos hacer esto:

difference = (consumption - average).abs
boundary = standard_deviation * deviation_factor

next unless difference > boundary

Ahora la expresión condicional es mucho más clara y la línea se puede leer como: pasar al siguiente a menos que la diferencia sea mayor que el límite.

Podría ser más correcto formalmente mover estos cálculos a métodos, ya que nos ahorramos las variables temporales, pero entonces volvemos a tener problemas. Fíjate lo que pasa con difference:

next unless difference(average, consumption) > standard_deviation * deviation_factor

Humm… es como un sí, pero no, ¿verdad? No ganamos mucho. Es preferible pagar el precio de tener un par de variables temporales.

La lección es que, si bien podemos lograr pequeños triunfos a base de estos pequeños refactors oportunistas es importante no dejarse llevar por el entusiasmo e intentar refactorizarlo todo. Momentos adecuados no nos van a faltar, como veremos a continuación.

Aislar lo que está mejor aislado

Consideremos ahora esta línea:

data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)

Cada vez que pasamos por ella se nos enciende una pequeña alarma: seguramente algún día nos pedirán leer archivos XML, o Json, o algún otro formato. Está claro que aquí necesitaremos aplicar un patrón Strategy y tener diversos adaptadores, así como devolver los datos leídos en una estructura que no sea dependiente del soporte del que se ha leído.

Resiste el deseo de meterte en ese refactor hasta el fondo. ¿Qué ocurriría si nunca se da el caso de tener que leer otros formatos de archivo? Pues que habríamos perdido el tiempo programando cosas que no vamos a necesitar.

Sin embargo, hay un punto de razón en esa alarma. La variable data es de un tipo específico de una librería de Ruby para leer archivos CSV. Y el hecho de que la lectura del archivo esté plantada ahí, en el cuerpo del método principal resulta como poco molesto.

Pero podemos aplicar un refactor tan sencillo como Extract method para dejar las cosas un poquito mejor de lo que estaban:

class ConsumptionAnalyzer
  def initialize

  end

  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4) 
    data = obtain_readings(file_name)
    # Code removed for clarity  
  end

  def obtain_readings(file_name)
    CSV.parse(File.read(file_name), headers: true, converters: :numeric)
  end

  # Code removed for clarity
end

Esto ya nos quita de delante el detalle de como obtenemos las lecturas. Tener esto separado nos facilitará hacer cambios en el futuro.

Otro cambio interesante sería introducir nuestros propios tipos para los datos leídos, de modo que seamos independientes del uso de una u otra librería. Pero la magnitud del cambio es demasiado grande para un refactor oportunista.

De nuevo, no nos conviene comprometer más esfuerzo en profundizar en los cambios.

Recomendaciones sobre el refactor oportunista

Es muy importante tener la disciplina de decir que no al refactor cuando nos aleja del objetivo que estábamos persiguiendo en primer lugar.

El refactor oportunista se aplica cuando estamos leyendo código y encontramos pequeños escollos para entender lo que hace. El tipo de arreglos que podemos aplicar en ese momento tiene que ser muy limitado. Lo suficiente para resolver esa dificultad puntual que nos hacía difícil entender un fragmento en particular. Nada más.

El refactor preparatorio

El siguiente momento del refactor también requiere disciplina. El refactor preparatorio se da en el contexto de una intervención en el código para añadir o modificar funcionalidades o para corregir errores.

Una vez que tenemos claro qué debe hacerse, la intervención tendría tres partes separadas:

Refactor preparatorio
La intervención que añade, modifica o corrige
Refactor posterior

Voy a intentar explicarlo.

Cuando vamos a intervenir en el código puede ocurrir que nos demos cuenta de que para introducir el cambio deseado sería deseable que el código existente tuviese otro diseño. Esto es, con un diseño mejor, el cambio podría ser trivial, o podría consistir en añadir código en vez de modificar lo que hay, etc.

Por tanto, deberíamos ocuparnos primero de refactorizar el código actual para tener ese mejor diseño que hemos podido visualizar. Dicho de otra manera: rediseñamos el código actual manteniendo el comportamiento que tiene ahora. Eso es el refactor preparatorio.

Una vez hecho eso, aplicamos el cambio que deseábamos originalmente, que ahora será mucho más fácil y seguro.

El refactor preparatorio puede ser trabajoso. Durante el tiempo de refactor no hacemos nada por añadir la funcionalidad nueva o corregir el bug que teníamos o cualesquiera que fuese el objetivo de la intervención. Todo nuestro foco es mejorar la situación del código. Por eso, no debería sorprendernos que nos ocupe bastante tiempo y esfuerzo. Pero como podríamos estar protegidas por tests el riesgo es reducido.

No es nuestro caso, por cierto, así que igual deberíamos añadir un paso previo. Si no tenemos tests que protejan ese aspecto concreto, lo primero sería introducirlos de la mejor manera posible.

El refactor posterior

Una vez que hemos realizado la intervención deseada en el código puede ocurrir que veamos nuevas oportunidades de refactorizar a fin de facilitar nuevas intervenciones en el futuro.

Pero como hemos dicho otras veces, no se trata de imaginar todos los futuros posibles y empezar a programar cosas por si acaso algún día se usan. Se trata de asegurar que el código que dejamos se entiende bien y, en su caso, es fácil de cambiar cuando sea necesario.

Veamos algunos ejemplos

Teniendo claro que el refactor forma parte de la rutina de desarrollo, lo interesante es que sean las necesidades del negocio nos guíen a la hora de decidir qué y cuando refactorizar. Así que vamos a ver algunos ejemplos de trabajo realista con el código de nuestro analizador y de qué manera usamos los distintos momentos de refactoring.

Analizar correctamente cada oficina

Ahora que la gente de negocio ha estado probando el script se han dado cuenta de algunos problemas. El análisis se está haciendo por oficina y año y ellos quieren que se haga por la totalidad de consumos de una oficina a lo largo de su historia. Es más: una oficina podría no tener doce lecturas de consumo por año.

Esto es lo mismo que detectamos al hacer el primer refactor oportunista, pero ahora está claro cómo se hace la colección de datos en el programa y cómo se debería estar haciendo. Podemos ver la diferencia y entender por qué se está haciendo mal, lo que es el primer paso para arreglarlo.

La cuestión es que la situación actual del código no es muy adecuada para resolver el problema. En el mismo bucle coleccionamos las lecturas que vamos a analizar y con cada una de estas colecciones realizamos el análisis:

class ConsumptionAnalyzer
  def initialize

  end

  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR

      average = average(consumptions)
      standard_deviation = standard_deviation(consumptions)

      consumptions.each do |consumption|
        difference = (consumption - average).abs
        boundary = standard_deviation * deviation_factor

        next unless difference > boundary

        outlier = Outlier.new
        outlier.office = row["office"]
        outlier.consumption = consumption
        outlier.deviation = (consumption - average) / standard_deviation

        outliers.append(outlier)
      end

      consumptions = []
    end
    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end

  # Code removed for clarity
end

Nos convendría separar ambas responsabilidades: por una parte, recorrer los datos brutos para extraer las colecciones de lecturas, y luego recorrer esas colecciones y hacer el análisis de cada una. Este sería el refactor preparatorio. Una vez hecha esta separación, podríamos centrarnos en la forma en que se coleccionan los datos de cada oficina.

El problema es que realmente no es posible hacer este refactor de manera automática o probada. Para tejer una red de seguridad necesitamos introducir algún test. Ciertamente, va a suponer un coste grande, pero eso que nos ahorramos en el futuro.

Poniendo el código bajo test

Tenemos dos dificultades para poner el código bajo test. La primera es el hecho de que leemos un CSV directamente del sistema de archivos. No es lo más complicado, ya que afortunadamente podemos especificar su nombre. Gracias a eso, podríamos preparar uno a medida para el test que queremos hacer.

La segunda dificultad es el conseguir el output. Ahora mismo lo estamos enviando a la consola o stdout. En Ruby es posible hacer un apaño que nos permite capturar el output a stdout y así poder verificarlo. Podemos usar esta función invocando el código del cual queremos capturar el output.

def capture_stdout
  original = $stdout
  foo = StringIO.new
  $stdout = foo
  yield
  $stdout.string
ensure
  $stdout = original
end

Quedaría algo así:

a = ConsumptionAnalyzer.new
result = capture_stdout {a.execute('../test.csv', 1.4)}

Y luego podríamos verificar el resultado contra un ejemplo creado a mano. O generado por la propia aplicación. Bienvenidas al test de caracterización.

Los tests de caracterización son tests que en lugar de verificar el output de una función o de un programa contra un criterio que hayamos definido previamente, generamos un output ejecutando el código. Ese output o snapshot será el criterio contra el que verificaremos en el test.

Es decir: tomamos una muestra de lo que hace el código en su estado actual y usamos eso como criterio para asegurar que el refactoring que hagamos provoca cambios en él.

Obviamente, una vez que empecemos a introducir nuevos comportamientos o modificarlo, tendremos que actualizar el snapshot o desecharlo en cuanto podamos usar otro tipo de tests.

Existen diversas librerías en todos los lenguajes para hacer tests de snapshot. Approval tests nos proporciona varias, pero puedes encontrar alternativas fácilmente, aunque varía el grado de soporte. En este caso voy a probar rspec-snapshot, de Mike Levin, que debería ser suficiente para nuestro propósito. Una vez instalada en el proyecto, el test queda así:

# frozen_string_literal: true

require 'rspec'
require "rspec/snapshot"

require_relative '../lib/energy/consumption_analyzer'

RSpec.describe 'Consumer Analyzer' do
    context "Default behaviour" do
        it "should generate report" do
            a = ConsumptionAnalyzer.new
            result = capture_stdout {a.execute('sample.csv', 1.4)}
            expect(result).to match_snapshot('default_snapshot')
        end
    end
end

def capture_stdout
    original = $stdout
    foo = StringIO.new
    $stdout = foo
    yield
    $stdout.string
ensure
    $stdout = original
end

Se generará un archivo llamado default_snapshot.snap, que captura el output generado. Para controlar que todo está bien, introduzco un cambio tonto en el código, como es sumar una cantidad arbitraria a los consumos, y compruebo que el test falla.

outlier = Outlier.new
outlier.office = row["office"]
outlier.consumption = consumption + 123123
outlier.deviation = (consumption - average) / standard_deviation

En este caso, nos basta con este test. Pero es habitual que tengamos que hacer test combinatorios. Los test combinatorios nos permiten bombardear la unidad bajo test con cientos de ejemplos, combinando los parámetros que tenemos que pasarle. En ese sentido, la librería ApprovalTests, nos aporta una forma fácil de conseguirlo. Puedes leer más sobre eso en este otro artículo.

Elimino ese cambio y ya estamos listas para empezar a trabajar.

Cambio paralelo al rescate

De momento, vamos a seguir coleccionando los datos con el mismo criterio, pero con el código mejor organizado. Después, cambiaremos el criterio.

Para evitar que algo se rompa por el camino, vamos a usar una estrategia de cambio paralelo. Consiste en introducir código nuevo que no se usará hasta que tengamos la certeza de que funciona. En ese momento, dejamos de usar la forma antigua y pasamos a la nueva.

Mi primer paso va a ser coleccionar los consumos en un nuevo array. Lo que haré será guardarme todos los array consumptions que vaya generando.

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    offices = []
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR
      offices.append(consumptions)

      # Code removed for clarity
    end
    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end

Al final debería tener un array de offices con todos esos consumptions que he obtenido. Este cambio no debería afectar al test.

Ahora, voy a introducir un nuevo bucle que recorra el array de offices, aunque sin hacer nada, por el momento. Esto debería permitirme mover el código marcado, separando la parte de agrupar los consumos por oficina, que es la que queremos llegar a corregir, de la parte de hacer el análisis.

class ConsumptionAnalyzer
  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    offices = []
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR
      offices.append(consumptions)

      # Code to move 
      average = average(consumptions)
      standard_deviation = standard_deviation(consumptions)

      consumptions.each do |consumption|
        difference = (consumption - average).abs
        boundary = standard_deviation * deviation_factor

        next unless difference > boundary

        outlier = Outlier.new
        outlier.office = row["office"]
        outlier.consumption = consumption
        outlier.deviation = (consumption - average) / standard_deviation

        outliers.append(outlier)
      end
      # End of code to move

      consumptions = []
    end

    offices.each do | consumptions|

    end

    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end

  # Code removed for clarity
end

Tras cada uno de estos movimientos ejecuto el test para asegurarme de que no cambio el comportamiento.

Ahora muevo el código de un bucle a otro.

class ConsumptionAnalyzer
  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    offices = []
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR
      offices.append(consumptions)

      consumptions = []
    end

    offices.each do | consumptions|
        # Code to move 
        average = average(consumptions)
        standard_deviation = standard_deviation(consumptions)

        consumptions.each do |consumption|
            difference = (consumption - average).abs
            boundary = standard_deviation * deviation_factor

            next unless difference > boundary

            outlier = Outlier.new
            outlier.office = row["office"]
            outlier.consumption = consumption
            outlier.deviation = (consumption - average) / standard_deviation

            outliers.append(outlier)
        end
        # End of code to move     
    end

    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end

  # Code removed for clarity
end

Tengo un problema. En esta línea hago referencia a una variable interna al bucle anterior. Necesito arreglar esto antes.

outlier.office = row["office"]

Así que deshago el cambio para ver primero como dejar de depender de ese dato y poder mover el bloque.

Esta forma de proceder tiene un nombre: método mi-ka-do. Brevemente explicado, consiste en:

Introducir un cambio que deseamos en nuestro código y ejecutarlo o ejecutar sus tests.
Tomar nota del error que salga.
Deshacer el cambio y asegurarnos de que todo vuelve a funcionar como antes.
Modificar el código para prevenir que salga el error anterior, pero sin alterar el comportamiento actual.
Introducir de nuevo el cambio que queríamos.
Si todo va bien, hemos terminado.
Si aparece otro error, deshacemos el cambio y volvemos a realizar el proceso.

Me doy cuenta de que podría solventar el problema si en vez de un array utilizo un hash o diccionario, guardando los identificadores de la oficina como claves. Luego solo tendría que cambiar el segundo bucle para que tenga en cuenta la clave.

class ConsumptionAnalyzer
  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    offices = {}
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR

      office_id = row["office"]
      offices[office_id] = consumptions

      # Code to move 

      average = average(consumptions)
      standard_deviation = standard_deviation(consumptions)

      consumptions.each do |consumption|
        difference = (consumption - average).abs
        boundary = standard_deviation * deviation_factor

        next unless difference > boundary

        outlier = Outlier.new
        outlier.office = office_id
        outlier.consumption = consumption
        outlier.deviation = (consumption - average) / standard_deviation

        outliers.append(outlier)
      end

      # End of code to move     
      consumptions = []
    end

    offices.each do | office_id, consumptions|

    end

    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end

  # Code removed for clarity
end

Una vez hecha esta modificación, intentamos el cambio de nuevo. Pero falla. La intención del cambio es correcta, pero dado que limitamos los conjuntos de consumos a los de un año (12 consumos), los datos se van machacando y el resultado es incorrecto.

La clave del hash o diccionario tendría que contemplar esto, asi que la cambiamos para que nos proporcione la resolución necesaria:

class ConsumptionAnalyzer

  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    offices = {}
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR

      office_id = "#{row["office"]}-#{row["year"]}"
      offices[office_id] = consumptions

      # Code to move 

      average = average(consumptions)
      standard_deviation = standard_deviation(consumptions)

      consumptions.each do |consumption|
        difference = (consumption - average).abs
        boundary = standard_deviation * deviation_factor

        next unless difference > boundary

        outlier = Outlier.new
        outlier.office = office_id.split('-')[0].to_i
        outlier.consumption = consumption
        outlier.deviation = (consumption - average) / standard_deviation

        outliers.append(outlier)
      end
      # End of code to move
      consumptions = []
    end

    offices.each do | office_id, consumptions|

    end

    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end
  # Code removed for clarity
end

Aquí se genera el id

office_id = "#{row["office"]}-#{row["year"]}"

Y aquí se recupera el id “real” de la oficina

outlier.office = office_id.split('-')[0].to_i

Aplicamos el cambio y movemos el bloque al otro bucle. Esta vez, los tests pasan, confirmando que el cambio no altera el comportamiento actual. Y, además, durante el proceso hemos podido aprender algo sobre la solución de nuestro problema.

class ConsumptionAnalyzer

  CONSUMPTIONS_A_YEAR = 12

  def execute(file_name, deviation_factor = 1.4)

    data = obtain_readings(file_name)
    consumptions = []
    outliers = []
    offices = {}
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < CONSUMPTIONS_A_YEAR

      office_id = "#{row["office"]}-#{row["year"]}"
      offices[office_id] = consumptions

      consumptions = []
    end

    offices.each do | office_id, consumptions|
        # Code to move 

        average = average(consumptions)
        standard_deviation = standard_deviation(consumptions)

        consumptions.each do |consumption|
            difference = (consumption - average).abs
            boundary = standard_deviation * deviation_factor

            next unless difference > boundary

            outlier = Outlier.new
            outlier.office = office_id.split('-')[0].to_i
            outlier.consumption = consumption
            outlier.deviation = (consumption - average) / standard_deviation

            outliers.append(outlier)
        end
        # End of code to move
    end

    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end
  # Code removed for clarity
end

Ahora sería el momento adecuado de terminar este refactor con otro que nos ayude a limpiar un poco el código y separar los intereses de cada parte, aislando lo que queremos corregir. El paso lógico a continuación es separar las distintas fases del proceso en métodos con un nombre significativo. Ahora el método execute describe bastante mejor lo que hace, y tenemos separadas las distintas partes o fases del proceso. Esto suena a un refactor Split phase, que consiste básicamente en identificar y separar las fases de un proceso.

De hecho, parece que nos pide hacer algo con toda la zona de generar el output, aunque solo fuese por mantener la simetría.

class ConsumptionAnalyzer
    def initialize

    end

    CONSUMPTIONS_A_YEAR = 12

    def execute(file_name, deviation_factor = 1.4)
        data = obtain_readings(file_name)
        offices = offices(data)
        outliers = outliers(deviation_factor, offices)
        puts outliers
        puts "Data sample #{data.size} rows"
        puts "Found #{outliers.size} outliers"
        puts "Found #{outliers.size / 300} per office"
    end

    def outliers(deviation_factor, offices)
        outliers = []
        offices.each do |office_id, consumptions|
            average = average(consumptions)
            standard_deviation = standard_deviation(consumptions)

            consumptions.each do |consumption|
                difference = (consumption - average).abs
                boundary = standard_deviation * deviation_factor

                next unless difference > boundary

                outlier = Outlier.new
                outlier.office = office_id.split('-')[0].to_i
                outlier.consumption = consumption
                outlier.deviation = (consumption - average) / standard_deviation

                outliers.append(outlier)
            end
        end
        outliers
    end

    def offices(data)
        offices = {}
        consumptions = []
        data.each do |row|
            consumptions.append(row["consumption"])

            next if consumptions.size < CONSUMPTIONS_A_YEAR

            office_id = "#{row["office"]}-#{row["year"]}"
            offices[office_id] = consumptions
            consumptions = []
        end
        offices
    end

    # Removed code for clarity
end

Todavía nos queda un refactor

Pero debemos centrarnos en la tarea que veníamos a realizar: lo que queremos es recopilar los datos por oficinas. Por un lado, tenemos la ventaja de haber aislado esa fase del proceso.

Pero, por otro, todavía arrastramos las consecuencias de que data es un objeto muy acoplado al tipo de archivo CSV que estamos usando como fuente de datos. La consecuencia es que el método offices está también acoplado a ese formato concreto de archivo. En realidad, offices siempre va a estar acoplado al tipo de datos que produzca obtain_readings, que es de donde viene data. A este tipo de acoplamiento se le llama Connascence y hace referencia al grado en que dos componentes del sistema comparten un conocimiento que obligaría a uno de ellos a cambiar si el otro lo hace.

El problema es que el tipo de dato (CSV::Table y CSV::Row) está definido por una dependencia externa a nuestro código (la librería CSV). Si el tipo de datos es definido por nosotras, ese acoplamiento es menos grave. Se trataría de un principio similar al de Inversión de Dependencias.

Sería preferible que offices recibiera los datos con una estructura diferente así que antes de empezar a cambiar su comportamiento, sería importante refactorizar para que obtain_readings produzca esa estructura y offices la consuma.

¿Cómo hacer este refactoring sin tener problemas? Creo que lo primero sería definir la estructura de datos básica que queremos obtener. Esta estructura puede basarse en la que tenemos en el archivo CSV, pero podría ser diferente si nos va mejor o pensamos que no necesitamos todos los datos. Si en el futuro añadimos otras fuentes de datos ya las adaptaremos.

Consumption = Struct.new(:office, :year, :date, :consumption)

El segundo paso será normalizar los datos obtenidos del archivo CSV, recorriendo el array y generando uno nuevo con los datos convertidos. Para eso, introducimos un método con el que obtenemos un array con los datos de consumo normalizados a una estructura de datos definida por nosotras.

def normalize(data)
    data.map do |row|
        Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
    end
end

Aquí podrían plantearse cuestiones de performance, pero sería un momento prematuro para la optimización. Es mejor perder un poco ahora, conseguir un código mejor organizado y buscar luego las optimizaciones.

Finalmente, tendremos que pasarle los datos con la nueva estructura a offices, que es la parte interesada. Ya que este caso es bastante trivial podemos usar el método mi-ka-do: hacemos el cambio y si todo va bien, ya nos vale. En caso de ocurrir un error, deshacemos y lo volvemos a intentar. Y todo ha ido bien a la primera:

class ConsumptionAnalyzer
    CONSUMPTIONS_A_YEAR = 12

    def execute(file_name, deviation_factor = 1.4)
        data = obtain_readings(file_name)
        normalized = normalize(data)
        offices = offices(normalized)
        outliers = outliers(deviation_factor, offices)

        puts outliers
        puts "Data sample #{data.size} rows"
        puts "Found #{outliers.size} outliers"
        puts "Found #{outliers.size / 300} per office"
    end

    def normalize(data)
        data.map do |row|
            Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
        end
    end

    # Code removed for clarity

    def offices(data)
        offices = {}
        consumptions = []
        data.each do |row|
            consumptions.append(row.consumption)

            next if consumptions.size < CONSUMPTIONS_A_YEAR

            office_id = "#{row.office}-#{row.year}"
            offices[office_id] = consumptions
            consumptions = []
        end
        offices
    end

    # Code removed for clarity
end

Por fin, introducir la funcionalidad nueva

Consolidamos el cambio con un nuevo commit y ahora es cuando podemos empezar a trabajar en los cambios que nos han pedido, que se pueden resumir en cambiar la forma en que recolectamos los datos de las oficinas, y no separarlas por años.

Esto nos va a plantear un par de problemas. Tenemos el test de caracterización, el cual habíamos decidido introducir para protegernos durante el refactor y no cambiar el comportamiento de forma accidental.

Ahora queremos lo contrario, o sea, cambiar el comportamiento. Es más que posible que el resultado del análisis sea diferente cuando hagamos los cambios, por lo que tendríamos que verificar que el cambio funciona bien de otra forma. Y el test tal como está actualmente ya no nos serviría. Por una parte, podríamos prescindir de él. Por otra, podemos dejarlo en espera hasta realizar el cambio de algoritmo y generar el snapshot del nuevo comportamiento.

Pero para desarrollar este nuevo comportamiento, nos vendría bien hacer test unitarios que verifiquen que la selección de oficinas es correcta. Lo bueno, es que los refactors que hemos hecho nos han llevado a una situación en la que esto es posible, ya que hemos aislado ese proceso en un método de la clase cuyo input es fácil de manipular porque no es más que un array de objetos que hemos definido nosotras.

Hay un problemilla. Este método debería ser privado y, por tanto, no deberíamos testearlo directamente. Sin embargo, venimos de una situación mucho peor así que es preferible ignorar esto de momento, avanzar en la funcionalidad y ver si en el futuro podemos llegar a un mejor diseño.

Empecemos con los tests. Para empezar, voy a verificar que si solo tengo datos de una oficina, incluso de distintos años o meses, se agrupan todos.

RSpec.describe ConsumptionAnalyzer do
    it "should aggregate same office data" do
        data = [
          Consumption.new(1, 2021, 1, 123456),
          Consumption.new(1, 2023, 1, 134143),
          Consumption.new(1, 2023, 2, 542543),
          Consumption.new(1, 2021, 2, 123454),
          Consumption.new(1, 2021, 3, 123345),
          Consumption.new(1, 2022, 5, 534542),
          Consumption.new(1, 2022, 6, 534542),
          Consumption.new(1, 2022, 7, 534542),
          Consumption.new(1, 2022, 8, 534542),
          Consumption.new(1, 2022, 9, 534542),
          Consumption.new(1, 2022, 10, 534542),
          Consumption.new(1, 2022, 11, 534542),
        ]

        analyzer = ConsumptionAnalyzer.new

        offices = analyzer.offices(data)

        expect(offices.size).to eq(1)
    end
end

Este test no sirve de mucho porque pasa tal cual con el código actual. Así que vamos a hacer un test que sí debería fallar. La opción más fácil es poner menos de 12 consumos. Ahora mismo, la recolección se hace literalmente por docenas, pero en la especificación de la tarea, nos debería haber quedado claro que se quiere permitir que el número de datos de cada oficina no tenga que coincidir con el número de meses del año.

Así que, si tenemos una muestra de 3 ó 4 ejemplos ya nos valdría para provocar el fallo del test. De hecho, con un solo ejemplo nos llegaría para empezar:

RSpec.describe ConsumptionAnalyzer do
    it "should aggregate same office data" do
        data = [
          Consumption.new(1, 2021, 1, 123456),
        ]

        analyzer = ConsumptionAnalyzer.new

        offices = analyzer.offices(data)

        expect(offices.size).to eq(1)
    end
end

El test falla diciendo que no se ha recopilado nada en offices.

Hagamos un cambio un poco drástico:

def offices(data)
    offices = {}
    consumptions = []
    data.each do |row|
        consumptions.append(row.consumption)
        office_id = "#{row.office}-#{row.year}"
        offices[office_id] = consumptions
        consumptions = []
    end
    offices
end

Con este cambio, el test pasará. Claro que ahora el resultado tiene pinta de que puede ser un poco caos. Pero vamos paso a paso.

Ahora que me fijo en el código, me interesa cambiar la clave office_id, que debería ser solo el número de oficina contenido en Consumpion.office. Hagamos otro test para verificarlo:

RSpec.describe ConsumptionAnalyzer do
    it "should aggregate same office data" do
        data = [
          Consumption.new(1, 2021, 1, 123456),
        ]

        analyzer = ConsumptionAnalyzer.new
        offices = analyzer.offices(data)
        expect(offices.size).to eq(1)
    end

    it "should identify office by its number" do
        data = [
          Consumption.new(1, 2021, 1, 123456),
        ]

        analyzer = ConsumptionAnalyzer.new
        offices = analyzer.offices(data)
        expect(offices.key? "1").to be_truthy
    end
end

Y este es el cambio que necesitamos para que el test pase:

def offices(data)
    offices = {}
    consumptions = []
    data.each do |row|
        consumptions.append(row.consumption)
        office_id = "#{row.office}"
        offices[office_id] = consumptions
        consumptions = []
    end
    offices
end

Vamos a ver ahora como discriminar entre oficinas. O si el sistema es capaz de discriminar entre ellas. Si tenemos datos de varias oficinas, deberíamos tener eso en el resultado, podemos empezar por tan solo dos. De paso, nos aseguramos de tener ambas claves.

RSpec.describe ConsumptionAnalyzer do

    # Code removed for clarity

    it "should separate data from different offices" do
        data = [
          Consumption.new(1, 2021, 1, 123456),
          Consumption.new(2, 2022, 4, 154325),
        ]

        analyzer = ConsumptionAnalyzer.new
        offices = analyzer.offices(data)
        expect(offices.size).to eq(2)
        expect(offices.key? "1").to be_truthy
        expect(offices.key? "2").to be_truthy
    end
end

Ahora necesitaríamos saber que se añaden todas las lecturas de la misma oficina:

RSpec.describe ConsumptionAnalyzer do

    # Code removed for clarity

    it "should aggregate all data of an office" do
        data = [
          Consumption.new(1, 2021, 1, 123456),
          Consumption.new(2, 2022, 4, 154325),
          Consumption.new(1, 2021, 3, 173412),
          Consumption.new(1, 2021, 7, 109324),
        ]

        analyzer = ConsumptionAnalyzer.new
        offices = analyzer.offices(data)
        expect(offices["1"].size).to eq(3)
        expect(offices["1"][0]).to be(123456)
        expect(offices["1"][1]).to be(173412)
        expect(offices["1"][2]).to be(109324)
    end
end

Este test no pasa. Nos dice que falla porque la oficina “1” solo tiene una entrada y nosotros esperamos 3.

Si estudiamos el código, vemos que el problema puede ser que estamos acumulando consumos en el array consumptions, que es vaciado cada vez en el bucle.

def offices(data)
    offices = {}
    consumptions = []
    data.each do |row|
        consumptions.append(row.consumption)
        office_id = "#{row.office}"
        offices[office_id] = consumptions
        consumptions = []
    end
    offices
end

En su lugar, deberíamos coleccionar esos datos directamente.

def offices(data)
    offices = {}
    data.each do |row|
        office_id = "#{row.office}"
        offices[office_id].append(row.consumption)
    end
    offices
end

Pero esto tiene otro problema:

     NoMethodError:
       undefined method `append' for nil:NilClass

Necesitamos inicializar un array si la clave no existe todavía en el hash de oficinas.

def offices(data)
    offices = {}
    data.each do |row|
        office_id = "#{row.office}"
        if !offices.key? office_id
            offices[office_id] = []
        end
        offices[office_id].append(row.consumption)
    end
    offices
end

Y ahora el test pasa. En principio, con esto debería ser suficiente para tener lista la funcionalidad. Sin embargo, hay dos detalles que revisar.

El primero es que hay un cambio que podría afectar a la función outliers. Es otro caso de connascence: ambas funciones tienen que saber como se forma una clave. Si lo cambiamos en una, tenemos que cambiarlo en la otra. Voy a hacer el cambio sin más, porque es trivial.

outlier.office = office_id.split('-')[0].to_i

Pasaría a ser:

outlier.office = office_id

Da la casualidad de que funciona igualmente sin cambios. Pero no deberíamos fiarnos de las casualidades y, menos aún, dejar código que puede llevar a confusión.

El otro detalle es una preferencia personal: Ahora solo guardamos el valor del consumo, pero ¿por qué no guardar el objeto entero? Sería una idea interesante, pero implica un montón de cambios en outliers. Esta idea podría ser el objetivo de refactor posterior, pero puede que sea demasiado ambiciosa para este momento.

Nos queda regenerar el test de caracterización. Si lo tiramos vemos que da resultados distintos. Al acumular todas las medidas de cada oficina en una sola colección los índices estadísticos cambian y se detectan outliers diferentes.

Aprovecho para hacer un pequeño cambio en el resumen:

puts outliers
puts "Data sample #{data.size} rows"
puts "Found #{outliers.size} outliers"
puts "Found #{outliers.size / offices.size} per office"

El test de caracterización se puede regenerar simplemente borrando el snapshot existente. Al volver a ejecutar el test, se creará con los nuevos resultados.

Data sample 18000 rows
Found 3051 outliers
Found 10 per office

Refactor posterior y entrega

Ahora podemos realizar arreglos que dejen el código en mejor estado.

En Ruby existe la estructura unless, que nos permite invertir ciertas condicionales y dejarlas más legibles:

def offices(data)
    offices = {}
    data.each do |row|
        office_id = "#{row.office}"
        unless offices.key? office_id
            offices[office_id] = []
        end
        offices[office_id].append(row.consumption)
    end
    offices
end

O incluso usarlas como modificadores:

def offices(data)
    offices = {}
    data.each do |row|
        office_id = "#{row.office}"
        offices[office_id] = [] unless offices.key? office_id
        offices[office_id].append(row.consumption)
    end
    offices
end

Los métodos obtain_readings y normalize están fuertemente acoplados. Tanto es así que sería mejor unificarlos:

def obtain_readings(file_name)
    CSV.parse(File.read(file_name), headers: true, converters: :numeric)
end

def normalize(data)
    data.map do |row|
        Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
    end
end

En principio, puede parecer que se podría simplemente encapsular ambas en un único método, pero resulta que usamos data un poco más abajo para reportar el tamaño de la muestra de datos:

def execute(file_name, deviation_factor = 1.4)
    data = obtain_readings(file_name)
    normalized = normalize(data)
    offices = offices(normalized)
    outliers = outliers(deviation_factor, offices)

    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / offices.size} per office"
end

Sin embargo, eso podría obtener de normalized, que tiene el mismo número de filas. Una vez arreglado eso, la extracción del método queda así:

def execute(file_name, deviation_factor = 1.4)
    normalized = obtain_consumptions(file_name)
    offices = offices(normalized)
    outliers = outliers(deviation_factor, offices)

    puts outliers
    puts "Data sample #{normalized.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / offices.size} per office"
end

def obtain_consumptions(file_name)
    data = obtain_readings(file_name)
    normalize(data)
end

Lo que ocurre es que tanto obtain_readings como normalize son métodos que tengo mayor interés en tener aislados. Así que pienso que sería una buena idea aplicar el refactoring Inline function que, básicamente consiste en reemplazar una llamada a una función por el cuerpo de esa función. Justo lo contrario de Extract function:

def obtain_consumptions(file_name)
    data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)
    data.map do |row|
        Consumption.new(row["office"], row["year"], row["month"], row["consumption"])
    end
end

Esto nos facilitará dos cosas en el futuro: optimizar la lectura del archivo y dar soporte a otros tipos de archivos o proveedores de datos.

Fin, por ahora

Aunque veo algunas posibilidades de refactoring más, voy a dejar el artículo en este punto. Mi objetivo era mostrar cómo se integra el refactoring en el trabajo del día a día, cuando estamos creando un producto o servicio basado en software y queremos priorizar la entrega de nuevas funcionalidades, pero también la mejora de la calidad del código y el testing.

Nos vendría bien empezar a estructurar el código en archivos, etc. Pero de momento, no hemos tenido esa necesidad.

Mi plan es hacer algunas entregas más en las que, basándonos en posibles peticiones de negocio para iterar el producto, seguiremos añadiendo funcionalidad y refactorizando para que hacerlo sea más fácil y seguro cada vez.

Refactoring para quienes no refactorizan

Fran Iglesias — Sun, 26 Nov 2023 00:00:00 +0000

Me he dado cuenta de que no había escrito nunca un artículo sobre refactoring dirigido a personas que no saben lo que es, o que no lo entienden correctamente.

Refactoring es una práctica de ingeniería de software que consiste en modificar un código de tal forma que no se altera lo que hace. El objetivo es conseguir que ese código tenga un mejor diseño, que se entienda mejor cómo funciona y que sea más barato modificar su comportamiento o añadir funcionalidades en el futuro.

Como no se modifica el comportamiento, el refactoring puede desplegarse a producción, incluso aunque se trate de un cambio muy pequeño. Estos cambios pequeños, a medida que se van acumulando, consiguen transformar una base de código con defectos de diseño en otra más fácil de comprender, mantener y modificar.

El beneficio buscado con el refactoring es económico, no es una supuesta mejora estética y subjetiva del código. El refactoring persigue que la evolución futura de un código sea lo más barata posible:

Manteniendo mínimo el riesgo asociado al cambio al introducir cambios frecuentes, pequeños e inocuos y así reducir o minimizar la necesidad de introducir grandes cambios de código en producción cuando llega el momento de añadir, modificar o corregir funcionalidad.
Habilitando puntos de cambio en el código que nos permitan añadir funcionalidad o modificar la existente con el mínimo esfuerzo y tiempo posible.
Reflejando nuestro conocimiento de negocio en el código de la manera más actualizada y fiel, lo que facilita el on boarding de nuevas desarrolladoras, la introducción de nuevas prestaciones e incluso cambios grandes en la arquitectura.

La idea básica es que un esfuerzo de refactoring pequeño, pero constante, puede facilitarnos grandes cambios en el futuro. Es como cuando tenemos un jardín: si trabajamos unos minutos todos los días en él no nos encontraremos teniendo que hacer grandes esfuerzos para limpiarlo cuando pasado el tiempo vemos que se ha convertido en una jungla. A su vez, el tenerlo ordenado y limpio, nos facilitará introducir plantas nuevas o hacer reformas de su diseño.

Entendiendo el refactoring

Cuando escribimos un código por primera vez, especialmente si tenemos poca experiencia, lo más seguro es que no consigamos hacerlo de la mejor forma posible. Aunque funcione correctamente, puede que resulte difícil entender cómo hace lo que hace, o que si necesitamos hacer alguna modificación nos cueste mucho trabajo. Puede que incluso, al principio cualquier cambio genere errores inesperados.

El refactoring es la práctica en la que aplicamos diversas técnicas para conseguir ese cambio a mejor del código, sin que deje de hacer lo que estaba haciendo hasta ahora, evitando errores que hagan que el programa deje de funcionar o genere resultados incorrectos. El refactoring consiste en realizar pequeños cambios inocuos de tal forma que el programa siempre pueda funcionar, incluso poniendo estos cambios uno a uno en producción.

No es fácil encontrar una analogía de la práctica de refactoring en otras disciplinas. En ingeniería es habitual crear modelos o prototipos para validar hipótesis de diseño, pero también para refinar detalles. Sin embargo, esto no se hace con el producto final. En el software, en cambio, refactorizamos sobre un código que, de hecho, está en producción.

La mejor analogía que me viene a la mente es la de la escritura. Las escritoras revisan constantemente el texto para encontrar formas mejores de organizarlo, utilizar un vocabulario que exprese mejor la idea, mueven palabras, frases o párrafos para estructurar mejor el discurso y, en general, manipulan el texto hasta lograr que cumpla el fin deseado: informar, conmover, divertir, denunciar, movilizar, entretener…

Personalmente, el refactoring es un proceso que me resulta muy familiar, pues mucho antes de profundizar en el desarrollo de software, tenía experiencia en la escritura. Por tanto, la idea de revisar el código de forma contínua a fin de lograr que expresase una idea de la mejor manera me resultaba natural y casi automática. En mi cabeza no existía la posibilidad de escribir el código correcto a la primera, sino que la versión inicial siempre sería una especie de boceto o borrador que iría puliendo a medida que aumentaba mi entendimiento de la tarea.

Con el tiempo descubrí el libro fundacional de M. Fowler, Refactoring, que me ayudó a entender lo que estaba haciendo como una práctica intencional y metódica. Hasta entonces, podría decirse que mi proceso de refactoring era intuitivo. En la primera edición del libro, de hecho, hay algunas colaboraciones de otros autores que reflejan también la novedad que suponía abordar esta práctica de manera sistemática en el momento de su publicación. Aún hoy, que ya tenemos una segunda edición y decenas de recursos de otras muchas autoras, el refactoring no es una práctica de la industria tan implantada, sistemática y metódica como debiera.

El refactoring como práctica consciente y metódica

El refactoring no es solo el hecho de modificar partes del código con la finalidad de mejorar su diseño. Es necesario subrayar que se trata de una práctica intencional, consciente y metódica.

Es intencional y consciente porque debería formar parte de la rutina de trabajo. Refactorizamos para preparar el código cuando necesitamos introducir algún cambio. Antes de intentar siquiera añadir una modificación a un algoritmo, reorganizamos el código para que la forma de introducir esa modificación esa sencilla. Es un poco como cuando una cocinera corta y prepara todos los ingredientes antes de empezar a elaborar un plato. Al tener a mano todo lo necesario, el proceso de cocinado resulta sencillo y la cocinera puede centrarse en controlar los tiempos, punto de cocción y sazonado. Si tuviese que picar una verdura mientras se sofríe la cebolla, es posible que esta se pase y que el resultado sepa a quemado.

La práctica es metódica porque no se refactoriza de cualquier manera. Existen técnicas y mecánicas específicas para refactorizar. De hecho, algunas están lo bastante bien definidas como para que se puedan automatizar. De este modo, los mejores entornos de desarrollo nos permiten ejecutar estas transformaciones de manera automática, lo que garantiza que se realizan sin errores y sin afectar al funcionamiento del código transformado.

También es metódica porque el refactoring se puede aplicar a patrones que podemos encontrar en el código a los que llamamos code smells. Los code smells son ciertos síntomas que indican defectos de diseño de software que, aunque no perjudican la funcionalidad del código, sí que dificultan su comprensión y su mantenimiento. Muchos de estos smells se pueden resolver aplicando técnicas de refactoring específicas.

Lo que no es refactoring

Un cambio que modifica el comportamiento del código no es refactoring. Los cambios del refactoring se refieren a la estructura y el diseño del código, no a lo que hace. Por tanto, en un código razonablemente cubierto de tests, los cambios de refactoring no tienen efectos visibles. Nadie debería percibir que se ha realizado algún tipo de modificación.

Un cambio de grandes dimensiones en el código que afecta a numerosos archivos tampoco es refactoring. A ese tipo de cambios prefiero llamarlos reescrituras. Aunque mantengan el comportamiento, desde el punto de vista de los tests o de las usuarias de la aplicación, las reescrituras suelen ser un intento de volver a escribir el mismo software partiendo de distintos principios o intentando plasmar un nuevo diseño. Este tipo de cambios requieren tiempo y, si no se hacen aplicando las tácticas adecuadas, pueden introducir mucho riesgo, bloquear el desarrollo de nuevas prestaciones, etc.

De hecho, el refactoring puede conducir al mismo rediseño, pero en lugar de hacer un gran salto de fe, transforma el código aplicando pasos tan pequeños que no suponen riesgo.

En general, se podría decir que no es refactor si:

Cambiamos comportamiento
El cambio hace que fallen los tests
Se tiene que planificar
Obstaculiza el desarrollo
Los cambios afectan a más de dos o tres archivos

Hagamos un ejemplo

Creo que lo mejor es verlo con un ejemplo. Voy a intentar ponerme en la piel de una persona que está empezando a aprender a programar. Tal vez alguien que acaba de empezar en la industria a la que le encargan su primera tarea.

Ejercicio y primera iteración

Imaginemos que trabajamos para una compañía que tiene cientos de sucursales repartidas por todo el mundo y quiere optimizar su consumo de energía. En una primera fase se va a estudiar si hay consumos anormales dentro de cada una de esas oficinas, lo que podría indicar instalaciones defectuosas, mal uso, etc. El objetivo es detectar las que podrían requerir una atención urgente.

El criterio para clasificar un consumo com anormal es que se encuentra más de una desviación estándar por encima o por debajo de la media de consumo de ese local. Como el estudio está en sus primeros pasos, no se descarta modificar este criterio en el futuro, ni que se apliquen otros análisis a los mismos datos.

Para ello se recoge una muestra de datos en un archivo csv. Nuestro trabajo es procesar ese archivo y extraer una lista indicando todas las oficinas que presentan problemas, indicando sus consumos anómalos y el grado de desviación expresado como el número de desviaciones estándar.

Para preparar el artículo he añadido un generador aleatorio que nos permite generar archivos con datos aleatorios para un número dado de oficinas y años. Para cada oficina genera valores de consumo en tres rangos diferentes.

Aquí tenemos un ejemplo de los datos, para una única oficina durante un año.

office,year,month,consumption
1,2023,1,268199
1,2023,2,99242
1,2023,3,245126
1,2023,4,88012
1,2023,5,394065
1,2023,6,579409
1,2023,7,909539
1,2023,8,891502
1,2023,9,550299
1,2023,10,423113
1,2023,11,326505
1,2023,12,172286

En fin. La solución parece relativamente sencilla. Se trataría de leer un archivo CSV y obtener los datos, extraer los datos de cada oficina, calcular su media, su desviación típica, realizar las comparaciones adecuadas e ir guardando todos los hallazgos sospechosos.

Así que empezamos por crear este código, que consigue realizar la tarea requerida:

# frozen_string_literal: true
require "csv"

Outlier = Struct.new(:office, :consumption, :deviation)

class ConsumptionAnalyzer
  def initialize

  end

  def execute(file_name)

    data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)
    consumptions = []
    outliers = []
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < 12

      average = consumptions.sum(0.0) / consumptions.size
      sum = consumptions.sum(0.0) { |element| (element - average)**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      consumptions.each do |consumption|
        next unless (consumption - average).abs > standard_deviation

        outlier = Outlier.new
        outlier.office = row["office"]
        outlier.consumption = consumption
        outlier.deviation = (consumption - average) / standard_deviation

        outliers.append(outlier)
      end

      consumptions = []
    end
    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end
end

a = ConsumptionAnalyzer.new
a.execute("sample.csv")

No es ninguna maravilla, pero hace su trabajo. Aunque no teniendo tests, puede ser difícil de asegurar.

Así que cuando nos dan un archivo con algo más de 18.000 filas, que serían 300 oficinas durante 5 años, encuentra alrededor de 6.790 consumos problemáticos. Esto son cuatro o cinco lecturas sospechosas por oficina y año.

Segunda iteración, criterios algo más laxos

Nuestro pequeño programa funciona y hace su trabajo. Ahora bien, a la vista de los resultados, parece que una sola desviación típica para marcar un consumo como sospechoso pueda darnos muchos falsos positivos, por lo que nos piden cambiar el cálculo de forma que detecte desviaciones con un factor de 1.4 o mayores.

Parece un cambio fácil. Solo hay que tocar esta línea:

next unless (consumption - average).abs > standard_deviation

Y cambiarla por:

next unless (consumption - average).abs > standard_deviation * 1.4

Este es el resultado, el cual resulta más manejable. Así que nuestra responsable de proyecto está contenta, aunque comenta que sería interesante poder modificar ese valor a medida que se van tomando medidas de ahorro en las oficinas y se puede empezar a ser más exigente. “Bueno”, pensamos, “no es más que cambiar un valor en el código”.

Data sample 18000 rows
Found 2310 outliers
Found 7 per office

Lo que acabamos de hacer no ha sido un refactor. Hemos hecho un cambio de funcionalidad, para lo cual hemos tenido que cambiar el código del programa. Esto viola el principio de diseño de software conocido como Open/Closed: abierto para extensión y cerrado para modificación.

Lo que nos dice este principio es que para hacer este cambio de funcionalidad que acabamos de realizar, sería preferible no tener que modificar el programa.

El primer refactor

Tener que modificar el código para hacer que el comportamiento del software cambie siempre es un problema.

Supongamos que este programa de ahorro de energía se va a aplicar por áreas geográficas o países. Cada dirección regional necesita personalizar el programa debido a diferentes razones. Por ejemplo, en países con climas más extremos es posible que haya mayores consumos por calefacción en invierno, así que requerirán un sistema un poco menos sensible con sus datos que otros.

Así que si tenemos, por ejemplo cinco direcciones regionales necesitaremos cinco versiones diferentes del programa. Y eso únicamente teniendo en cuenta este pequeño aspecto. Y si fuese una aplicación web única para todos, tendrían que pedirnos que la cambiásemos para cada uso. No parece una solución ni útil ni escalable.

Lo ideal sería que cada dirección regional usase el mismo programa, pero con diferente configuración. La configuración es una de las formas en que un código puede estar abierto a extensión sin tener que modificarlo, ya que puede hacerse independiente del código del programa.

El problema que tenemos en este caso es que esta parte del algoritmo incluye un parámetro que está fijado en el propio código aunque, por su naturaleza, tiene sentido que su valor cambie. No es incorrecto en sí mismo, pues el código funciona, pero introduce una dificultad si necesitamos ajustarlo a otro valor.

next unless (consumption - average).abs > standard_deviation * 1.4

Esto es lo que llamamos un code smell: un aspecto del código que no es incorrecto per se, pero revela un problema subyacente que puede manifestarse cuando necesitamos cambiar el comportamiento de la unidad de código en que se encuentra.

El smell o síntoma es el hecho de que aparezca un número arbitrario en el código. Este en concreto tiene un nombre: número mágico o magic number. El problema inmediato es que es fácil perder la pista de su significado. Nosotras tenemos fresco lo que significa, pues acabamos de escribir el programa. Pero si otra persona tiene que ocuparse de ese cambio, puede encontrarse con dificultades para averiguar qué pinta ahí ese número.

Eliminar este code smell implica un cambio en el código que debe hacerse sin afectar a la funcionalidad actual. Por ejemplo, si ahora mismo el código detecta consumos inusuales mayores de 1.4 desviaciones estándar, una vez que hagamos el cambio, el programa debería detectar exactamente las mismas.

Para eliminar el smell tenemos varias soluciones que esencialmente consisten en darle un nombre a ese valor de forma que siempre podamos saber qué representa. Así que disponemos de tres posibles refactors que podríamos aplicar:

Introducir una constante : que aplicaremos cuando sepamos que ese valor no va a cambiar, al menos no en un futuro previsible. Tenemos ejemplos de constantes matemáticas y físicas, como PI, pero en nuestro negocio podrían existir otros valores constantes. No es nuestro caso porque los requisitos que nos piden es que se pueda cambiar.
Introducir una variable : que aplicaremos cuando ese valor puede cambiar y pueda proceder de una función usada en el ámbito de nuestra pieza de código. Aunque sabemos que el valor del que estamos hablando es variable, también sabemos que lo queremos cambiar desde fuera del propio código.
Introducir un parámetro : que será la solución cuando queremos que el valor venga de fuera de nuestra pieza de código, por lo que su lugar lógico es convertirse en un parámetro en la signatura de nuestra función o método.

Este último refactor tiene el objetivo de que podamos usar nuestro código de esta manera:

a = ConsumptionAnalyzer.new
a.execute("sample.csv", 1.4)

Por lo general, todos los refactorings tienen una mecánica específica. En algunos casos está lo bastante definida como para que se pueda automatizar. Así, muchos IDE nos proporcionan estos automatismos, de modo que simplemente tenemos que indicar el trozo de código que queremos cambiar y el editor hará el resto.

Introducir parámetro es uno de esos refactoring que está automatizado. Sin embargo, es muy sencillo de hacer. Añadimos el parámetro en la signatura del método execute y le ponemos un valor por defecto para asegurar que podemos usar el software exactamente igual que antes.

# frozen_string_literal: true
require "csv"

Outlier = Struct.new(:office, :consumption, :deviation)

class ConsumptionAnalyzer
  def initialize

  end

  def execute(file_name, deviation_factor = 1.4)

    # Removed for clarity

    data.each do |row|
      # Removed for clarity

      consumptions.each do |consumption|
        next unless (consumption - average).abs > standard_deviation * deviation_factor

        # Removed for clarity
      end

      consumptions = []
    end
    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end
end

Es decir, así:

a = ConsumptionAnalyzer.new
a.execute("sample.csv")

Una vez que comprobamos que la nueva versión funciona igual, podemos empezar a explotarla pasándole un parámetro. Para nuestro ejemplo, hemos creado una utilidad de línea de comandos a la que le podemos pasar el parámetro:

#!/usr/bin/env ruby
# frozen_string_literal: true

require_relative '../lib/energy/consumption_analyzer'

deviation = ARGV[0].to_f
deviation = 1.4 if deviation.zero?

a = ConsumptionAnalyzer.new
a.execute('../sample.csv', deviation)

Algunas reflexiones sobre este primer refactor

En realidad si nuestro código funcionase bien y no hubiese ninguna necesidad de cambiarlo, no estaríamos hablando de refactoring. Simplemente, el programa seguiría ahí, prestando su servicio, sin necesidad de tocarlo. Pero aunque eso es algo relativamente frecuente, también lo es que cuando introducimos un software, especialmente al principio, descubrimos limitaciones o posibilidades que nos impulsan a cambiarlo.

Por supuesto, lo ideal sería que fuese barato cambiar el código, entendiendo como barato que:

El tiempo y esfuerzo necesario para realizar el cambio sea el mínimo posible.
El riesgo de introducir defecto sea el mínimo posible, preferentemente cero.

Lo que acabamos de ver es que la existencia de code smells contribuye a incrementar el tiempo y el riesgo requerido para el cambio. Los motivos serían:

El code smell en sí dificulta la comprensión del código y, por tanto, hace que tardemos más en entender donde y cómo tenemos que aplicar el cambio necesario.
El estado del código dificulta introducir la nueva funcionalidad porque la estructura actual no contempla la posibilidad de una forma diferente de hacer las cosas.
Manipular el código implica la posibilidad de alterar su comportamiento que puede introducir errores o resultados no deseados.

Como hemos podido ver, antes de poder aplicar el cambio deseado, hemos tenido que resolver el code smell. Una vez reparado el código, introducir el cambio fue fácil.

Una vez hecho esto, lo primero que podría venirnos a la mente es algo así como: entonces podría ser buena idea identificar los code smells que haya en el código y arreglarlos. De este modo, en el futuro nos encontraremos con menos problemas para hacer cambios en el sistema.

A esta propuesta no le falta razón, pero tiene algunos inconvenientes. Para empezar, muchas veces no vamos a tener tiempo de hacer eso. El negocio se mueve y es más importante introducir nuevas funcionalidades o mejoras y arreglar errores. Perfectamente, puede ocurrir que identifiquemos code smells en áreas del código que no son importantes para el negocio, ya que funcionan como es debido y no se han necesitado cambios.

Por este motivo, es mucho más práctico limitar esto a las áreas del código que tenemos que tocar por necesidades del negocio. De hecho, es lo que ha ocurrido hace un momento. Nos han pedido que se pueda cambiar un cierto parámetro que originalmente estaba fijado por el código. Tuvimos que deshacer eso para ofrecer esa posibilidad. En este caso, la solución del code smell, el refactoring, coincidió prácticamente con la solución del problema.

Hay muchas posibles necesidades que pueden surgir en este pequeño proyecto que nos darían oportunidades para refactorizar todo el código. En algunos casos con más coste y en otro con menos. Así, se me ocurren:

Los datos podrían ser proporcionados en otros formatos: XML, Json
De hecho, ahora vienen en forma de archivos, pero en otros casos podrían obtenerse consultando una API, etc.
Quizá nos pidan entregar los resultados en un CSV o similar que se pueda abrir en una hoja de cálculo
O quizá nos pidan incluir más datos en el reporte
O incluso cambiar completamente el algoritmo, basándose en otra medida

Todos estos cambios podrían llegar a ser solicitados. Pero tampoco podemos obsesionarnos con imaginar todos los futuros posibles y anticiparlos. Como mucho, podemos asumir que llegarán cambios.

Por esa misma razón, es preferible actuar de una manera reactiva: refactorizar cuando surge la necesidad o cuando tenemos la oportunidad.

Donde nacen los code smells

Como hemos dicho más arriba, los code smells son ciertos patrones del código que señalan la presencia de defectos de diseño, dificultando el cambio del software a la hora de arreglar errores o modificar las funcionalidades. En sí mismos no son errores ni provocan un mal funcionamiento del código.

Pero el hecho de que aparezcan en nuestro código sería un indicador de un diseño inadecuado para el programa que tenemos entre manos y este puede venir motivado por varias causas.

Existen dos fuerzas que mueven el desarrollo de software: nuestro conocimiento del dominio o negocio que nos dice qué es lo que tenemos que programar, y nuestro conocimiento técnico, que nos dice cómo tenemos que implementarlo.

Nuestras carencias en cada una de ellas son las dos principales fuentes de problemas.

La deuda técnica

La deuda técnica sería provocada por nuestra falta de conocimiento del dominio o negocio en un momento dado. Esta falta de conocimiento no sería causada por desidia o desinterés, sino por la incertidumbre. Cuando ponemos un software en producción, puede que no tengamos una idea clara de como va a responder sus usuarias potenciales, puede que no conozcamos lo bastante de sus necesidades o de las soluciones que realmente necesitan.

Podríamos haber supuesto que ciertos parámetros se mantendrán constantes, o partir de ciertas hipótesis sobre cómo las usuarias interactuarán con el software. Podríamos asumir que ciertos aspectos del negocio cambian lentamente o, al contrario, que lo hacen con mucha frecuencia. Y así, un largo etcétera de aspectos de los que no sabemos mucho.

Por tanto, podemos empezar a desplegar el software con el objetivo de descubrir precisamente eso que no conocemos. En consecuencia el código reflejará una serie de asunciones por nuestra parte que podrían revelarse incorrectas o incompletas.

A esa diferencia entre el conocimiento real del negocio y lo que está reflejado en el código es a lo que llamamos deuda técnica. Cuando asumimos esa diferencia y ponemos código en producción de forma consciente, también asumimos que en algún momento tendremos que pagar esa deuda, cosa que haremos refactorizando para poder introducir tanto el nuevo conocimiento que hemos desarrollado sobre el negocio como las nuevas funcionalidades.

Con todo, la deuda técnica no es la principal causa de smells, ya que la deuda técnica bien manejada implica reconocer en el propio código la posibilidad de cambios en el futuro y preparar el código para que no sea muy costoso aplicarlos llegado el momento.

El mal diseño

La otra fuerza que mueve el desarrollo de software es nuestra pericia técnica. Es decir, nuestra capacidad para escribir un software que refleje el conocimiento del negocio de la mejor manera posible.

Si no tenemos mucha experiencia o ideas claras en el diseño de software y los principios que lo guían, lo más probable es que nuestro código presente muchos code smells. Cuando tenemos experiencia, podemos prevenir algunos de los más groseros. A veces, la experiencia nos sirve para tolerar algunos defectos de diseño basándonos principios de conveniencia, como sería el caso de asumir deuda técnica: aceptar el compromiso de tener que refactorizar esto en el futuro, mientras no tengo conocimiento suficiente para hacerlo mejor.

Sin embargo, una programadora novel o una con experiencia, pero con bajo interés en el diseño de software, introducirá muchos smells que, a la larga, dificultarán el progreso de ese software. Las prisas por salir a producción, o unas prácticas técnicas descuidadas, también nos llevarán a introducir muchos code smells.

La consecuencia es que se crearán diseños inflexibles, acoplados a tecnologías específicas, con el código mal organizado, etc, que incrementarán el coste del desarrollo.

Y esta situación empeora si carecemos de tests.

La carencia de tests

La falta de tests perjudica nuestras posibilidades de refactorizar código. Los tests nos proporcionarían la red de seguridad necesaria para hacer cambios sabiendo que en caso de alterar el comportamiento del programa, algún test dejaría de pasar y nos indicaría donde estamos introduciendo problemas.

Además, la falta de test favorece problemas de diseño. Para poder testear nuestro programa necesitamos una forma fácil de hacer el setup necesario para ejecutarlo. Si el código es fácil de poner bajo test, normalmente es indicativo de que tiene un diseño razonablemente bueno. Lo que no quiere decir que no sea mejorable.

Pero si no tenemos tests, es muy posible que haya muchos aspectos que habremos pasado por alto.

En nuestro ejemplo, para poder poner el código bajo test tendríamos que forzar un poco las cosas. Por un lado, generar un archivo de datos de ejemplo que contenga una muestra adecuada de lo que podemos esperar. Pero, además, tal como lo hemos escrito es complicado capturar el output del programa, ya que se lanza directamente a la consola. Aunque es posible hacerlo en Ruby, que es el lenguaje que hemos estado usando, implica un trabajo extra.

En resumen: el hecho de no haber tenido el testing en mente, y ya no estoy hablando de TDD, ha favorecido un mal diseño. O bien, debido al mal diseño, el código es difícil de poner bajo test. Y como no tenemos tests, introducir cambios conlleva un riesgo.

¿Qué podemos hacer entonces?

Refactor seguro y refactor probado

Poner un código bajo tests cuando no ha sido diseñado para ello es costoso y puede tener riesgos. El programa con el que estamos trabajando tiene ese problema. No es fácil ponerlo bajo test tal y como ha sido diseñado. El código no tiene estructura y está completamente incorporado en un solo método de un objeto, por lo que tampoco podemos poner bajo tests partes del mismo.

En estos casos podemos optar por refactors seguros o probados. ¿En qué consisten? Los refactors seguros o probados son aquellos que podemos aplicar con la confianza de que no se alterará el comportamiento de la pieza de software ni se introducirán errores, bien porque son automáticos, bien porque está probado que no introducen riesgo. Tenemos dos formas principales:

Refactor automático proporcionado por una herramienta de nuestro editor o entorno de desarrollo. Ya los hemos comentado anteriormente, la automatización nos proporciona la seguridad de que el refactor se aplicará de forma consistente.
Seguir una receta bien conocida. Con frecuencia, muchos refactorings de ese tipo están automatizados, pero cuando no es así, seguir los pasos de la receta paso a paso, garantiza que el refactor se realiza correctamente y no introduce errores.

Por otro lado, a fin de minimizar los riesgos lo mejor es seguir este procedimiento general y asumiendo que tenemos el código bajo control de versiones:

Hacer un commit del estado actual del código antes de iniciar el refactor, para poder revertir los cambios fácilmente en caso de problemas.
Ejecutar el refactor, ya sea automático o siguiendo la receta.
Comprobar que no se han introducido errores. Si es así, revertir los cambios y volver a empezar.
Si todo ha ido bien, consolidar el cambio, haciendo un nuevo commit.

Si el refactor lo hacemos por pasos, deberíamos hacer un commit por cada paso que hagamos siempre que no introduzca errores. En principio, las recetas de refactor probados no dejan nunca el código en estado inestable. Por esa razón, si introducimos un error, podemos deshacer ese cambio fácilmente y volver a un punto estable.

Mejorando nuestro código con refactors probados

Como hemos dicho, nuestro código reside en un único método de la clase ConsumptionAnalyzer de tal modo que es muy difícil de testear y modificar en su caso. ¿Podríamos convertirlo en un código más manejable al cual pudiésemos añadir tests, aunque sean parciales?

La respuesta es que sí. Disponemos de varios refactors que podríamos aplicar en este código y mejorar la situación. Pero antes de eso, me gustaría que nos fijásemos en algunos aspectos de la forma que tiene el código. A ver si los descubres:

# frozen_string_literal: true
require "csv"

Outlier = Struct.new(:office, :consumption, :deviation)

class ConsumptionAnalyzer
  def initialize

  end

  def execute(file_name, deviation_factor = 1.4)

    data = CSV.parse(File.read(file_name), headers: true, converters: :numeric)
    consumptions = []
    outliers = []
    data.each do |row|
      consumptions.append(row["consumption"])

      next if consumptions.size < 12

      average = consumptions.sum(0.0) / consumptions.size
      sum = consumptions.sum(0.0) { |element| (element - average)**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      consumptions.each do |consumption|
        next unless (consumption - average).abs > standard_deviation * deviation_factor

        outlier = Outlier.new
        outlier.office = row["office"]
        outlier.consumption = consumption
        outlier.deviation = (consumption - average) / standard_deviation

        outliers.append(outlier)
      end

      consumptions = []
    end
    puts outliers
    puts "Data sample #{data.size} rows"
    puts "Found #{outliers.size} outliers"
    puts "Found #{outliers.size / 300} per office"
  end
end

Lo primero es que tiene muchas líneas, creo que 29. Puede parecer que no es mucho, pero en mi opinión 29 líneas pueden ser demasiadas para un método. Esto es otro code smell llamado muy apropiadamente long method. No existe un límite objetivo de líneas para un método, pero cuando hay muchas, debería llevarnos a preguntarnos cosas como:

¿Contribuyen todas las líneas al objetivo del método?
¿Podríamos hacer grupos de líneas que colaboran en realizar una tarea?
¿Se ocupa el código de hacer varias cosas diferentes?

De hecho, hay otro rasgo que he visto definido a veces como code smell: las líneas en blanco para separar bloques de código que aparentemente se ocupan de cosas distintas. Nosotras podríamos usarlas como herramienta, como veremos a continuación.

Así que todo apunta a que el método está encargándose de varios trabajos o responsabilidades:

Obtener los datos de un archivo físico en formato CSV
Coleccionar los datos de una oficina
Calcular los índices estadísticos de media y desviación típica para…
Decidir si un consumo es excesivamente alto o bajo

Un refactor seguro que podemos hacer es el conocido como extraer método. Consiste en:

Agrupar todas las líneas que se ocupan de algún asunto en particular.
Identificar las variables o parámetros de los que depende ese bloque de líneas.
Identificar el resultado que esas líneas generan y que es usado por el resto del código a continuación.
Crear un nuevo método vacío cuyo nombre refleje la tarea que hacen esas líneas.
Copiar el grupo de líneas en el cuerpo del método recién creado.
Añadir en la signatura del método los parámetros necesarios.
Hacer que el método devuelva el resultado procesado por las líneas si procede.
Finalmente, reemplazamos el bloque de líneas por una llamada al método que acabamos de introducir.

Dicho así suena un poco complicado, sin embargo, es un refactor que suele estar automátizado.

Vamos a verlo paso a paso con algunos ejemplos.

Obtención de los índices estadísticos

Estas cuatro líneas hacen dos cosas diferentes: la primera calcula la media de los consumos y las otras tres, calculan la desviación típica.

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity

    data.each do |row|
      # Code removed for clarity

      average = consumptions.sum(0.0) / consumptions.size
      sum = consumptions.sum(0.0) { |element| (element - average)**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      # Code removed for clarity
      end
  end
end

Separémoslas:

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity

    data.each do |row|
      # Code removed for clarity

      average = consumptions.sum(0.0) / consumptions.size

      sum = consumptions.sum(0.0) { |element| (element - average)**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      # Code removed for clarity
      end
  end
end

La línea que calcula la media hace uso de dos datos que provienen del array consumptions. Así que depende completamente de este array. Podríamos extraer esta línea a un método lo que ocultaría los detalles del cálculo en este nivel, pero también nos permitiría hacer un test para verificar que el cálculo se hace correctamente.

El parámetro para ese método sería un array como el de consumptions.
El método debería devolver el valor calculado de la media aritmética.
Al método podríamos llamarlo simplemente average.

Creamos el método vacío:

class ConsumptionAnalyzer

  # Code removed for clarity

  def average(consumptions)

  end
end

Copiamos y pegamos las líneas implicadas. Nota: en Ruby no hace falta poner el return explícito cuando lo que se va a devolver es el último cálculo.

class ConsumptionAnalyzer

  # Code removed for clarity

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end
end

Finalmente, reemplazamos el bloque con la llamada:

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity

    data.each do |row|
      # Code removed for clarity

      average = average(consumptions)

      # Code removed for clarity
    end
    # Code removed for clarity
  end

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end
end

El otro bloque tiene varias características interesantes. Las variables sum y variance no se usan fuera de ese bloque. Son variables temporales. ¿Y sabes una cosa? Eso también puede ser un code smell y es un buen ejemplo de que son síntomas, no problemas en sí mismos. De hecho, la razón de tener variables temporales es que es más fácil entender lo que está pasando.

sum no es la suma de los consumos del array, sino la suma de los mínimos cuadrados de las diferencias de cada consumo con la media, lo que nos va a proporcionar un nuevo índice estadístico denominado varianza. De hecho, acto seguido usamos el valor de sum para calcular variance, la cual nos proporciona la desviación estándar. En otras palabras, estas líneas colaboran entre sí para calcular la desviación estándar de los datos, pero con nadie más en el código.

La presencia de estas variables temporales que solo se usan para almacenar durante un momento cálculos parciales nos indica que tendría sentido aislar esas líneas en un método ocupado de calcular la desviación típica.

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity
    data.each do |row|
      # Code removed for clarity

      average = average(consumptions)

      sum = consumptions.sum(0.0) { |element| (element - average)**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      # Code removed for clarity
    end
    # Code removed for clarity
  end

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end
end

Un elemento que nos llama la atención es que también se usa la media, recién calculada en la línea anterior. Parece lógico pasar la media ya calculada para calcular la desviación típica. Pero igual no es tan buena idea. Me explico: el cálculo solo tiene sentido si se hace sobre los mismos datos. Si pasamos el dato de la media calculada sería posible calcular una media con otros datos y la desviación típica con otros.

Si este bloque solo tuviese el array de datos sería capaz de calcular la media por sus propios medios, bien haciendo el cálculo directamente, bien invocando el método average que acabamos de introducir y que espera que le pasemos un array de datos.

Podríamos reescribir ese fragmento así:

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity
    data.each do |row|
      # Code removed for clarity

      average = average(consumptions)

      sum = consumptions.sum(0.0) { |element| (element - average(consumptions) )**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      # Code removed for clarity
    end
    # Code removed for clarity
  end

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end
end

Por cierto, esto es otro refactoring que tiene el nombre de inline variable y que, como hemos visto, es muy fácil de hacer. Basta con reemplazar el uso de una variable con el contenido de esa variable o, como en este caso, la expresión cuyo resultado se le asigna.

Es posible que alguien enarque una ceja pensando, ¿por qué hacer el cálculo dos veces si ya lo tenemos hecho? ¿No es perjudicial para el consumo de recursos? Podría ser, aunque también podríamos aplicar otras soluciones. Sin embargo, en este caso preferimos hacer independientes entre sí ambos cálculos. Esta discusión nos llevaría a hablar de acoplamiento, pero es un tema en el que ahora mismo prefiero no entrar.

Ahora que hemos aislado las líneas de código, vemos que:

Necesitarán recibir consumptions.
Devolverán la desviación estándar.
El método se llamará standard_deviation.

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity
    data.each do |row|
      # Code removed for clarity

      average = average(consumptions)

      sum = consumptions.sum(0.0) { |element| (element - average(consumptions) )**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      # Code removed for clarity
    end
    # Code removed for clarity
  end

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end

  def standard_deviation(consumptions)

  end
end

Copiamos y pegamos el cuerpo:

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity
    data.each do |row|
      # Code removed for clarity

      average = average(consumptions)

      sum = consumptions.sum(0.0) { |element| (element - average(consumptions) )**2 }
      variance = sum / (consumptions.size - 1)
      standard_deviation = Math.sqrt(variance)

      # Code removed for clarity
    end
    # Code removed for clarity
  end

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end

  def standard_deviation(consumptions)
    sum = consumptions.sum(0.0) { |element| (element - average(consumptions) )**2 }
    variance = sum / (consumptions.size - 1)
    standard_deviation = Math.sqrt(variance)   
  end
end

Reemplazamos el bloque copiado con la llamada. Ahora las variables temporales están limitadas a un contexto por lo que dejan de preocuparnos como smell.

class ConsumptionAnalyzer
  def execute(file_name, deviation_factor = 1.4)

    # Code removed for clarity
    data.each do |row|
      # Code removed for clarity

      average = average(consumptions)
      standard_deviation = standard_deviation(consumptions)

      # Code removed for clarity
    end
    # Code removed for clarity
  end

  def average(consumptions)
    consumptions.sum(0.0) / consumptions.size
  end

  def standard_deviation(consumptions)
    sum = consumptions.sum(0.0) { |element| (element - average(consumptions) )**2 }
    variance = sum / (consumptions.size - 1)
    Math.sqrt(variance)   
  end
end

Con todo, podríamos seguir aplicando el refactor inline variable si nos parece que tiene sentido.

  def standard_deviation(consumptions)
    Math.sqrt(variance(consumptions))
  end

  def variance(consumptions)
    sum = consumptions.sum(0.0) { |element| (element - average(consumptions))**2 }
    sum / (consumptions.size - 1)
  end

He dejado todos los métodos públicos, pues de este modo los podría poner bajo test de forma separada.

Por otra parte, el resultado de estos cambios con respecto al diseño es el aumento de la cohesión del código. Los métodos que hemos introducido agrupan líneas que colaboran en una tarea.

Ahora bien, si seguimos por aquí tenemos el peligro de caer en una espiral de refactoring que no nos lleve a ninguna parte. El diseño del software debe reflejar la comprensión del problema que resuelve. Si hacemos refactoring sin tener esto en cuenta, podemos perjudicar ese aspecto, moviendo el código hacia un diseño que no coincide con nuestro modelo mental.

Por eso, es preferible usar estar técnicas solo cuando necesitamos intervenir en el código para añadir o modificar funcionalidades del producto, o cuando necesitamos corregir errores.

En fin, pienso que como artículo introductorio ya está quedando un poco largo, así que voy a parar aquí y continuaremos hablando sobre la oportunidad del refactor en un nuevo artículo.

null is something… to care about

Fran Iglesias — Sat, 13 Nov 2021 16:52:06 +0000

null is something… to care about

In case you feel familiar with the title… You’re right. I’ve shamelessly stolen it from a talk by Sandi Metz:

I’ve found myself recently in a situation in which using null was a bad decision, so I decide to research ideas about how to avoid it and model with a better approach. This article by Arho Huttunen about avoiding *null *checks helped a lot and also provided me with some useful links, as well as this other article by Yegor Bugayenko.

The null reference was introduced by Tony Hoare in 1965 when he was developing the type system for ALGOL W. And he regretted it:

This has led to innumerable errors, vulnerabilities, and system crashes, which have probably caused a billion dollars of pain and damage in the last forty years. (Tony Hoare (2009): Null References: The Billion Dollar Mistake, QCon, 2009)

So, in this post we will try to discuss why null can be a really big problem, and how to avoid it.

What is null anyway?

It is a strange fate that we should suffer so much fear and doubt over so small a thing. (Boromir in The fellowship of the Ring, J.R.R.Tolkien)

What is null? null is a pointer that doesn’t refer to a valid object. This is similar to saying that null points to nothing. Or, in simpler words: null is nothing.

For example, if we consider a linked list, in which every item in the list keeps a pointer to the next one, null is used to mark the end of the list. If this item points to nothing, then the list ends.

Also, one typical use of null is to signal that a certain object has not been initialized and is waiting to have a value. We have to put something there, at some moment. The sooner, the better. For example, when we define the properties of a class, we usually initialize them at construction time. Type systems can prevent us from not initializing them by failing when we try to use an uninitialized property. The code above shows some examples.

Those were situations in which null is nothing. Also, they have some traits in common:

Usually, we don’t explicitly set them as null.
Those nulls are internal to the object or data structure. The outer world doesn’t need to know about them.

But, sometimes null is something. And that’s very important when communicating with other objects in the system.

Null as “no result”

One frequent use of null is to represent that something is not found. This can happen in several situations.

Let’s consider a repository of entities. There are two main ways to get an entity from such a repository:

by its identifier: we get one entity providing its identifier.
by specification: we can get a set of entities that match certain criteria.

To be or not to be

In a first way, what we can expect is that one entity with that identifier exists or not. If it exists is not a problem at all. We retrieve it and do whatever we need.

But if the entity doesn’t exist we get nothing. It makes sense. If the entity with that identifier was never stored, then you will never find it. There are two big possible reasons: something happened that prevented the entity from being stored in the first place or someone is trying to fool the system by using a non-existent identifier.

So, we get nothing and the temptation to represent it as null is strong.

But it is also wrong because it forces us to check if there is something in the response. It seems kind of ridiculous to verify if my partner has given me something I asked her for. If she cannot give it to me, she simply will say “I don't have it”. I don’t need to check my hand to be sure that I don’t have the damn thing.

The repository should fail with an exception because an entity that we hope does exist does not exist is something that shouldn't happen. The consumer only has to deal with that problem or pass it to a higher level until the exception finds a proper handler.

Sorry, we don’t find any of those

In the specification way, the problem is slightly different. In this case, we can expect that there are no entities that fulfill the specification, thus the result can be nothing.

If there are entities that meet the conditions of the specification, then you will get a set of entities. Maybe in the form of an array, a set, a collection, or a similar container data structure. Possibly you will get the set and iterate over it to provide some functionality.

Here, nothing means empty. And that’s because this time nothing is something: the result is a set or collection that happens to be empty. If you iterate an empty collection, nothing will happen, but also, nothing would break. You simply inform the consumer that no results were found.

So, you don’t usually need to check if the result set is empty.

Null as “maybe…”

Sometimes we use null to express that something is optional and it can be there or not. Consider this situation:

We have a student assessment system that allows us to assess their tasks. A grade is mandatory, but you can also add feedback with a customizable message and one standard label.

Ok. Feedback is optional, so we can have an Assessment class with a mandatory Grade and nullable Feedback property. It sounds reasonable, but it leads to several problems.

Most of the time you have to check if Feedback is present. If you forget that, you will find a situation in which you will be sending a message to nothing, and the system will complain, throwing a Null pointer exception or its equivalent.

For example, if you want to show a list view of the assessments for a student, you will have to add a conditional to check if the Feedback exists and render something in the affirmative case and render something different in the opposite one.

The problem here is that No Feedback is also a Feedback and null doesn’t represent that properly, because null is nothing, and not having feedback is something.

You can avoid that by introducing the Null Object pattern instead. A Null Object is an object with the same interface that does nothing or provides some kind of default behavior. The Null Object can even fail with exceptions if it makes sense for your domain at some point.

Let’s see an example. We define an interface for Feedback:

This is the standard feedback object:

And this is the Null Object implementation:

This way, you can communicate with the Null Object the same way you do with any standard objects of the same type, sending the same messages and expecting a proper response from it:

And not having to worry about checking for null.

Null as “I can’t do this in that way”

Related to the previous topic, sometimes you query an object for some data to perform an operation with the object itself.

The point here is that if the data we asked for is null, the consumer has to decide how to manage the situation. For example, gathering another data from the object and trying to call again the same method or another one.

This problem comes from a code smell known as an anemic object. An anemic object is mostly a container of data with little or no behavior at all. In OOP we expect that objects combine data and processes, in such a way that you don’t need to ask them for data. You tell them to do things, instead.

A typical example is sending notifications to users in our systems. They usually have several ways to be notified (email, SMS, etc), but it is possible that they don’t have defined a destination for one or several of them. So, you need to ask the User or Customer object about every possible communication channel and decide if you can use one or another.

This is our anemic Customer model:

And this is a service suffering the consequences:

One of the possible solutions is to apply the Tell, don’t ask principle. This way, you can tell the User object to perform the notification and it will do it using the preferred or configured channel.

This way, the problems of managing the nulls are hidden to other components. So, NotificationService could do something like this, using a Double Dispatch:

Another solution is to re-think the problem and the solution design. If the User object cannot manage the communication, because it is the responsibility of another object, then the User object can provide the preferred communication channel or a Null Object if none is available. This way, the notification service won’t need to worry about checking if they are valid or not.

Trust-driven programming

While I was writing this post I started to think about defensive programming techniques. In general, defensive programming considers all inputs as dangerous, so you shouldn’t trust them. This way, you need to validate all inputs to your system from the outside, but also to your functions, applying sanitization and validation rules to accept them.

Instead of that, object-oriented programming proposes something that we could call trust-driven programming. Objects in the system are created valid and complete, and we should expect them to provide valid data, in the form of valid objects and that they will properly manage their concerns. Objects cooperate for the benefit of others.

When an object answers with a null, it introduces mistrust into the system. Mistrust leads to fear, and fear leads to the dark side… And bad programming.

Don’t force checking for null

Most of the time, the problem with null is the need to check if some object response is null. That means that you cannot trust in that response, and the lack of trust is always a problem.

Having to check for null requires the insertion of a conditional clause that increases cyclomatic complexity and introduces risks. What will happen if you forget the check for null? Or worst: What can happen if you pass along the null to an indirection chain?

So, as a general rule, you should not return null. Instead, try to manage it inside the objects using one or several of these techniques:

Fail with an exception. This is the way to go when it is possible that the response doesn’t exist at all.
Return Null Objects. This allows the consumer to interact with an object that can have some kind of neutral or default behavior. Empty containers, such as collections, can be considered null objects in the sense that they represent empty sets of data.
Apply the “*Tell, don’t ask” *principle. Sometimes, having to deal with null is a design flaw. If you have to ask an object for some information to do something with the same object, there are a lot of chances that you can tell the object to do all the process itself, providing adequate behavior in the case that something is not there.
Re-think your design. Maybe “Tell, don’t ask” doesn’t apply, but you can consider other options or patterns to redesign the code.

'Learn TDD' book English version mostly finished

Fran Iglesias — Mon, 26 Jul 2021 13:56:51 +0000

For all of you interested in learning Test Driven Development, we announce that the book Learn TDD is mostly finished in its English version.

You can preview the full content for free in the LeanPub page for the book.

Entry-level TDD with the Leap Year Kata

Fran Iglesias — Mon, 21 Jun 2021 11:20:15 +0000

Maybe you have heard about the wonders of practicing TDD to develop your software. Or maybe you have read one of those rants about TDD being death. Never mind, if you want to learn how to inaugurate your TDD journey, you are at the right place. Let’s start from the very beginning.

What is TDD

TDD is a development technique based on the idea of writing tests before the production code. It was invented by KentBeck in the nineties as part of Extreme Programming. It basically works this way:

You decide the new piece of functionality you want to add and write a test that describes it
You write production code until the test pass, indicating that the functionality is working
You refactor the code for a better design
Repeat the steps until you have developed the full functionality

Key points to take into account are:

You only write one test on each iteration. That is: you don’t define the complete functionality up-front.
Each test asks you to write a part of the functionality that the production code currently lacks.
Every test should be as small as possible, and the changes in production code should be also as small as possible. This is what we call “baby steps”.

Other authors contributed to refining the technique. Namely, Robert C. Martin developed the Three Laws of TDD. They help us to decide how much test and code we should write. Here they are:

You must write a failing test before you write any production code.
You must not write more of a test than is sufficient to fail or fail to compile.
You must not write more production code than is sufficient to make the currently failing test pass.

We will return to these laws in the examples of this post.

The TDD cycle

One of the characteristics of TDD is the red-green-refactor cycle.

The cycle starts with a test that fails because no code makes it pass yet. Usually, testing frameworks represent failing tests in red color, so we say that we are in the red phase of the cycle.

Our goal is to make the test pass, something that is represented with the green color. So, we can say that our goal is to put the tests in green adding enough production code, but no more.

At this point, we should start with the most simple or obvious implementation that can work. Even if it seems too obvious or rough, we only want to make the test pass as soon as possible, establishing the desired behavior.

When we have all the tests passing in green, it is time to refactor the current implementation to a better design. This includes, among other actions:

Remove code duplication if possible
Improve naming of variables, functions, classes, constants
Improve code organization by extracting parts of it to private methods or new classes as responsibilities emerge
Improve code design as applicable conditions appear

What TDD is not

TDD uses tests as a development tool, but tests in TDD are not the same as Quality Assurance tests, although they overlap a lot.

The main differences between the TDD style of testing and QA are:

Some TDD tests are redundant for a QA suite. Usually, you start writing a lot of tests that are not needed for QA and should be removed after they fulfilled their purpose of driving development.
In general, the goal of TDD tests is to challenge the current production code to add new behavior. QA tests are meant to verify that behavior is the expected one.

Nevertheless, one of the outcomes of working using TDD is that we get a nice regression suite of unitary tests after clean it a bit.

Code quality and TDD

TDD is not a guarantee of code quality or good code design. It is a tool that helps to build software with better quality and design, preventing lots of defects and providing us with a methodology to add new functionality or improve design, without breaking anything.

This is possible because once we make a test pass, these very same tests become a regression test that ensures that the developed behavior stays untouched.

The refactor phase in the TDD cycle is the moment that we use to increase code quality, migrating from simple, naive, implementation to better-structured ones.

Benefits of TDD

TDD helps us to develop a discipline when writing software. Every test defines a short-test goal and helps us to keep focused on a single task. Using TDD you work relaxed, one step at a time. In fact, some studies show evidence that teams doing TDD spent less time debugging, have fewer bugs after deployment, and write a lot more tests.

Hands-on

Let’s start learning how to develop using Test Driven Development with a pretty simple exercise known as The Leap Year Kata. A Kata is a coding exercise that we should repeat frequently to gain automation of certain thought processes and steps. Kata is a term borrowed from a kind of exercise in martial arts.

The Leap Year Kata consists of developing a simple class or function to calculate if a given year is a leap year or not. The point of the exercise is to learn how the TDD cycle works, thus the simplicity of the problem.

For this post, I will develop a Year class, with a method isLeap that returns a bool value indicating if the year is a leap year or not. Let’s suppose a pairing session between an expert TDD practitioner and an entry-level developer.

– Ok, let’s do this. — said the expert. We are going to write this Year class guided by tests. First of all, we should take a look at the rules for calculating if a year is a leap year.

– Yes. All years that can be divided by 4, but not by 100, are Leap years.

– Good. So, years that can be divided by 100 are not leap ones. Except for years divisible by 400 that are leap years.

– Then, maybe we can start by writing a test to verify that a year is not a leap year. – Said the newbie developer.

– No, my friend. Those are a lot of steps at once. – Replied the expert.

– A lot? But, it’s only a single thing.

– Let’s see. What do you need to check that a year is or not a leap year?

– Well, I only need a class called Year, with a method isLeap. I can instantiate it with an example of a known common year and verify that the isLeap method returns false. Something like this:

– I can see that we need no less than three things: a class named year, a method in thas class named isLeap, and that the method can return false if the year is not a leap year.

– And are you suggesting writing a test for every single one of them?

– Exactly, my little padawan.

– How is that possible?

– By applying the TDD rules. What is the first one of them?

– Hum… We cannot write production code unless we have a failing test.

– So…

– So, we need to write a test.

– And… what does the second law say?

– It says that you cannot write more than one test enough to fail or not compile.

– That’s correct. And if we write that test you show me before, we will find that it can fail for a lot of reasons. The first thing we need is to be able to instantiate the class, so our first test should force us to define the class to the point that we can instantiate one object.

– So, that’s a baby step!

– Sure! Like this:

– But, but, but… you are not even passing an argument to instantiate the class.

– I know. But this test is enough to fail and to tell us exactly what we need to do next. See what happens if we execute it:

Error : Class ‘Year’ not found

– We should write the class, I guess.

– Yes, but how much code should we write.

– Well, we can create a new file named year.php, and define a class named Year, containing a meth…

– Hold! We only need to write enough code to make this test pass. We don’t even need to create a new file for that.

– But, that’s not a correct practice. You should separate classes in files.

– I know that, but we will have the refactoring phase for that. In the meantime, we need to make the test pass, and we can achieve that by doing the following:

– See? The test passes.

– All right, all right, teacher. But now, we should move the class to its own file.

– Of course, but now we are protected by a test. If we move the class to the wrong file, the test will not pass, indicating our mistake.

– Really?

– For sure! Let's move it and finish our first iteration.

– So, we have moved the class and the test is still passing. That’s great. We have completed a cycle. What should we do the next, little padawan developer?

– We could write a test that invokes the “isLeap” method, but we should instantiate the class before.

– You’re right. However, we can refactor our current test to have an instance of the class before writing the test. What do you think?

– I can’t see how useful that would be, but I trust you.

We prepare ourselves for the next iteration by doing this, leaving the code easy to change as needed, while the test is green. See the code:

– That’s not a huge change.

– No, it isn’t. But it is an enabler for our next step.

– Now, if we run the test, it fails because of a new reason:

Error : Call to undefined method App\Katas\LeapYear\Year::isLeap()

– Yes, little padawan. And the reason is that we don’t have the required method.

– I see. But… you are writing all in the same test. Is that right?

– At this point, it is fine. We could write each iteration in a different test, but we will remove these simple tests at the end of the process, so…

– Wait! Are we going to remove this test at the end? Are you kidding me?

– No, I’m not kidding. The point is that some of the tests that we write in TDD are useless or redundant out of this context, so we’ll remove them, leaving those that can act as regression tests.

– Oh!

– Let’s continue. We need to write our “isLeap” method.

– Yes, we can then pass the year as a parameter and check if we can divide it by four or not.

– My dear padawan: what is the minimum piece of code that will make the test pass?

– Hum… Let me think… Oh! I know: it will be enough with the method definition.

– That’s correct.

– So, we have the test passing again. Could we improve the code in some way?

– Is there code to improve?

– Maybe.

– Ufff. Let me think. Possibly we can declare the return type but it will force us to return a value, and we don’t know what to return at this moment.

– Any other improvement that can help us with the next steps?

– Errr… We could introduce the parameter needed in the constructor even if we don’t use it. This will break the test but… we can prevent that by changing the test first, and then modifying the production code to use it.

– Fantastic, my padawan! You learn fast. Now the production code, please.

– Yes. Here it is:

– Great! Now we are ready to start testing behavior. Maybe your first test makes sense now.

– Do you mean that we have been doing this workaround only to return to my first test? Seriously?

– Don’t get angry. Anger leads to the dark side. You should practice those baby steps until you automate and perform them in a few seconds. That will help you avoid many silly errors, like typos, putting the wrong file in the wrong folder, and using wrong names... Let’s see your test now and run it. What should happen?

– Easy. It will fail.

– Because…

– Because… we don’t have any code that checks that the value of the year can be divided by four, so we will need to add code that can do exactly that.

– Can you see my point? If we had executed your first test, we would have found that it failed because we have neither the class nor the method. Now, we are sure that the unique valid reason for the test to fail is not having code that performs the behavior that we want.

– So, let’s consider the test again:

– Fine. If we execute it, it fails this way:

Failed asserting that null is false.

– Yes. And it fails because of a good reason: the behavior is not implemented.

– Then we now can check if the year is divisible by…

– I have a better idea, for now, we can simply do the following:

– Now, I know you’re fooling me.

– Not at all. Run the test, please.

– It passes. But, this will not detect leap years.

– Yes. It’s pretty obvious. But that’s exactly the behavior that we want at this moment as defined by our tests. Now, our goal is to challenge the current implementation with a new test.

– Let me guess. We should write a test that verifies that if we instantiate the class with a genuine leap year it will be detected.

– You are right. And here is such a test:

– And it fails because of the right reason:

Failed asserting that false is true.

– True. We need to implement something that makes it pass.

– Yesssss. Finally!

– Great job, young padawan. Now the test passes and the code identifies most of the leap years. But maybe we can refactor it a bit, don’t you think?

– Yes, let’s do it. I see that we can eliminate the if and return the result of the boolean expression. Also, we can declare the return type as boolean.

– Nice. But we need to detect special no leap years, like 1900 or 1800.

– We need a test for that, teacher.

– And here it is:

– If we run it, it fails.

Failed asserting that true is false.

– I‘ve noted that you are running all of the tests. Why?

– Because I want to be sure that we don’t break a test that was passing before. This would mean that we have altered the behavior in some way. If this happens, we should stop and fix the code to make that test pass again.

– Ok, so every test that we make pass becomes a regression test.

– Exactly. That’s mandatory to be able to refactor. While refactoring, tests must be passing. All of them. That guarantees that we preserve the behavior.

– But right now, we have a test in red, so it’s time to implement something new, isn’t it?

– Yes. We should manage the situation in which a year is divisible by 100.

– That was pretty simple, but it does the job.

– It is fine enough. Maybe we can refactor something.

– Let me see... We have this idea of divisible by in the code, perhaps we can make it explicit in code extracting the calculation to a private method.

– Interesting. I like it. Let’s do it. But don’t forget to check that tests keep passing.

– Wow! We are near to complete the development. We need a new test so we can implement the management of the special leap years every 400 years.

– For example, the year 2000.

– That’s right. Let’s write the test:

– As expected, this test doesn’t pass.

– We are on the right track to implement production code that makes it green:

– So, my dear padawan, we are mostly done. Do you think we could refactor something here?

– I can’t see a good opportunity for that. The code is pretty simple and clean. Perhaps we could combine the two last conditions, because if a year number is divisible by four, but not by 100, then the year is a leap year and common if not.

– Yes. I agree. So, we can refactor the code to reflect that and finish here our first lesson.

– It’s nice, but I’m not sure if it is better. Anyway, having tests has allowed us to try this new approach being sure that the behavior is not broken. TDD allows for that and to decide if you can refactor to a better design, or experiment with different options.

– This approach has surprised me. It seems slow, but I liked the process a lot.

– You should practice TDD exercises, every day if possible. By doing so, you will gain experience, speed, and trust. Now, you should practice this kata several times, until you perform it fluidly and fast.

– I will do.

– Next time, I will propose you a slightly more complex exercise, and you’ll see that the process is exactly the same.

– Thank you. I’m looking forward to a new session.

Refactoring conditionals

Fran Iglesias — Mon, 31 May 2021 11:59:44 +0000

Refactoring conditionals

Conditionals are supposed to add intelligence to code, but they can be a nightmare to understand.

Why refactor conditionals

Our targets when refactoring conditionals are:

Making it easy to understand the conditions that we are checking: conditions can be hard to understand if we check for specific values that are meaningfully related to the concept they try to evaluate. See this example:

The condition is fairly easy to describe in human terms: select documents that have too long names. But its code expression is really hard to understand. Let’s simplify this a bit, extracting the conditional expression to a method with a meaningful name.

Reduce the cognitive complexity when following the execution flow through their branches. When you read a conditional structure you need to keep track of the main flow. If you introduce conditionals inside the branch of another conditional you are creating new breaking points that need to be tracked. At a certain level, you will overflow the ability of your working memory to handle all that tracking.

You can easily spot this by looking at the indentation levels of the code. The more indented it is, the more difficult it to understand and the easier it for bugs to appear.

You can manage this with several refactors. The most simple and easy to apply is to extract code in branches to methods. That opens new refactoring opportunities inside the private methods.

Conditions with meaning

You don’t always need conditions

We use conditionals to control the execution flow of a program: go this way if a condition is met. If not, go this other. But, sometimes, we can avoid that. Let’s look at this example:

We don’t need the flow control part here, we are served with the response from the object. Also, it’s pretty confusing that you return false in the true leg of the conditional.

To be precise, we can simply return the negation of that response:

This kind of refactor helps to avoid some potential points of failure. With the original code, you have to ensure no less than three things:

The method invoked provides the correct value
The conditional expression evaluates to the correct value
Each leg of the conditional returns the correct value

Now, you only have to check one thing:

The method invoked provides the correct value

Ternary operator instead of if/then

The ternary operator is pretty convenient under certain conditions. You should use it carefully because it can bring more problems than solutions. But it is perfect for this situation:

When we have two possible ways of calculating a value depending on a simple condition, the ternary operator is perfectly fine:

The conditions to use the ternary operator are:

We need to choose between two ways of calculating something
We don’t nest ternary operators.

The benefit here is that we state clearly that we are calculating something (the $event that we want to send in some next step).

If it’s complicated, extract a method

The ternary operator works pretty fine in the simple situations exposed before. But if the calculation is more complex or can’t be reduced to clean construction, you’d better extract a method and hide the conditional inside of it:

Replace type checking with type safety

Take a look at this code:

This code reveals a big bad smell. If you need to check for type to perform a task, then you have a design problem. Probably, you are trying to ensure that the Visit has a related Patient, so you can do something with it.

Does it make sense that a visit has no patient-related? If not, you probably should have to fail with an exception. In fact, you should ensure that when instantiating the Visit object. Anyway, the Visit::getVisitPatient method should be typed to state that the expected outcome is a VisitPatient. By doing that, you can enclose the code block in a try/catch to manage the situation.

Visit::getVisitPatient() guarantees that it only will return a VisitPatient. Otherwise, something is wrong.

If not having a VisitPatient is fine for Visit, then you should use nullable return type. This way, instead of checking for type, you will check for object existence, making code cleaner:

You can do this because getVisitPatient guarantees that it will return a VisitPatient or a null, keeping consumers safe about the type, but having to manage the situation of empty value. So you simply need to check that $visitPatient has a value.

Also, remember that this example could reveal a violation of the Tell, don’t ask principle or Demeter’s law. It is possible that code related to VisitPatient can be happily encapsulated in Visit. You can take a look at this previous article on the topic.

Give combined conditions a meaning

Sometimes combined conditions represent a concept that is not explicitly put in code. Let’s see a pretty simple example:

This line of code makes you think about what it is actually checking for. This line is in the context of an SMS notification service, so it makes sense to think about the idea of a patient can be notified via SMS or not. This can be achieved if both two conditions are met:

The patient has a phone number (that can receive SMS)
The patient chose to allow being notified with SMS

So, instead of asking about these two things to the same object, we can encapsulate both questions into only one, inside VisitPatient, because both conditions belong to the object:

If we don’t have a better place to move the combined conditions, we can simply create a private method. This is the proper way when we are using values from different origins.

Managing the flow

Conditionals are control flow structures and we use them to take different actions when certain requirements are met. We’ve been talking about the conditions, but now we will focus on their branches.

We can have different problems with the branches of conditional expressions:

Too large branches: too much code in the body of a branch makes it easy to lose track of the context.
Unbalanced branches: one of the branches is comparatively larger than the other, making the latter invisible.
Nested conditionals: conditions inside conditions are especially difficult to understand and are a perfect place for intricated bugs to hide.

Too large branches

The best approach is to extract branches to their own method with a descriptive name for the abstraction level. This will make it easier to understand the overall flow, and you always be able to dig into details if needed.

Look at this code. The branch is remarkably long:

We move all the code in the branch to a new method, taking care of managing how we return the data:

Now, the main flow is far easier to read. We can dive into details by jumping into the extracted method. We even have a chance to extract this code to a collaborator in the future if we need to reuse this particular piece.

Unbalanced branches

When one of the legs of a conditional structure is a oneliner and the other is huge, it is good practice to invert the conditional, so the shortest branch appears in the first place. This avoids missing it:

It’s much better to extract both branches to methods with a significant name. This way you stress the fact that two flows are depending on the province variable having value or not.

You should consider eliminating the else branch. Sometimes you won’t need it. For example, because you can perform an early return.

Nested conditionals

Nested conditionals are a source of frustration and pain. They are difficult to follow and unsafe to modify when needed. How can we avoid them?

First of all, try to ensure that nesting is needed. In the following example, we can see that the inner conditional should be moved to another place. We can guess that by putting next, things that are related to each other. For example, the calculation of validSince.

The value of validSince can be calculated before, instead of waiting until being in the conditional. And we can introduce the null coalesce operator to simplify the assignment using the first value among all possible that it’s not null.

Our first approach will be to extract the main branches into private methods. Then, you repeat this extraction iteratively so all methods have a maximum of one indentation level in each one. This opens opportunities for further refactors that are pretty difficult, or impossible, in the nested structure. In fact, part of nested structure complexities relies on the fact that you have to carefully manage the execution flow to make sure you don’t overwrite variables or introduce some other bugs.

Let’s take a look at this code extracted from the famous GildedRose kata:

This is only half of the code and has a lot of nesting.

For this example, we extract the legs to their methods. Note that this kata deserves an object-oriented approach, but I want to illustrate a different point this time:

We can see that in the extracted methods the body is inside a conditional. We can apply a return early refactor that will reduce the nesting. We need to invert the conditional expressions first. As you can see, the readability has increased a lot.

We can apply return early again in the increaseQuality method:

At this point, we can spot some remaining code duplication that we could refactor. It can be argued that the nesting is there yet. Nevertheless, the takeaway here is that with pretty simple steps we have structured the code so it is much more readable and easier to understand.

Final words

Conditionals can be tricky to manage, but we can improve their readability a lot by using simple refactoring techniques and thinking twice about why we need them.

Of course, we could go further by applying a good object-oriented design that can even avoid the need for conditional structures. But that should be the topic for another post. In the meantime, you can watch this talk by Sandi Metz about how and why to achieve this using the Gilded Rose example.

Keep an eye on our blog for more tips about refactoring and testing to keep your code healthy.

Do you enjoy the article? Follow our profile and visit our other channels: Docplanner.tech website, Facebook, Twitter, and LinkedIn.

Test doubles

Fran Iglesias — Mon, 31 May 2021 11:59:15 +0000

Test doubles

When you need to test an object using collaborators and want to be sure that the tested behavior is provided only by the code in the object, you probably will need test doubles.

Using test doubles, you can suppress or keep under control the behavior of the collaborators. This way you isolate the object from any other influence. Also, by using test doubles, you can avoid costly dependencies that can be out of your control or that introduce indetermination or performance penalties.

But, let’s start from the beginning.

How we test a piece of software

We test a piece of software by comparing the result of executing it against some defined criteria. We usually name this piece of software the subject under test or SUT.

These pieces of software can be one of these two types, but not both:

A query retrieves some information about the system's state but doesn’t produce any side effects.

A command produces a change in the state of the system but doesn’t return a response.

Yes: This is the Command Query Separation principle by Bertrand Meyer.

Testing queries

Queries are pretty easy to test because we only need to get the response and compare it with the expected outcome.

When testing queries we could use test doubles to replace expensive dependencies and to define their behaviors for the different scenarios. Usually, we will be using stubs or fakes for that.

Testing commands

Commands, instead, are a bit harder for testing. We need to verify that we get the expected outcome by looking for their effect on the system. Nevertheless, sometimes we don’t have the possibility to do that because we can’t use the real dependency and we use a test double.

We will need doubles to define the collaborators' behavior and verify that we produce the desired effect, expecting that we send the proper messages to them. These kinds of doubles are called spies or mocks.

An example is when we need to test a service that sends an email. There is no way to check that we send a real email. Even if we could do it, looking in some specific mailbox, the performance and reliability of the test would be a complete disaster. So, instead of that, we ensure that we call the appropriate methods of the mailer library with the correct message. We can see an example here:

The side-effect of the command is tested in the last two lines, where we interrogate the MailerSpy about the calls performed and the receiver of the message

Introducing test doubles

We will be using a test example to introduce the different types of test doubles. Imagine that we are building a feature to greet our customers for their birthday, sending them an email, maybe with a promo code or another goodie. Here is the setUp:

To unit test the use case GreetCustomerForBirthday, we will need to double all of its collaborators.

Dummies

Dummies are test doubles that have no behavior, all their methods return null or nothing. We use them because we need to comply with some interface and we are not particularly interested in what they do.

Let’s start with the Logger. We may want a logger in the use case but we are not worried about how it is used, but we need it to instantiate the GreetCustomerForBirthdayHandler. This is the case for a dummy.

Stubs

Stubs are test doubles that should have a predefined behavior that we want to control. Imagine that we need to test a service that gets some information talking to an external API. We have an adapter to talk with this API so we will need to double it instead of calling the real API.

In one of the possible scenarios, we can simulate that the API returns a correct value. In another, we can simulate that the API is down, so we test that our service can manage that situation gracefully. In every possible scenario, we define a stubbed behavior so we can verify our piece of software.

The use case in our example needs to get the current date to find the customers celebrating a birthday. Working with dates is always a tricky question, so, instead of access to the real system clock, we abstract it in the form of a ClockService. This way. we simply need to stub a fixed date and store a crafted Customer in the test repository.

Fakes

Sometimes we need a collaborator that has the same behavior as the real dependency but in a cheaper way. For example, instead of having a database-backed repository, we could use memory storage to provide the same functionality without the performance penalty. This kind of double is called a Fake.

Fakes should pass the same tests that the original collaborator, they are alternative implementations.

We decided to use a FakeCustomerRepository for this test using a memory collection implementation.

Spies

When we are testing commands we need to verify that they produce the expected outcome. If we need to double the dependency we won’t be able to check that outcome. Alternatively, we will need to verify that we send the right message to the collaborator in the object-oriented programming sense.

Spies are test doubles that can register how they are used so we can interrogate them after executing the subject under test and make assertions about that.

The side effect of this use case is to send an email. We need to create a Spy that can verify that we call the send method of the MailerService with the appropriate data.

Mocks

Mocks resolve the same problem as spies: we use them to verify that the correct message is sent to the collaborator. The difference is that a mock expects to be used in a certain way so you could say that it carries the assertion with it.

The problem with spies and mocks is that they introduce a certain degree of fragility given that they couple the test with the implementation of the code under test.

Also, tests using mocks are more difficult to understand because they hide the assertion into the expectation. It is preferable to use spies, instead.

But, do we really need to use doubles?

There is some controversy about using or not doubles in tests. Most of the concerns are with spies and mocks because of the coupling they introduce. Other objections have to do with the fact that we could consider the behavior of an object as a composition of the behaviors of its collaborators, and test that through the public API of the subject under test. Those are valid points, but using doubles is a trade-off we need to accept in several circumstances.

Doubles as boundaries

Functional tests are those that verify the behavior of the subject under test. We can have different types of functional tests according to the scope of the test:

Unitary test: check a unit of software in isolation. We should double any collaborator of the unit to ensure that the outcome is produced only by the code of the unit. Nevertheless, you can use real collaborators instead of doubling them if they have no external dependencies.

Integration tests: check subsystems to verify that their components communicate properly. We need to double the dependencies used by the subsystems that don’t belong to it.

End-to-end tests: check the behavior of a system from the outside, using its entry and output points. We should double things that we don’t own, that are expensive in terms of performance, or that have undefined behavior, such as external services, mostly via fakes or crafted versions.

As you can see, doubles help to define the boundaries of the test scope. You use a double whenever you need to set a boundary that you don’t want to cross in testing.

Isolation

Those boundaries seldom match architectural boundaries that put your tests at risk if you cross them.

You don’t want to include in your test the performance penalty of talking to a real database at the unitary level, so you will double the database adapter to achieve this.

You don’t want the risk of having pre-existing data that will make unpredictable the behavior of the subject under test and the overload of having to manage to keep the database state clean.

You don’t want to cross your fingers trusting some external API to be up and running or returning the expected responses for your tests. In consequence, you will need to fake it in some way.

You don’t want to be dependent on the machine or the concrete set-up on which the test runs, so you use doubles to isolate from those. A typical situation is when a test has to manage dates or times, you should stub the behavior of the system clock to avoid tests that fail or pass depending on the machine, the timezone, or even the date or time when you run them.

So, doubles are one of the tools that we count on to isolate tests from all the details that we don’t own or cannot control.

Outside-in test-driven development

The so-called London school of TDD uses doubles as first-class citizens in the outside-in approach to test-driven development.

In the inner unitary test loops, mocks are used extensively as design tools, doubling the collaborators used by the component under development to define and refine their interfaces and the communication between units. The drawbacks of this approach, such as the test-implementation coupling, are compensated by the fact that we are designing the implementation and it probably won’t change.

Best practices with doubles

You don’t share doubles behavior between tests, except fakes.

Shared doubles can introduce dependencies between tests, something that leads to low reliability and low trustfulness. The needs of one test are not the same as another, and they should be able to evolve separately.

So, you can share doubles when:

They are fakes in the sense of alternative, low-cost fully-owned implementations of a given dependency.
They are dummies, so they have no behavior, and, therefore, they don’t introduce cross dependency.
They are configurable stubs, so you can control the behavior from the test.

You only stub the behavior that is interesting for the current test.

In addition, you don’t want to have one-size-fits-all doubles. You only stub the behavior you need for the test at hand, forgetting about other possible scenarios. Remember that you want to have isolated tests, even inside the same test case.

You set the minimum expectations needed to verify a side effect in mocks.

Expectations are useful to assert side effects in mocks, but you don’t use them to verify calls to stubs.

I mean: in a query, you could need to stub some behavior in a collaborator, the test will execute the call to that collaborator and get the stubbed response. The test will pass if the code under test manages the collaborator response the right way returning the correct response. In this situation, you don’t need to assert that the message was issued to the collaborator: it is implied in the successful test execution.

When testing a command, you will need to verify the side-effect of calling a specific collaborator, so an expectation should be set to check that this really happens, and only for that.

Ideally, you will have only one expectation by test except when you need some triangulation.

How to create doubles

There are several techniques to create doubles:

Using real objects

This technique is specially used when creating fakes, but you can use it for other types of doubles you would like to share between tests, like dummies.

It’s pretty straightforward: create and use a new implementation for the interface you need to double. The following example is a pretty simple example of a CustomerRepository implementation only for tests:

To use it, we only need to do this:

Let’s see an example of a spy:

The main problem with this type of implementation is that it is not Liskov Substitution Principle compliant given we add some query methods to be able to verify the expected outcomes. This is a trade-off that we can accept because their use is limited to the test environment.

Using anonymous classes

Anonymous classes are a great way to create disposable doubles for a specific test without using a mocking framework and keeping the advantages of real objects. Here, you have an example:

The previous example violates Liskov Substitution Principle, so you can be stricter:

As you can see, we don’t even need to extract the class to another file, but you could use a factory if makes sense for your use case.

Using mocking frameworks

Mocking frameworks can be an easy way to build doubles that are expensive to double using the other techniques. For example, in PHP, the PSR Logger Interface is huge, with no less than nine methods to implement. It is way simpler to use the mocking framework like this and get a dummy double:

Every mocking framework has its own syntax. I will be using the native PHPUnit one in these examples, but you will get the point.

To build a stub, you only have to define a response to a method call, like this:

This is an example of a MailerService mock that expects the method send to be called with a message.

In this snippet we have defined a mock of the MailerService interface, setting the expectation that we will be calling its send method once with a Message object.

We can create a spy, instead of a pure mock:

In this example, you inject the $mailerMock as a dependency, but you check the outcomes asking to the $mailerSpy. It is pretty weird, but other mocking frameworks offer better interfaces for this feature.

In fact, you probably will be better served by crafting your own doubles instead of using a mocking library.

Summing up…

Test doubles are a tool that we will need to use sooner or later. Their purpose is to help us isolate the code under test replacing their dependencies.

We can use several techniques to create them ranging from implementing our own doubles to use a mocking library.

Refactor for better knowledge allocation

Fran Iglesias — Mon, 31 May 2021 11:56:02 +0000

Refactor for better knowledge allocation

One of the ongoing problems inside all organizations is documentation. From an agile perspective, code can document a good part of the knowledge about the business when written in an expressive, well organized, fashion.

But this frequently does not happen due to several reasons. Poor communication, too much emphasis in implementation details, framework driven development, among others. So, many code bases present problems when you try to understand how they express the domain of the business.

In our previous post we talked about some tips to refactor code to be a better storyteller. This time we will continue digging into the same idea, but from a slightly different point of view: how to better structure the knowledge.

Knowledge means a lot of things. In every software project there are both business knowledge (how to book a visit with a doctor, for example), but also technical knowledge (how to connect to some database server). The problems arise when different domains of knowledge appear mixed in code. It can be technical concerns entangled with business concerns, different areas of the business sharing pieces of code or huge levels of coupling between different parts of the code.

Object-Oriented Software… or not so much

There are lots of code bases in the wild written in object oriented languages, using classes and objects, that doesn’t really apply well the OO paradigm. They consist of procedural code with an object-oriented costume. In this code bases, knowledge is something global and objects are usually only containers for data, behavior or both, but they are not really self-consistent, well encapsulated objects.

In the OO paradigm, objects are experts that encapsulate knowledge and behavior, communicating and cooperating to fulfill the tasks.

The most difficult problem in teaching object- oriented programming is getting the learner to give up the global knowledge of control that is possible with procedural programs, and rely on the local knowledge of objects to accomplish their tasks. Novice designs are littered with regressions to global thinking: gratuitous global variables, unnecessary pointers, and inappropriate reliance on the implementation of other objects. (Beck & Cunningham: A Laboratory For Teaching Object-oriented Thinking)

Object-oriented design is guided by a plethora of principles that serve as guidelines to make decisions about how to allocate knowledge and how objects should interact. We can benefit from applying this principles to improve the quality of our code, moving from a procedural style to a more object-oriented one.

So, in this post we will review some of these principles and we will try to show them in action.

Separation of concerns principle

This is pretty simple, and the basement for most architectural and code organization decisions: different parts of the code addresses different concerns.

We can understand this principle from another perspective: a unit of code should not address more than one concern. If so, it needs to be broken into parts. We can apply this to different levels: the methods in a class, the different class in a software module, the different modules in a software application, or the different layers.

This basic principle, enunciated by Dijsktra, is in the roots of the well know Single Responsibility Principle, the S in SOLID, and most of the patterns and principles that help us to put knowledge where it belongs.

Single Responsibility Principle states that software units should have one unique responsibility, defined as a unique reason to change. It doesn’t mean to do only one thing, that could lead us to a convoluted design. Instead of that, this principle is better applied if we consider the reasons that could force us to change that unit. Let’s see an example.

Price is a common concept for most business. It could be modelled initially with this class:

Pretty anemic, huh? But it is a starting point. Now, the Finance team asks for a way to add VAT to the price. We honor immutability and add a factory method that returns the price with added VAT. Well done!

Also, the Front-End crew asks for a price representation that includes the currency information. We add a simple format method to the class.

Hey! Different currencies requires different representation formats. Also, our beloved Marketing team asks for a feature to add discounts and promotions to our Price objects. Wait a moment! This poor little class has a lot of reasons to change. We are asking it to hold a lot of responsibilities.

We need a change to address that.

First, Price should be only responsible for holding information about… price. We can model Taxes and Discounts as Price decorators, so we always will be able to recover the base price. Here is an example. It’s not perfect, but now we have separated some of the responsibilities.

The method addVat in Price will become unnecessary but if it is being used we can remove it iteratively.

So, do you need a discount? No problem:

Now, you can use it like in the following test. Price is now a compounded object:

Now, if the Finance team asks us to apply another Tax to our price, or one different because we need prices for several countries with different laws, we only need to add classes accordingly without touching Price itself.

Also, if Marketing asks us to apply new discounts and promotions, we only need to add classes for each one.

All of those new classes may change for one unique reason, so we are honoring the Single Responsibility Principle… at least in this area. Remember that we need to address the format concern. But we can apply a similar solution.

Oh, and another benefit is that by doing this approach, we also are honoring the Open for extension, closed for modification principle. This principle states that we should avoid to modify existing code in order to add or modify behavior. Instead, we should provide means to extend object behavior without touching that existing code.

Let’s return to the format problem. It exposes an interesting problem that requires us to talk about segregating interfaces.

Interface Segregation Principle

If you look at our Decorators you can see that they have to carry the format method, and they really don’t need it. Price and their Decorators are having two reasons for change: one related to their business meaning, and one related to their presentation. Also, you have to duplicate it in every type you need to create, multiplying the problems with the format variations.

When working with legacy objects, it is very easy to find classes that hold too many responsibilities and had huge public interfaces with lots of methods serving different kinds of consumers.

The Interface Segregation Principle, states that an object should not depend on methods that it will not use. We should design narrower interfaces based on the needs of its consumers.

In our example, PriceInterface has methods that obey to different kinds of consumers: one is interested in the amount concerns, and one is interested in the presentation concerns. So, we should separate those concerns in different interfaces.

Now, the old Price class implements two interfaces, but our decorators only implement one of them: the one that relates with amount modifications. They are free about presentation concerns.

We can solve these concerns with another family of decorators using the PriceFormatterInterface:

Now, you can compose them:

It looks nice. And it is because we now have separated responsibilities in different classes, with pretty narrow interfaces.

An extra benefit is that we removed some duplication. Let’s talk about being DRY.

The DRY principle

The Don’t Repeat Yourself principle, by Hunt and Thomas, states that “Every piece of knowledge must have a single, unambiguous, authoritative representation within a system”.

The principle doesn’t refer to code duplication, it talks about knowledge duplication. This is an important distinction, because trying to apply DRY to any code duplication can lead us to tough problems. Sandi Metz has a post that worth the reading: The wrong abstraction.

Anyway, code duplication can be a symptom of knowledge duplication, it could mean that there is an interesting concept emerging. But you should consider this carefully. Code duplication can:

Be unavoidable: three or more fragments of code look similar, but they don’t represent the same knowledge.
Be unnecessary: three or more fragments of code can be reconciled extracting the parts that are different for economy. For example, blocks of code that can be extracted to one method or function.
Represent knowledge duplication: three or more fragments of code are examples of a more general concept or abstraction.

Have you notice the three? There is an heuristic called The Rule of Three that advises to not refactor duplication unless you have three or more occurrences of the same code. It is a simple rule to prevent premature abstraction if we don’t have enough information to decide what kind of duplication we are spotting.

But, is there a similar rule to find knowledge duplication that we should refactor? Yes. If you need to make the very same change in several parts of the code at once, in order to introduce or modify a behavior, then you have a candidate.

I will try to show you some examples of when to apply DRY and when don’t with our Price Decorators family.

Does it make sense that both decorators share a common ancestor in order to avoid duplications? No, it doesn’t. Taxes and Discounts are two different concepts, two different pieces of knowledge with their own business rules and they are also managed by different teams in the company. Instead, we could model Taxes and Discounts as different abstractions.

The tell, don’t ask principle

The basic idea is that if you are getting information from an object to operate with it, you should encapsulate the whole operation in that object because it is where it belongs. Instead of asking the object about the data needed for the calculation, it is better to tell the object to calculate that itself.

In a more formal definition: if you need to ask an object about its state in order to change the state itself, you are violating both the information hiding and encapsulation principles. Then, you should encapsulate the operation in a method.

This principle is very useful to start moving knowledge to the objects to which belongs. Objects should be information experts about themselves. Also, objects are the solely responsibles about its state and consistency. You should be able to trust your objects in that concern. This will make your life easier.

Let’s see an example. Imagine you have a concept TimeSlot to allocate, ahem…, time slots in a Calendar application. If you want to know if two slots overlap you could do this beauty:

You are asking time slots about data to perform a calculation in order to guess something about the state of both slots, among other code quality violations. But this is a knowledge that belongs to the TimeSlot object itself: to know if it overlaps with another TimeSlot:

So you can tell a TimeSlot to calculate if another one is overlapping or not, because it has all the knowledge about itself.

Anemic object smell

This principle is related with the anemic object smell. An anemic object, or more specifically an anemic model, is an object that has only state, but no behavior. There are several objects that are designed to only contain data, for example Data Transfer Objects, but we are referring here to objects that model entities or values that should also have domain behavior.

In our example, the first implementation of TimeSlot was an anemic object. It only had properties and accessors to those properties.

As we have shown above, TimeSlot can, and must, encapsulate its own knowledge in the form of properties and behavior. Rich objects also can attract more behavior needed by the application. Imagine that you need a way to get a new TimeSlot just exactly after a given one. Here is the dirty way:

But, TimeSlot could be able to create them for us:

Because it knows how:

This way, you know that if you need to learn about time slots, your first stop should be the TimeSlot class.

The minimum knowledge principle or Demeter’s law

This principle states that a unit of software, usually a function or method, should not talk with objects that they don’t know directly. What objects are those?

Objects instantiated inside the unit.
Object passed to the unit as parameters.
The object that owns the unit (its methods and attributes)

The application of this principle helps a lot to avoid coupling. Coupling is the degree of dependency between software units. Some level of coupling is unavoidable but you can keep it under control. The secret for a healthy coupling is that objects know the minimum about other objects.

An object should not rely in having intimate knowledge about the internal structure of another. For example, an object should not perform calls on an object inside another one. This is a smell called inappropriate intimacy.

You should talk with an object using only its public interface. If you really need to access to some property or internal, you should consider to add a method that exposes it. But also, you should ask yourself if that knowledge should be available in another object.

Consider this piece of code. We have a PricingCalculator that allows us to calculate product pricing applying different rules. This is a new way to model some of the behaviors of our previous examples.

As you can see, base price of Product is defined in its product Family. Let’s see what happen inside the calculator:

This works, but PricingCalculator has to know that, in order to get the product price, it has to ask first for the Family and then for the price. Now, that’s tight coupling: PricingCalculator must know intimate details of the internal structure of Product. Nevertheless, It shouldn’t need to know how the price is built or where it come from. It only should know that Product has a price.

In the future, if you change Product to have its price defined in a different way, the program could break. For example, imagine that Family is no longer responsible of the Product price:

If we don’t change PricingCalculator, the program will fail, because it expects to find the price in the Family object. It depends on talking with an object of which it doesn’t know about.

In this case, Product should be the source of truth about its price:

Now, PricingCalculator only depends on Price, a known object because it is passed as a parameter in the method forProduct.

In this case, applying Demeter’s Law helps us to decouple and to make our code more resilient to changes, allowing objects to change its internals without affecting others unnecessarily. So, every time you find those chained calls for access objects contained in another object, take a time to encapsulate that logic or event consider if that knowledge belongs to another place.

More refactoring tips coming

We expect that with this post you have some food for thought about how to allocate knowledge and behavior in your code.

Object-oriented design principles are a very good guide to help us to move and organize concepts in our codebase, allowing for a better modelling of the domain knowledge. We didn’t address all of them, of course. The lesson here is to take them into consideration when refactoring.

There are a lot of things about refactoring, so stay tuned to learn about managing complex and nested conditionals and more ways to improve the health of your code.

No hagas estos tests

Fran Iglesias — Thu, 08 Apr 2021 00:00:00 +0000

O al menos, no de esta manera.

Objetos mensaje

Eventos, Comandos y Queries, son objetos que nos interesan por su significado y que llevan datos para sus respectivos handlers. En ocasiones me han planteado: ¿no hay que testear esto?

Quiero decir:

class PostWasCreated
{
    private string $title;
    private string $body;

    public function __construct(string $title, string $body)
    {
        $this->title = $title;
        $this->body = $body;
    }

    public function title(): string
    {
        return $this->title;
    }

    public function body(): string
    {
        return $this->body;
    }
}

– ¿No hay que testear que se construye bien?

class PostWasCreatedTest extends TestCase
{
    public function testShouldCreateEvent(): void
    {
        $event = new PostWasCreated('Title', 'Body');

        self::assertTrue('Title', $event->title());
        self::assertTrue('Body', $event->body());
    }
}

– Pues no. Tanto si es en una situación de TDD, como si en QA, este test es una pérdida de tiempo.

– Pero imagínate que te lías y asignas el valor de body a title y viceversa.

– Es que no lo tienes que testear así.

En primer lugar, buscamos testear comportamientos. Asignar los parámetros pasados a las propiedades no es un comportamiento en sí mismo.

– ¿Pero no testeas que se crean objetos consistentes? Si hasta tienes una kata basada en eso en este mismo blog.

– Por supuesto, pero lo que verificamos es que construimos objetos que cumplen reglas de negocio y mantienen invariantes. De nuevo: la inicialización de propiedades no es un comportamiento.

– ¿Entonces?

– La forma adecuada de testear esto es testeando su handler.

Imaginemos algún servicio de reservas que notifica a la usuaria por correo electrónico que la reserva ha sido realizada correctamente. Esto se hace porque al realizar la reserva, se lanza un evento BookingWasCreated, que contiene los datos básicos de la misma.

Uno de los suscriptores o listeners de este evento es SendConfirmationMessage que compone un mensaje y lo envía a través de un Mailer, aquí representado con un doble.

El razonamiento es el siguiente: si el evento está bien construido, el mensaje se construirá como se espera. Si el test falla porque el mensaje resultante es, por ejemplo, “Booked from 15/5/2021 to 15:35 @ 16:20”, está claro que en alguna parte se han cruzado los datos.

class SendConfirmationMessageTest extends TestCase
{
    public function testShouldNotify(): void
    {
        $event = new BookingWasCreated(
            'fran@example.com', 
            '15/5/2021', 
            '15:35', 
            '16:20'
        );

        $message = new Message('Booked from 15:35 to 16:20 @ 15/5/2021');
        $message->to('fran@example.com');

        $mailer = $this->createMock(Mailer::class);
        $mailer
            ->expect(self::once())
            ->method('send')
            ->with($message);

        $sendConfirmationMessage = new SendConfirmationMessage($mailer);

        $sendConfirmationMessage->handle($event);  
    }
}

Todo esto aplica igualmente a DTO y otros objetos parámetros que solo llevan datos y no tienen comportamientos.

Refactor for better comprehension

Fran Iglesias — Wed, 03 Mar 2021 16:03:17 +0000

You are in front of a long-lived codebase and you understand… Well, not so much.

Reading and understanding code can be hard

You want the code to tell you its story in order to add your own part to it. Nevertheless, you may find a code base, or some piece of it, that is a little pandemonium, where you cannot obtain too much information easily. You need to interpret how some concepts are represented and how some processes are reflected. Maybe, you try to read the documentation, but it can be outdated, redundant, useless, or non-existent.

You need to refactor for better comprehension before your start implementing a new feature or fixing that bug.

Refactor for comprehension is the process we perform to evolve present code structure to another one that is more understandable. One that reflects better the state of business knowledge and one that is easier to operate with.

In fact, we should consider code as an executable representation of the business or domain knowledge. Tests, on the other hand, are another representation of the same knowledge, built around the outcomes of the production code.

When we need to work with a messy piece of code, we surely will need to introduce some changes that should improve our ability to understand what’s going on. We have three areas in which to intervene:

Production code itself
Test covering that production code
Comments and documentation

Before we start talking about how to make code easier to understand, we should first learn about the “two hats metaphor”.

Two hats to rule them all

The two hats metaphor by Kent Beck helps us to understand something very important about refactoring.

The basic idea is that you must not mix refactoring with changing the behavior of a piece of code.

So, you work on preparatory refactoring crafting structural changes wearing the refactor hat. Then, you commit that set of modifications as a whole. Once committed, you work in the behavioral change and commit it wearing the change behavior hat. After that, you may need to put on the refactor hat again to tidy up things.

Beck says that the preparatory refactoring consists on reorganizing code to make the change of behavior easy and safe to apply:

// Detect dark theme var iframe = document.getElementById('tweet-250733358307500032-863'); if (document.body.className.includes('dark-theme')) { iframe.src = "https://platform.twitter.com/embed/Tweet.html?id=250733358307500032&theme=dark" }

You can’t wear two hats at the same time… Well, you shouldn’t. So, we ask you to put the refactor hat right now because we are going to introduce some nice ideas about how to improve the storytelling abilities of the code.

Refactor for better names

The first tool to make code explain itself is to choose names wisely.

Naming things is also a teamwork

Naming is reputed as one of the most difficult things in computer science, along with invalidating the cache, but you don’t have the obligation to spot on the first try. You can rename things for very good reasons, among others:

A concept that is represented in code with a name that doesn’t describe it accurately from the start.
A concept that could have evolved and its name became obsolete.
A name that could be too general or too narrow for the concept that it describes.

We can apply a rename refactor in these situations. Let’s see some examples.

Name coupled to implementation

Consider this variable name:

Leaving aside other questions about the code, what is the problem with this name? As we can see, the name mentions the current specific time limits decided by the business.

So, if those limits change someday, we could end with something like the first line of the following example, so we should change the variable name in order to keep it consistent with business requirements.

We will need to do the same every time those limits are changed. Nevertheless, it’s easy to forgot updating the name at some point, and that will cause some moments of anxiety to the future developer.

We can see that a more abstract concept is arising: the idea that there is a period of time (overnight) that requires a special treatment and that is defined by certain time limits. Probably, when talking about this feature with the business people or users, someone asked something similar to “What if the appointment is cancelled overnight?“. As programmers we need to define that with some kind of temporal marks, but the concept itself isn’t tied to a precise hour range.

We could express it this way:

Now, we have a more expressive name that doesn’t depend on implementation details. The concept overnight is an abstraction easier to understand than an arbitrary time interval, so its name doesn’t need to change frequently.

This very same idea can be applied to function and method names.

How to rename things

You can use the tools provided by your IDE, taking into account some safety measures.

You will have no problems when:

changing a variable name inside a method or function.
changing a private method name inside a class.

Most IDEs offer the rename refactor. At the end, it’s a find and replace, but IDEs can work at the syntax tree level, so it is usually more precise than us to complete this refactor.

The refactor menu in PHPStorm

For example, in the JetBrains IDEs you select the variable or function you want to rename, select Refactor, then Rename (caps-F6), type the new name, and you’re done.

If you change a public function name, you may find that there are a lot of places affected, so we will use a more conservative approach.

Let’s see an example: Imagine that you have this method in a class that you want to rename because its current naming doesn’t help too much. Sure, it’s funny, but a bit misleading. Nevertheless, the method is called in a lot of places, so a bulk rename involves risks:

First of all, duplicate the function and rename the copy. All calls to the original name are preserved and now you have a method with the better name that has the very same behavior.

Now, delete the body of the old function and write a call to the new one instead. This way, you avoid the code duplication without hurting existing uses of the old name.

Progressively, change the calls to the old method to the new one every time you find them and has something to do with your current task.

When there are no calls to the old method remaining, delete it.

Alternatively, you can use *extract method *refactor provided by the IDE selecting the body of the method you want to rename and extracting it with the new name.

Where the things had no name

There is a well-known code smell called magic numbers. It refers to primitive values that are present in the code but you don’t have any clue about what they mean.

In the same line we have used in the previous example about the overnight period. the limits of this period are represented by numbers.

Those numbers were decided by business people. From the code point of view they are arbitrary values, and with the passing of time, their meaning can be forgotten. So, it is a good idea to give them names.

The simplest way to do that is to convert these values into constants with a name:

There are other potential improvements for this line, but we will leave them for future articles about refactoring for better design.

This refactor can be applied to any arbitrary value you could find in a code fragment and needs explanation. But there are some more advantages:

If those values change you don’t need to touch the code that uses them, lowering the chances to break something by accident.
If they are used in several places, you will have only one point to change them, guaranteeing consistency.

Break expressions into meaningful parts

Complex expressions should be broken into parts in order to improve readability, but also to avoid potential errors. Tangled expressions with a lot of elements are a good place for bugs to hide, specially when we need to modify them.

We can use the *extract variabl*e refactor, that consists of replacing part of a expression with a variable. A good rule of thumb is to apply this refactor to parts wrapped in brackets. Let’s see a typical example:

Ok. This looks like a very simple expression, but it should help us to understand the intent of the refactor and how to proceed.

The *$this->amount * vat **subexpression represents the concept of *tax or VAT amount. That concept could be expressed in a variable:

Now, the expression is easier to read. Imagine the same applied to more complex calculations.

Sometimes, these kind of extractions reveal the need for a behavior, even for a public one:

Extract variable helps us abstracting concepts inside the scope of a method or function. Extract method, instead, is useful when we identify public or reusable behaviors.

Explain conditional expressions

Every conditional expression that is a combination of two or more single conditions is a good candidate to be encapsulated in a method with a name that provides a meaning.

Imagine that we have a pricing schema that allow us to offer different prices to different age ranges.

The conditions in this method apply the price for each age range. Let’s take a look at line 12. This condition expresses an age interval for adults. We could extract this to a method that explicitly states that idea:

Even single conditions could need to be extracted if they are not expressive enough or easy to understand. We can do the same for line 17:

This is especially true when the condition is negated, because this kind of expression is more difficult to process when reading.

Not to mention negated negations. In that case, introduce a new method in the class being tested or a function that doesn’t need to be negated.

Introducing new methods is pretty cheap because they are not used in any other part of the code and you are not removing existing code. You can introduce progressively the use of the new code.

By the way, we want to mention that this kind of boolean properties are usually best represented by positive names that are easy to process even when they are being negated. Nevertheless, you should pay attention to the business value of that property in order to choose the best name.

We will back to conditional expressions in future articles about moving knowledge to the right place.

Refactor to guard clause

Conditionals can be tricky. And sometimes, in subtle ways. Take a look at this code, for example. It works perfectly, but can you spot the problem?

The problem is that the main responsibility of this method is under a condition. That makes the method name and body contradict each other in a way.

As a general rule is better to check requisites first and fail fast if they are not met. Or, as in this example, to return early doing nothing.

This kind of conditional is called a guard clause and its purpose is to avoid invalid data to reach the main processing. Guard clauses are used to ensure preconditions are met before proceed with the job in that method. We can use assertions or throw an exception if we prefer to fail.

Attack on the block

Not only magic values need names. A lot of parts of the code will benefit from having good intent revealing names.

In general, every cohesive code block could be extracted to a private method, giving it an expressive name. Let’s return to the extract method refactor in a future post about long methods and classes, so we will pay attention for now to some refactor opportunities in which extract method contributes to better understability.

Body of loops. It is a good practice to separate iterations from actions in loops. To do this, extract the body of the loop to its own method. You can see an example here:

Of course, in this example you could go functional, but take into account that this approach could be way more difficult to understand at first sight:

Conditional legs. This extraction can also be applied to the legs of conditionals, making them easier to understand at a higher level. You simply has to extract legs to private methods. Let’s consider this example:

We can extract the body of the true branch to its own method.

Now, we are hiding the details in the extracted method and can scan the conditional structure faster, digging deeper if we need.

This also paves the way for applying further refactors. If you look at the code you will see that this part is pretty strange and it will need more work to get a better organization.

Extracting the branches of conditionals to methods reduces the indentation level and makes it easier to tidy things.

To be continued…

In this post your have seen some techniques that can help your code to be a better storyteller, for you and for future developers.

In new posts we will talk about other areas of improvement. For example, how to deal with long classes and methods. Those long code blocks usually mix responsibilities and are difficult to manage and test.

Also, we will address the problem of moving knowledge and responsibilities to their proper places, applying some well known design principles.

Keep an eye on our blog for this series and other interesting posts.