Diseño ágil con TDD

8.5. Refactorizando

Cuando se avecina la jugada de copiar y pegar tests a diestro y siniestro, la cosa huele mal. ¿Qué necesitamos probar? Necesitamos asegurarnos de que el validador valida y de que todas las operaciones aritméticas preguntan al validador. En verdad es ésto lo que queremos. Nos hemos dado cuenta al identificar un mal olor. De acuerdo, modificamos los dos tests que hacen al validador comprobar los argumentos:

[Test]
public void ArgumentsExceedLimits()
{
    Calculator calculator = new Calculator(-100, 100);
    try
    {
        calculator.ValidateArgs(
            calculator.UpperLimit + 1, calculator.LowerLimit - 1);
        Assert.Fail("This should fail: arguments exceed limits");
    }
    catch (OverflowException)
    {
        // Ok, this works
    }
}

[Test]
public void ArgumentsExceedLimitsInverse()
{
    Calculator calculator = new Calculator(-100, 100);
    try
    {
        calculator.ValidateArgs(
            calculator.LowerLimit -1, calculator.UpperLimit + 1);
        Assert.Fail("This should fail: arguments exceed limits");
    }
    catch (OverflowException)
    {
        // Ok, this works
    }
}

¿Cómo comprobamos ahora que las operaciones aritméticas validan primero sin repetir código? Porque tal como está ahora el test sería el mismo código, solo que cambiando ValidateArgs por AddoSubstract. Lo que queremos validar no es el resultado de las funciones matemáticas, que ya está probado con otros tests, sino su comportamiento. Y cuando aparece la necesidad de validar comportamiento hay que detenerse un momento y analizar si las clases cumplen el Principio de una sola responsabilidad.

La clase Calculator se concibió para realizar operaciones aritméticas y ahora también está haciendo validaciones. Tiene más de una responsabilidad. De hecho el modificador public con que se definió el método ValidateArgs quedaba bastante raro, cualquiera hubiera dicho que se debería haber definido como privado.

A menudo los métodos privados son indicadores de colaboración entre clases, es decir, puede que en lugar de definir el método como privado sea más conveniente extraer una clase y hacer que ambas cooperen. Vamos a escribir el primer test que valida la cooperación entre la calculadora y el validador incluso aunque todavía no hemos separado el código... ¡El test siempre primero! y para ello nos servimos del framework Rhino.Mocks.

[Test]
public void SubstractIsUsingValidator()
{
    int arg1 = 10;
    int arg2 = -20;
    int upperLimit = 100;
    int lowerLimit = 100;
    var validatorMock =
        MockRepository.GenerateStrictMock<LimitsValidator>();
    validatorMock.Expect(x => x.ValidateArgs(arg1, arg2));

    Calculator calculator = new Calculator(validatorMock);
    calculator.Add(arg1, arg2);

    validatorMock.VerifyAllExpectations();
}

El código dice que hay un objeto que implementa la interfaz LimitsValidator y que se espera que se llame a su método ValidateArgs. Crea una instancia nueva de la calculadora y le inyecta el validador como parámetro en el constructor, aunque no es el validador verdadero sino un impostor (un mock).

A continuación se ejecuta la llamada al método de suma y finalmente se le pregunta al mock si las expectativas se cumplieron, es decir, si se produjo la llamada tal cual se especificó. Hemos decidido modificar el constructor de la calculadora para tomar una instancia de un validador en lugar de los valores límite. Al fin y al cabo los límites sólo le sirven al validador.

Parece que es lo que queríamos hacer pero... entonces, ¿para comprobar que todas las operaciones aritméticas hablan con el validador tenemos que copiar y pegar este test y modificarle una línea? ¡Sigue oliendo mal!

Los métodos de suma y resta no solo están realizando sus operaciones aritméticas respectivas, sino que incluyen una parte extra de lógica de negocio que es la que dice... antes y después de operar hay que validar.

¿No sería mejor si hubiese una clase que coordinase esto?. Desde luego el mal olor del copiar/pegar indica que hay que cambiar algo. Es cierto, si la responsabilidad de la calculadora (la clase Calculator, no la aplicación) es resolver operaciones pequeñas, que sea otra quien se encargue de operar comandos más complejos. Lo que queremos hacer tiene toda la pinta del patrón Decorador.

Nota En Python los decoradores existen como parte del lenguaje; son funciones que interceptan la llamada al método decorado. En este sentido tienen más potencia que los atributos de C# (lo que va entre corchetes), que no interceptan sino sólo marcan.

Por lo tanto un decorador a lo Python parece apropiado aquí. Sin embargo sé por experiencia que tal herramienta del lenguaje debe limitarse a funciones muy pequeñas que añaden atributos a la función decorada.

A veces con la propia carga de módulos Python en memoria se ejecuta el código de los decoradores con resultados impredecibles. Además si el código del decorador va más allá de etiquetar al decorado, estamos dejando de hacer programación orientada a objetos para regresar a la vieja programación procedimental.

En C# tenemos varias alternativas. La más común sería que la clase coordinadora implementase la misma interfaz de la calculadora y que tuviese una instancia de la calculadora internamente de manera que envolviese la llamada y le añadiese código.

Lo malo de esta solución es que nos lleva de nuevo a mucho código duplicado. Lo más elegante sería el patrón Proxy para interceptar la llamada. Una opción es Castle.DynamicProxy, que es la base de Rhino.Mocks, pero la curva de aprendizaje que conlleva usarlo, aunque es suave nos desvía de la materia que estamos tratando, por lo que vamos a implementar nuestra propia forma de proxy. Vamos a modificar el test anterior para explicar con un ejemplo qué es lo que que queremos:

[Test]
public void CoordinateValidation()
{
    int arg1 = 10;
    int arg2 = -20;
    int result = 1000;
    int upperLimit = 100;
    int lowerLimit = -100;

    var validatorMock =
        MockRepository.GenerateStrictMock<LimitsValidator>();
    validatorMock.Expect(x => x.SetLimits(
                        lowerLimit, upperLimit)).Repeat.Once();
    validatorMock.Expect(x => x.ValidateArgs(
                        arg1, arg2)).Repeat.Once();

    var calculatorMock =
            MockRepository.GenerateStrictMock<BasicCalculator>();
    calculatorMock.Expect(x => x.Add(arg1, arg2)).Return(result);

    validatorMock.Expect(x => x.ValidateResult(
                        result)).Repeat.Once();

    CalcProxy calcProxy =
        newCalcProxy(validatorMock, calculatorMock, lowerLimit, upperLimit);
    calcProxy.BinaryOperation(calculatorMock.Add, arg1, arg2);

    validatorMock.VerifyAllExpectations();
    calculatorMock.VerifyAllExpectations();
}

Lo que dice este ejemplo o test es lo siguiente: Existe un validador al cual se invocará mediante los métodos SetLimits y ValidateArgs consecutivamente (y una sola vez cada uno). Existe una calculadora que ejecutará su operación de suma y acto seguido el validador chequeará el resultado.

Hasta ahí hemos definido las expectativas. Ahora decimos que hay un proxy (CalcProxy) que recibe como parámetros de su constructor al validador, la calculadora y los límites máximos permitidos para las operaciones aritméticas. Queremos que exista un método BinaryOperation donde se indique el método de la calculadora a invocar y sus parámetros. Finalmente verificamos que la ejecución del proxy ha satisfecho las expectativas definidas. ¿Complicado, no?

Como vimos en el capítulo anterior, el test es realmente frágil. Cuenta con todo lujo de detalles lo que hace el SUT. Es como si quisiéramos implementarlo. Personalmente descarto esta opción. Pensar en este test y escribirlo me ha ayudado a pensar en el diseño pero he ido demasiado lejos. Si puedo evitar los mocks en este punto mejor y como ninguna de las operaciones requeridas infringen las reglas de los tests unitarios, voy a seguir utilizando validación de estado. Es momento de replantearse la situación.

¿De qué manera podemos probar que el supuesto proxy colabora con validador y calculadora sin usar mocks? Respuesta: Podemos ejercitar toda la funcionalidad de que disponemos a través del proxy y fijarnos en que no haya duplicidad. Si no hay duplicidad y todos los casos de uso se gestionan mediante el proxy, entonces tiene que ser que está trabajando bien.

Plantearlo así nos supone el esfuerzo de mover tests de sitio. Por ejemplo los de suma y resta los quitaríamos de la calculadora y los pondríamos en el proxy, ya que no los vamos a tener por duplicado. Empecemos por implementar primero el test de suma en el proxy:

[TestFixture]
public class CalcProxyTests
{
    private Calculator _calculator;
    private CalcProxy _calcProxy;

    [Test]
    public void Add()
    {
        _calculator = new Calculator();
        _calcProxy = new CalcProxy(_calculator);
        int result = _calcProxy.BinaryOperation(_calculator.Add, 2, 2);
        Assert.AreEqual(4, result);
    }
}

Por cierto, hemos eliminado el test de suma del conjunto CalculatorTests (para no duplicar). De la clase Calculator he movido las propiedades de límite inferior y límite superior a una clase Validator junto con el método ValidateArgs por si en breve los reutilizase. El SUT mímimo es:

public class CalcProxy
{
    private BasicCalculator _calculator;

    public CalcProxy(BasicCalculator calculator)
    {
        _calculator = calculator;
    }

    public int BinaryOperation(
        SingleBinaryOperation operation,
        int arg1, int arg2)
    {
        return_calculator.Add(arg1, arg2);
    }
}

He decidido que el primer parámetro del SUT es un delegado: public delegate int SingleBinaryOperation (int a, int b);

Nota En lugar de pasar una función como primer parámetro de BinaryOperationpodríamos haber usado una cadena de texto (Add) pero la experiencia nos dice que las cadenas son frágiles y hacen el código propenso a errores difíciles de corregir y detectar.

Si la persona que se está enfrentando a estas decisiones tuviese poca experiencia y hubiese decidido utilizar cadenas, igualmente tendría muchas ventajas al usar TDD.

Seguramente su código incluiría un gran bloque switch-case para actuar en función de las cadenas de texto y en algún momento pudiera ser que tuviese que reescribir funciones pero al tener toda una batería de pruebas detrás, tales cambios serían menos peligrosos, le darían mucha más confianza.

Así, aunque TDD no nos da siempre la respuesta a cuál es la mejor decisión de diseño, nos echa una mano cuando tenemos que retroceder y enmendar una decisión problemática.

En el capítulo 11 repetiremos la implementación con TDD pero sobre Python, asi que no se preocupe si algo no le queda del todo claro. Serán los mismos casos que en este capítulo pero marcados por las particularidades de Python. Además en el capítulo 9 continuamos trabajando con TDD, avanzando en la implementación de la solución. Vamos a triangular el proxy trasladando el test de la resta hasta él:

[TestFixture]
public class CalcProxyTests
{
    private Calculator _calculator;
    private CalcProxy _calcProxy;

    [SetUp]
    public void SetUp()
    {
        _calculator = new Calculator();
        _calcProxy = new CalcProxy(_calculator);
    }

    [Test]
    public void Add()
    {
        int result = _calcProxy.BinaryOperation(_calculator.Add, 2, 2);
        Assert.AreEqual(4, result);
    }

    [Test]
    public void Substract()
    {
        int result = _calcProxy.BinaryOperation(_calculator.Substract, 5, 3);
        Assert.AreEqual(2, result);
    }
}

Ya está más difícil buscar el código mínimo para que los dos tests pasen. No vamos a escribir un bloque condicional para conseguir luz verde porque eso no triangula a ninguna parte. Es hora de implementar algo más serio.

public int BinaryOperation(SingleBinaryOperation operation,
                           int arg1,int arg2)
{
    int result = 0;
    MethodInfo[] calcultatorMethods =
        _calculator.GetType().GetMethods(BindingFlags.Public |
                                         BindingFlags.Instance);
        foreach (MethodInfo method in calcultatorMethods)
        {
            if(method == operation.Method)
            {
                result = (int)method.Invoke(
                    _calculator,newObject[] { arg1, arg2 });
            }
        }
        return result;
}

Hemos usado un poco de magiaReflection8para buscar dinámicamente el método de la clase calculadora que toca invocar. Los dos tests pasan y ya han sido eliminados del conjunto en que se encontraban inicialmente. Estamos empezando a notar que reescribir no cuesta mucho cuando hacemos TDD.

Una pregunta frecuente de quienes comienzan a aprender TDD es si los tests se pueden modificar. Aquí estamos viendo claramente que sí. Se pueden modificar tantas veces como haga falta porque un test es código vivo, tan importante como el SUT. Lo único inamovible es el test de aceptación porque ha sido definido por el cliente. Al menos es inamovible hasta la siguiente reunión de fin de sprint con el cliente (sprint si usamos Scrum). Terminemos de mover los tests de calculadora al proxy:

[TestFixture]
public class CalcProxyTests
{
    private Calculator _calculator;
    private CalcProxy _calcProxy;

    [SetUp]
    public void SetUp()
    {
        _calculator = new Calculator();
        _calcProxy = new CalcProxy(_calculator);
    }

    [Test]
    public void Add()
    {
        int result =
            _calcProxy.BinaryOperation(_calculator.Add, 2, 2);
        Assert.AreEqual(4, result);
    }

    [Test]
    public void Substract()
    {
        int result = _calcProxy.BinaryOperation(_calculator.Substract, 5, 3);
        Assert.AreEqual(2, result);
    }

    [Test]
    public void AddWithDifferentArguments()
    {
    int result = _calcProxy.BinaryOperation(_calculator.Add, 2, 5);
    Assert.AreEqual(7, result);
    }

    [Test]
    public void SubstractReturningNegative()
    {
        int result = _calcProxy.BinaryOperation(_calculator.Substract, 3, 5);
        Assert.AreEqual(-2, result);
    }
}

Perfecto, todos pasan estupendamente con un esfuerzo mínimo. Repasemos la libreta:

# Aceptación - "2 + 2", devuelve 4
* La cadena "2 + 2" tiene dos números y un operador que son '2', '2' y '+'
# Aceptación - "5 + 4*2 / 2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "* *4 - 2": produce ERROR
# Aceptación - "*4 5 - 2": produce ERROR
# Aceptación - "*4 5 - 2 : produce ERROR
# Aceptación - "*45-2-": produce ERROR
# Aceptación - "2 + -2" devuelve 0
# Aceptación - Límite Superior = 100
# Aceptación - Límite Superior = 500
# Aceptación - Límite Inferior = -1000
# Aceptación - Límite Inferior = -10
* A: El primer argumento sobrepasa el límite superior
* B: El primer argumento sobrepasa el límite inferior
* C: El segundo argumento sobrepasa el límite superior
* D: El segundo argumento sobrepasa el límite inferior
* E: El resultado de una operación sobrepasa el límite superior
* F: El resultado de una operación sobrepasa el límite inferior
* Todos los casos de uso anteriores se aplican a todas las operaciones aritméticas

Habíamos llegado a este punto para coordinar la validación de argumentos y resultados. No vamos a implementar el validador con su propio conjunto de tests para luego moverlos al proxy sino que ya con las ideas claras y el diseño más definido podemos ejercitar el SUT desde el proxy:

[Test]
public void ArgumentsExceedLimits()
{
    CalcProxy calcProxyWithLimits =
        new CalcProxy(newValidator(-10, 10), _calculator);

    try {
        _calcProxy.BinaryOperation(_calculator.Add, 30, 50);
        Assert.Fail("This should fail as arguments exceed both limits");
    }
    catch(OverflowException)
    {
        // Ok, this works
    }
}

He decidido que el proxy tiene un constructor que recibe al validador y a la calculadora. Al validador se le indican los valores límite vía constructor. El SUT:

public int BinaryOperation(SingleBinaryOperation operation,
                           int arg1,int arg2)
{
    _validator.ValidateArgs(arg1, arg2);

    int result = 0;
    MethodInfo[] calcultatorMethods =
            _calculator.GetType().GetMethods(BindingFlags.Public |
                                             BindingFlags.Instance);
    foreach (MethodInfo method in calcultatorMethods)
    {
        if(method == operation.Method)
        {
            result = (int)method.Invoke(
                _calculator,newObject[] { arg1, arg2 });
        }
    }
    return result;
}

El método simplemente añade una línea al código anterior. Para que el test pase rescatamos el método de validación que teníamos guardado en el validador.

public class Validator : LimitsValidator
{
    private int _upperLimit;
    private int _lowerLimit;

    public Validator(int lowerLimit,int upperLimit)
    {
        SetLimits(lowerLimit, upperLimit);
    }

    public int LowerLimit
    {
        get { return _lowerLimit; }
        set { _lowerLimit = value; }
    }

    public int UpperLimit
    {
        get { return _upperLimit; }
        set { _upperLimit = value; }
    }

    public void ValidateArgs(int arg1,int arg2)
    {
        if(arg1 > _upperLimit)
            throw newOverflowException("ERROR");
        if(arg2 > _upperLimit)
            throw newOverflowException("ERROR");
    }

    public void SetLimits(int lower, int upper)
    {
        _lowerLimit = lower;
        _upperLimit = upper;
    }
}

Nos queda probar el límite inferior.

[Test]
public void ArgumentsExceedLimitsInverse()
{
    CalcProxy calcProxyWithLimits =
        new CalcProxy(new Validator(-10, 10), _calculator);

    try
    {
        calcProxyWithLimits.BinaryOperation(_calculator.Add, -30, -50);
        Assert.Fail("This should fail as arguments exceed both limits");
    }
    catch (OverflowException)
    {
        // Ok, this works
    }
}

El SUT junto con su posterior refactoring:

public class Validator : LimitsValidator
{
    private int _upperLimit;
    private int _lowerLimit;

    public Validator(int lowerLimit, int upperLimit)
    {
        SetLimits(lowerLimit, upperLimit);
    }

    public int LowerLimit
    {
        get { return _lowerLimit; }
        set { _lowerLimit = value; }
    }

    public int UpperLimit
    {
        get { return _upperLimit; }
        aet { _upperLimit = value; }
    }

    public void ValidateArgs(int arg1, int arg2)
    {
        breakIfOverflow(arg1, "First argument exceeds limits");
        breakIfOverflow(arg2, "Second argument exceeds limits");
    }

    private void breakIfOverflow(int arg, string msg)
    {
        if(ValueExceedLimits(arg))
            throw newOverflowException(msg);
    }

    public bool ValueExceedLimits(int arg)
    {
        if(arg > _upperLimit)
            return true;
        if(arg < _lowerLimit)
            return true;
        return false;
    }

    public void SetLimits(int lower,int upper)
    {
        _lowerLimit = lower;
        _upperLimit = upper;
    }
}

Ya podemos quitar de la libreta unas cuantas líneas:

# Aceptación - "2 + 2", devuelve 4
* La cadena "2 + 2" tiene dos números y un operador que son '2', '2' y '+'
# Aceptación - "5 + 4*2 / 2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "* *4 - 2": produce ERROR
# Aceptación - "*4 5 - 2": produce ERROR
# Aceptación - "*4 5 - 2 : produce ERROR
# Aceptación - "*45-2-": produce ERROR
# Aceptación - "2 + -2" devuelve 0
* E: El resultado de una operación sobrepasa el límite superior
* F: El resultado de una operación sobrepasa el límite inferior
* Todos los casos de uso anteriores se aplican a todas las operaciones aritméticas

Solo nos queda validar el resultado. Los dos ejemplos y su implementación son inmediatos. Pero siempre de uno en uno:

[Test]
public void ValidateResultExceedingUpperLimit()
{
    try
    {
        _calcProxyWithLimits.BinaryOperation(_calculator.Add, 10, 10);
        Assert.Fail("This should fail as result exceeds upper limit");
    }
    catch (OverflowException)
    {
        // Ok, this works
    }
}
public int BinaryOperation(SingleBinaryOperation operation,
                          int arg1, int arg2)
{
    _validator.ValidateArgs(arg1, arg2);

    int result = 0;
    MethodInfo[] calcultatorMethods =
        _calculator.GetType().GetMethods(BindingFlags.Public |
                                         BindingFlags.Instance);
    foreach (MethodInfo method in calcultatorMethods)
    {
        if (method == operation.Method)
        {
            result = (int)method.Invoke(
                _calculator, new Object[] { arg1, arg2 });
        }
    }
    _validator.ValidateResult(result);
    return result;
}

Le hemos añadido una línea al método para validar el resultado. El resto del SUT en el validador:

public void ValidateResult(int result)
{
    breakIfOverflow(result, "Result exceeds limits");
}
[Test]
public void ValidateResultExceedingLowerLimit()
{
    try
    {
        _calcProxyWithLimits.BinaryOperation(_calculator.Add, -20, -1);
        Assert.Fail("This should fail as result exceeds lower limit");
    }
    catch (OverflowException)
    {
        // Ok, this works
    }
}

Para este último test ni siquiera ha hecho falta tocar el SUT. La libreta queda así:

# Aceptación - "2 + 2", devuelve 4
* La cadena "2 + 2" tiene dos números y un operador que son '2', '2' y '+'
# Aceptación - "5 + 4*2 / 2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "* *4 - 2": produce ERROR
# Aceptación - "*4 5 - 2": produce ERROR
# Aceptación - "*4 5 - 2 : produce ERROR
# Aceptación - "*45-2-": produce ERROR
# Aceptación - "2 + -2" devuelve 0

Copyright (c) 2010-2013 Carlos Ble. La copia y redistribución de esta página se permite bajo los términos de la licencia Creative Commons Atribución SinDerivadas 3.0 Unported siempre que se conserve esta nota de copyright.