uniwebsidad

Diseño ágil con TDD

Capítulo 9. Continuación del proyecto - Test Unitarios

En el último capítulo llegamos a conseguir que nuestra calculadora sumase y restase teniendo en cuenta los valores límite de los parámetros y del resultado. Continuamos el desarrollo por donde lo dejamos atendiendo a lo que pone 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

Es el momento de evaluar la cadena de texto que se utiliza para introducir expresiones y conectarla con la funcionalidad que ya tenemos. Empezamos a diseñar partiendo de un ejemplo, como siempre:

[TestFixture]
public classParserTests
{
    [Test]
    public void GetTokens()
    {
        MathParser parser = new MathParser();
        List<MathToken> tokens = parser.GetTokens("2 + 2");

        Assert.AreEqual(3, tokens.Count);
        Assert.AreEqual("2", tokens[0].Token);
        Assert.AreEqual("+", tokens[1].Token);
        Assert.AreEqual("2", tokens[2].Token);
    }
}

Acabo de tomar varias decisiones de diseño: MathParser es una clase con un método GetTokens que recibe la expresión como una cadena y devuelve una lista de objetos tipo MathToken. Tales objetos todavía no existen pero prefiero pensar en la expresión como en una lista de objetos en lugar de una lista de cadenas. La experiencia me dice que devolver cadenas no me hará progresar mucho. La implementación mínima para alcanzar verde:

public classMathParser
{
    public List<MathToken> GetTokens(string expression)
    {
        List<MathToken> tokens = new List<MathToken>();

        tokens.Add(newMathToken("2"));
        tokens.Add(newMathToken("+"));
        tokens.Add(newMathToken("2"));

        return tokens;
    }
}

La simplicidad de este SUT nos sirve para traer varias preguntas a la mente. Afortunadamente las respuestas ya se encuentran en la libreta: sabemos qué expresiones son válidas y cuales no. Además sabemos que en caso de encontrar una cadena incorrecta lanzaremos una excepción. Podríamos triangular hacia el reconocimiento de las expresiones con sentencias y bloques de código varios pero las expresiones regulares son la mejor opción llegado este punto.

En lugar de construir de una vez la expresión regular que valida todo tipo de expresiones matemáticas, vamos a triangular paso a paso. Una expresión regular compleja nos puede llevar días de trabajo depurando. Si construimos la expresión basándonos en pequeños ejemplos que vayan casando con cada subexpresión regular, más tarde, su modificación y sofisticación, nos resultará mas sencilla.

TDD es ideal para diseñar expresiones regulares si mantenemos la máxima de escribir un test exclusivo para cada posible cadena válida. Vamos con el ejemplo que construirá la primera versión de la expresión regular:

[Test]
public void ValidateMostSimpleExpression()
{
    string expression = "2 + 2";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsTrue(result);
}

En lugar de un método void me ha parecido mejor idea que devuelva verdadero o falso para facilitar la implementación de los tests. En vez de retornar verdadero directamente podemos permitirnos construir la expresión regular más sencilla que resuelve el ejemplo:

public bool IsExpressionValid(string expression)
{
    Regex regex = new Regex(@"\d\+\d");
    return regex.IsMatch(expression);
}

¿Qué tal se comporta con números de más de una cifra?

[Test]
public void ValidateMoreThanOneDigitExpression()
{
    string expression = "25 + 287";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsTrue(result);
}

¡Funciona! No hemos tenido que modificar el SUT. Ahora vamos a probar con los cuatro operadores aritméticos. En lugar de hacer cuatro tests nos damos cuenta de que la expresión que queremos probar es la misma, aunque variando el operador. Eso nos da permiso para agrupar los cuatro usos en un solo ejemplo:

[Test]
public void ValidateSimpleExpressionWithAllOperators()
{
    string operators = "+-*/";
    string expression = String.Empty;

    foreach (charoperatorChar in operators)
    {
        expression = "2" + operatorChar + "2";
        Assert.IsTrue(
            _parser.IsExpressionValid(expression),
            "Failure with operator:" + operatorChar);
    }
}

El peligro de este ejemplo es que estemos construyendo mal la cadena, en cuyo caso diseñaríamos mal el SUT. Después de escribirlo la he mostrado por consola para asegurarme que era la que quería. En mi opinión merece la pena asumir el riesgo para agrupar tests de una forma ordenada. Fíjese que en el Assert he añadido una explicación para que sea más sencilla la depuración de bugs.

Incrementamos la expresión regular para hacer el test pasar.

public bool IsExpressionValid(string expression)
{
    Regex regex = new Regex(@"\d[+|\-|/|*]\d");

    return regex.IsMatch(expression);
}

El test pasa. Podemos eliminar el primero que habíamos escrito (ValidateMostSimpleExpression) ya que está contenido en el último. Es importante recordar que el código de los tests es tan importante como el del SUT y que por tanto debemos cuidarlo y mantenerlo.

Me asalta una duda... ¿podrá haber varios espacios entre los distintos elementos de la expresión? Preguntamos, nos confirman que sí es posible y anotamos en la libreta.

# Aceptación - "2 + 2", devuelve 4
* La cadena "2 + 2" tiene dos números y un operador que son '2', '2' y '+'
* Se permiten varios espacios entre símbolos
# 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

De acuerdo, probémoslo:

[Test]
public void ValidateWithSpaces()
{
    string expression = "2 + 287";
    bool result = _parser.IsExpressionValid(expression);
    Assert.IsTrue(result);
}

Mejoramos la expresión regular:

public bool IsExpressionValid(string expression)
{
    Regex regex = new Regex(@"\d\s+[+|\-|/|*]\s+\d");

    return regex.IsMatch(expression);
}

¿Estará cubierto el caso en que no se dejan espacios?

[Test]
public void ValidateFailsNoSpaces()
{
    string expression = "2 + 7";
    bool result = _parser.IsExpressionValid(expression);
    Assert.IsFalse(result);
}

Pues sí, funciona sin que tengamos que tocar el SUT. Escogemos nuevas expresiones de la libreta:

[Test]
public void ValidateComplexExpression()
{
    string expression = "2 + 7 - 2 * 4";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsTrue(result);
}

Vaya, esta pasa incluso sin haber modificado la expresión regular. Resulta que, como una subcadena de la expresión casa, nos la está dando por buena. Busquemos un test que nos obligue a modificar la expresión regular:

[Test]
public void ValidateComplexWrongExpression()
{
    string expression = "2 + 7 a 2 b 4";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsFalse(result);
}
// MathParser
public bool IsExpressionValid(string expression)
{
    Regex regex = new Regex(
        @"^\d((\s+)[+|\-|/|*](\s+)\d)+$");

    return regex.IsMatch(expression, 0);
}

Algunos tests que antes funcionaban están fallando. Vamos a retocar más la expresión regular:

public bool IsExpressionValid(string expression)
{
    Regex regex = new Regex(
        @"^\d+((\s+)[+|\-|/|*](\s+)\d+)+$");

    return regex.IsMatch(expression, 0);
}

El hecho de que algunos otros tests se hubieran roto me ha creado cierta desconfianza. Vamos a probar unas cuantas expresiones más para verificar que nuestra validación es buena.

[Test]
public void ValidateSimpleWrongExpression()
{
    string expression = "2a7";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsFalse(result);
}

El test pasa. A por otro caso:

[Test]
public void ValidateWrongExpressionWithValidSubexpression()
{
    string expression = "2 + 7 - 2 a 3 b";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsFalse(result);
}

También funciona. ¿Qué tal con dos operadores consecutivos?

[Test]
public void ValidateWithSeveralOperatorsTogether()
{
    string expression = "+ + 7";
    bool result = _parser.IsExpressionValid(expression);

    Assert.IsFalse(result);
}

Correcto, luz verde. La expresión que nos queda por probar de las que tiene la libreta es aquella que contiene números negativos:

[Test]
public void ValidateWithNegativeNumers()
{
    Assert.IsTrue(_parser.IsExpressionValid("- 7 + 1"));
}

He aprovechado para simplificar el test sin que pierda legibilidad. Por cierto, está en rojo; hay que retocar la expresión regular.

// MathParser
public bool IsExpressionValid(string expression)
{
    Regex regex =new Regex(
        @"^-{0,1}\d+((\s+)[+|\-|/|*](\s+)-{0,1}\d+)+$");
    return regex.IsMatch(expression, 0);
}

Funciona. Probemos alguna variante:

[Test]
public void ValidateWithNegativeNumersAtTheEnd()
{
    Assert.IsTrue(_parser.IsExpressionValid("7 - -1"));
}

Sigue funcionando. Vamos a por la última prueba del validador de expresiones.

[Test]
public void ValidateSuperComplexExpression()
{
    Assert.IsTrue(_parser.IsExpressionValid(
        "-7 - - 1 * 2 / 3 + -5"));
}

Me da la sensación de que nuestro validador de expresiones ya es suficientemente robusto. Contiene toda la funcionalidad que necesitamos por ahora. ¿Dónde estábamos?

# 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

¡Ah si!, le estábamos pidiendo al analizador que nos devolviera una lista con los elementos de la expresión. Habíamos hecho pasar un test con una implementación mínima pero no llegamos a triangular:

[Test]
public void GetTokensLongExpression()
{
    List<MathToken> tokens = _parser.GetTokens("2 - 1 + 3");

    Assert.AreEqual(5, tokens.Count);
    Assert.AreEqual("+", tokens[3].Token);
    Assert.AreEqual("3", tokens[4].Token);
}

Nótese que no repetimos las afirmaciones referentes a los tokens 0, 1 y 2 que ya se hicieron en el test anterior para una expresión que es casi igual a la actual.

public List<MathToken> GetTokens(string expression)
{
    List<MathToken> tokens = new List<MathToken>();
    String[] items = expression.Split('');
    foreach (String item in items)
    {
        tokens.Add(new MathToken(item));
    }

    return tokens;
}

Tengo la sensación de que la clase Parser empieza a tener demasiadas responsabilidades. Refactoricemos:

public class MathLexer
{
    public List<MathToken> GetTokens(string expression)
    {
        List<MathToken> tokens = new List<MathToken>();
        String[] items = expression.Split('');
        foreach (String item in items)
        {
            tokens.Add(new MathToken(item));
        }
        return tokens;
    }
}

public class ExpressionValidator
{
    public bool IsExpressionValid(string expression)
    {
        Regex regex =
            new Regex(@"^-{0,1}\d+((\s+)[+|\-|/|*](\s+)-{0,1}\d+)+$");

        return regex.IsMatch(expression, 0);
    }
}

public classMathParser
{
}

Hemos tenido que renombrar algunas variables en los tests para que pasen después de esta refactorización pero ha sido rápido. Los he dejado dentro del conjunto de tests ParserTests aunque ahora se ha quedado vacía la clase Parser.

La libreta dice que ante una expresión inválida el analizador producirá una excepción. Escribamos un ejemplo que lo provoque:

[Test]
public void GetTokensWrongExpression()
{
    try
    {
        List<MathToken> tokens = _lexer.GetTokens("2-1++3");
        Assert.Fail("Exceptiondidnotarise!");
    }
    catch (InvalidOperationException)
    { }
}

Nos hemos decantado por InvalidOperationException. Ahora podríamos escribir un hack veloz y triangular pero es un poco absurdo teniendo ya un validador de expresiones. Inyectemos el validador:

public classMathLexer
{
    ExpressionValidator _validator;

    public MathLexer(ExpressionValidator validator)
    {
        _validator = validator;
    }

    public List<MathToken> GetTokens(string expression)
    {
        if (!_validator.IsExpressionValid(expression))
            throw new InvalidOperationException(expression);

        List<MathToken> tokens = new List<MathToken>();
        String[] items = expression.Split('');
        foreach (String item in items)
        {
            tokens.Add(newMathToken(item));
        }
        return tokens;
    }
}

¿Se creará bien la lista de tokens cuando haya varios espacios seguidos? Mejor lo apuntalamos con un test:

[Test]
public void GetTokensWithSpaces()
{
    List<MathToken> tokens = _lexer.GetTokens("5 - 88");

    Assert.AreEqual("5", tokens[0].Token);
    Assert.AreEqual("-", tokens[1].Token);
    Assert.AreEqual("88", tokens[2].Token);
}

Pues resulta que no funciona. Luz roja. Deberíamos poder partir por cualquier carácter en blanco:

public List<MathToken> GetTokens(string expression)
{
if (!_validator.IsExpressionValid(expression))
    throw newInvalidOperationException(expression);

    List<MathToken> tokens = new List<MathToken>();
    String[] items = expression.Split((new char[] {', '\t'}),
                     StringSplitOptions.RemoveEmptyEntries);
    foreach (String item in items)
    {
        tokens.Add(new MathToken(item));
    }
    return tokens;
}

OK luz verde. Refactorizo un poco:

public List<MathToken> GetTokens(string expression)
{
    if(!_validator.isExpressionValid(expression))
        throw new InvalidOperationException(expression);

    string[] items = splitExpression(expression);
    return createTokensFromStrings(items);
}

private string[] splitExpression(string expression)
{
    return expression.Split((new char[] { ', '\t' }),
        StringSplitOptions.RemoveEmptyEntries);
}

private List<MathToken> createTokensFromStrings(string[] items)
{
    List<MathToken> tokens = new List<MathToken>();
    foreach (String item in items)
    {
        tokens.Add(new MathToken(item));
    }
    return tokens;
}

Limpiemos la lista para ver qué toca ahora:

# Aceptación - "2 + 2", devuelve 4
# Aceptación - "5 + 4*2/2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "2 + -2", devuelve 0

El primer test de aceptación de la lista nos exige comenzar a unir las distintas piezas que hemos ido creando. Por una lado sabemos que somos capaces de sumar y por otro ya conseguimos la lista de tokens de la expresión. Queda conectar ambas cosas. En este caso concreto el test de aceptación lo podemos expresar con NUnit. Aunque voy a intentar que cumpla con algunas normas de los tests unitarios (inocuo y rápido) no es atómico así que no me atrevería a llamarle unitario sino simplemente funcional.

[Test]
public void ProcessSimpleExpression()
{
    MathParser parser = new MathParser();
    Assert.AreEqual(4, parser.ProcessExpression("2 + 2"));
}

Antes de implementar el SUT mínimo, me parece buena idea escribir un par de tests unitarios que fuercen la colaboración entre los objetos que tenemos. Así no se me olvida utilizarlos cuando me adentre en detalles de implementación:

[Test]
public void ParserWorksWithCalcProxy()
{
    CalculatorProxy calcProxyMock =
        MockRepository.GenerateMock<CalculatorProxy>();
    calcProxyMock.Expect(x =>
        x.Calculator).Return(_calculator);
    calcProxyMock.Expect(
        x => x.BinaryOperation(_calculator.Add, 2,
                                    2)).Return(4);

    MathParser parser =
        new MathParser(calcProxyMock);
    parser.ProcessExpression("2 + 2");

    calcProxyMock.VerifyAllExpectations();
}

Para escribir el test tuve que extraer la interfaz CalculatorProxy a partir de la clase CalcProxy. La intención es forzar la colaboración. No me gusta tener que ser tan explícito al definir la llamada a la propiedad Calculator del proxy en la línea 6. Siento que me gustaría que Calculator estuviese mejor encapsulado dentro del proxy. Es algo que tengo en mente arreglar tan pronto como un requisito me lo pida. Y seguro que aparece pronto. Conseguimos el verde rápido:

public class MathParser
{
    CalculatorProxy _calcProxy;

    public MathParser(CalculatorProxy calcProxy)
    {
        _calcProxy = calcProxy;
    }

    public int ProcessExpression(string expression)
    {
        return _calcProxy.BinayOperation(
            _calcProxy.Calculator.Add, 2, 2);
    }
}

Forcemos también la colaboración con MathLexer:

[Test]
public void ParserWorksWithLexer()
{
    List<MathToken> tokens = new List<MathToken>();
    tokens.Add(new MathToken("2"));
    tokens.Add(new MathToken("+"));
    tokens.Add(new MathToken("2"));
    Lexer lexerMock =
        MockRepository.GenerateStrictMock<Lexer>();
    lexerMock.Expect(
        x => x.GetTokens("2 + 2")).Return(tokens);

    MathParser parser = new MathParser(lexerMock,
        new CalcProxy(new Validator(-100, 100),
                            new Calculator()));
    parser.ProcessExpression("2 + 2");

    lexerMock.VerifyAllExpectations();
}

Extraje la interfaz Lexer para generar el mock. El SUT va tomando forma:

public class MathParser
{
    Lexer _lexer;
    CalculatorProxy _calcProxy;

    public MathParser(Lexer lexer, CalculatorProxy calcProxy)
    {
        _lexer = lexer;
        _calcProxy = calcProxy;
    }

    public int ProcessExpression(string expression)
    {
        List<MathToken> tokens = _lexer.GetTokens(expression);
        return _calcProxy.BinaryOperation(
                        _calcProxy.Calculator.Add,
                        tokens[0].IntValue,
                        tokens[2].IntValue);
    }
}

Modifiqué el test anterior ya que el constructor ha cambiado. Estos dos tests nos posicionan en un código mínimo sin llegar a ser el clásico return 4. Buen punto de partida para triangular hacia algo más útil.

[Test]
public void ProcessExpression2Operators()
{
    Assert.AreEqual(6 ,
        _parser.ProcessExpression("3 + 1 + 2"));
}

Voy a implementar un código mínimo que resuelva la operación procesando la entrada de izquierda a derecha:

public int ProcessExpression(string expression)
{
    List<MathToken> tokens = _lexer.GetTokens(expression);
    MathToken total = tokens[0];
    for (int i = 0; i < tokens.Count; i++)
    {
        if (tokens[i].isOperator())
        {
            MathToken totalForNow = total;
            MathToken nextNumber = tokens[i + 1];
            int partialResult =
                _calcProxy.BinaryOperation(
                    _calcProxy.Calculator.Add,
                    totalForNow.IntValue, nextNumber.IntValue);
            total = new MathToken(partialResult.ToString());
            i++;
        }
    }
    return total.IntValue;
}

Lo más sencillo que se me ha ocurrido es coger los operadores y dar por sentado que los operandos (números) están a su izquierda y derecha. Al escribir el código me he dado cuenta que necesitaba la función isOperator en la clase MathToken y he hecho uso de ella sin que esté implementada. Así pues dejo a un lado el SUT y voy a por un test que me ayude a implementar dicha función, o sea, cambio de SUT un momento.

[TestFixture]
public class MathTokenTests
{
    [Test]
    public void isOperator()
    {
        MathToken numberToken = new MathToken("22");
        Assert.IsFalse(numberToken.isOperator());
    }
}
public bool isOperator()
{
    string operators = "+-*/";
    foreach (char op in operators)
    {
        if (_token == op.ToString())
            return true;
    }
    return false;
}

Ahora el caso positivo:

[Test]
public void isOperatorTrue()
{
    MathToken numberToken = new MathToken("*");
    Assert.IsTrue(numberToken.isOperator());
}

Funciona. Podemos regresar al SUT anterior y ejecutar el test para comprobar que ... ¡también funciona!. La cosa marcha. Revisamos la lista a ver cómo vamos:

# Aceptación - "5 + 4*2/2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "2 + -2", devuelve 0

¡No hemos tenido en cuenta la precedencia de operadores! El test anterior no la exigía y me centré tanto en el código mínimo que cumplía con la especificación, que olvidé el criterio de aceptación de la precedencia de operadores.

No pasa nada, seguro que es más fácil partir de aquí hacia la solución del problema que haber partido de cero. Cuando el problema se hace complejo como en el caso que nos ocupa, es especialmente importante no saltarse la regla de diseñar en pequeños pasos. Quizás si hubiésemos contemplado el caso complejo desde el principio hubiésemos olvidado casos que a la larga se hubiesen traducido en bugs.

Antes de seguir voy a refactorizar un poco los tests, moviendo los que son de validación de expresiones a un conjunto fuera de ParserTests ya que se está convirtiendo en una clase con demasiados tests1. Una vez movidos los tests tenemos que diseñar una estrategia para la precedencia de los operadores.

De paso podemos añadir a la lista los casos en que se utilizan paréntesis en las expresiones, para irles teniendo en mente a la hora de tomar decisiones de diseño:

# Aceptación - "5 + 4*2/2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "(2 + 2)*(3 + 1)", devuelve 16
# Aceptación - "2 + -2", devuelve 0

Una vez más. Un ejemplo sencillo primero para afrontar el SUT:

[Test]
public void ProcessExpressionWithPrecedence()
{
    Assert.AreEqual(9, _parser.ProcessExpression("3 + 3 * 2"));
}

Si el operador tuviera asociado un valor para su precedencia,podría buscar aquellos de mayor precedencia, operar y a continuación hacer lo mismo con los de menor. De paso para encapsular mejor la calculadora podría mover las llamadas al proxy a una clase operador.

Como el test que nos ocupa me parece demasiado grande para implementar el SUT de una sola vez, voy a escribir otro test que me sirva para obtenerla precedencia más alta de la expresión. Un poco más adelante retomaré el test que acabo de escribir.

[Test]
public void GetMaxPrecedence()
{
    List<MathToken> tokens = _lexer.GetTokens("3 + 3 * 2");
    MathOperator op = _parser.GetMaxPrecedence(tokens);
    Assert.AreEqual(op.Token, "*");
}

El SUT:

public MathOperator GetMaxPrecedence(List<MathToken> tokens)
{
    int precedence = 0;
    MathOperator maxPrecedenceOperator =null;

    foreach (MathToken token in tokens)
    {
        if (token.isOperator())
        {
            MathOperator op = OperatorFactory.Create(token);
            if(op.Precedence >= precedence)
            {
                precedence = op.Precedence;
                maxPrecedenceOperator = op;
            }
        }
    }
    return maxPrecedenceOperator;
}

No compila porque las clases MathOperator y OperatorFactory no existen y el método Create tampoco. Voy a intentar que compile lo más rápidamente posible:

public class MathOperator
{
    int _precedence = 0;
    string _token = String.Empty;

    public string Token
    {
        get { return _token; }
        set { _token = value; }
    }

    public intPrecedence
    {
        get { return_precedence; }
    }
}

public class OperatorFactory
{
    public static MathOperator Create(MathToken token)
    {
        MathOperator op = new MathOperator();
        op.Token = token.Token;
        return op;
    }
}

Bien. El test para el método de obtener la máxima precedencia funciona parcialmente pero no hemos triangulado. Para ello tenemos que probar que la factoría de operadores les pone precedencia:

[TestFixture]
public class OperatorFactoryTests
{
    [Test]
    public void CreateMultiplyOperator()
    {
        MathOperator op = OperatorFactory.Create(newMathToken("*"));
        Assert.AreEqual(op.Precedence, 2);
    }
}

SUT mínimo:

public static MathOperator Create(MathToken token)
{
    MathOperator op;
    if(token.Token == "*")
        op = new MathOperator(2);
    else
        op = new MathOperator(0);
    op.Token = token.Token;
    return op;
}

He tenido que añadir un constructor para MathOperator que reciba el valor de precedencia. El test pasa. Si escribo otro test para la división creo que por ahora tendré las precedencias resueltas:

[Test]
public void CreateDivisionOperator()
{
    MathOperator op = OperatorFactory.Create(newMathToken("/"));
    Assert.AreEqual(op.Precedence, 2);
}
public static MathOperator Create(MathToken token)
{
    MathOperator op;
    if((token.Token == "*") || (token.Token == "/"))
        op = new MathOperator(2);
    else
        op =newMathOperator(0);
    op.Token = token.Token;
    return op;
}

Perfecto los tests pasan. Tanto MathToken como MathOperator comparten la propiedad Token. Empiezo a pensar que deberían compartir una interfaz. Podría refactorizar pero habrá que refactorizar más cosas pronto. Primero voy a terminar de implementar el SUT para el test que habíamos dejado en el aire:

[Test]
public void ProcessExpressionWithPrecedence()
{
    Assert.AreEqual(9, _parser.ProcessExpression("3 + 3 * 2"));
}

El SUT:

public int ProcessExpression(string expression)
{
    List<MathToken> tokens = _lexer.GetTokens(expression);
    while(tokens.Count > 1)
    {
        MathOperator op = GetMaxPrecedence(tokens);
        int firstNumber = tokens[op.Index -1].IntValue;
        int secondNumber = tokens[op.Index +1].IntValue;
        int result = op.Resolve(firstNumber, secondNumber);
        tokens[op.Index - 1] = new MathToken(result.ToString());
        tokens.RemoveAt(op.Index);
        tokens.RemoveAt(op.Index);
    }
    return tokens[0].IntValue;
}

He simplificado el algoritmo. Me limito a buscar el operador de mayor prioridad, operar los números a su izquierda y derecha y sustituir los tres elementos por el resultado. He necesitado una propiedad Index en el operador que no existía así que para poder compilar la añado:

public MathOperator GetMaxPrecedence(List<MathToken> tokens)
{
    int precedence = 0;
    MathOperator maxPrecedenceOperator = null;

    int index = -1;
    foreach (MathToken token in tokens)
    {
        index++;
        if (token.isOperator())
        {
            MathOperator op = OperatorFactory.Create(token);
            if (op.Precedence >= precedence)
            {
                precedence = op.Precedence;
                maxPrecedenceOperator = op;
                maxPrecedenceOperator.Index = index;
            }
        }
    }
    return maxPrecedenceOperator;
}

El método Resolve del operador tampoco existe. En mi cabeza es el método que encapsula el uso del proxy y a su vez la calculadora. Voy a implementar un stub rápido para compilar.

public int Resolve(int a, int b)
{
    if(Token == "*")
        return a * b;
    if(Token == "+")
        return a + b;
    return 0;
}

Muy bien. Ahora pasan todos los tests menos aquel en el que forzábamos al analizador a utilizar el proxy, porque en el método Resolve hemos hecho un apaño rápido y feo. El primer cambio que voy a hacer es inyectar el proxy como parámetro en el método para utilizarlo:

public int Resolve(int a, int b, CalculatorProxy calcProxy)
{
    if(Token == "*")
        return a * b;
    if(Token == "+")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Add, a, b);
    return 0;
}

Perfecto. Todos los tests pasan. No he utilizado el proxy para la multiplicación porque no la tenemos implementada. Voy a añadir un par de tests sencillos de multiplicación y división para completar la funcionalidad de la clase Calculator. Los tests estaban dentro del conjunto de tests del proxy:

[Test]
public void Multiply()
{
    Assert.AreEqual(
        _calcProxy.BinaryOperation(_calculator.Multiply,
                                                2, 5), 10);
}

Clase Calculator:

public int Multiply(int arg1, int arg2
{
    return arg1 * arg2;
}
[Test]
public void Division()
{
    Assert.AreEqual(
        _calcProxy.BinaryOperation(_calculator.Divide,
                                                10, 2), 5);
}
public int Divide(int arg1,int arg2)
{
    return arg1 / arg2;
}

Bien, voy a completar el método que resuelve en el operador:

public int Resolve(int a, int b, CalculatorProxy calcProxy)
{
    if (Token == "*")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Multiply, a, b);
    if(Token == "+")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Add, a, b);
    return 0;
}

Todos los tests pasan. Aprovecho para refactorizar:

public int ProcessExpression(string expression)
{
    List<MathToken> tokens = _lexer.GetTokens(expression);
    while(tokens.Count > 1)
    {
        MathOperator op = GetMaxPrecedence(tokens);
        int firstNumber = tokens[op.Index -1].IntValue;
        int secondNumber = tokens[op.Index +1].IntValue;
        int result = op.Resolve(firstNumber,
                                secondNumber, _calcProxy);
        replaceTokensWithResult(tokens, op.Index, result);
    }
    return tokens[0].IntValue;
}

private void replaceTokensWithResult(List<MathToken> tokens,
    int indexOfOperator, int result)
{
    tokens[indexOfOperator - 1] =
                new MathToken(result.ToString());
    tokens.RemoveAt(indexOfOperator);
    tokens.RemoveAt(indexOfOperator);
}

¿Cómo está la libreta?

# Aceptación - "5 + 4*2/2", devuelve 9
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "(2 + 2)*(3 + 1)", devuelve 16
# Aceptación - "2 + -2", devuelve 0

Estamos preparados para escribir un test de aceptación para la primera línea:

[Test]
public void ProcessAcceptanceExpression()
{
    Assert.AreEqual(9, _parser.ProcessExpression("5 + 4 * 2 / 2"));
}

Se esperaba 9 pero se devolvió 5. ¡Ah claro! Es que el método de resolver no implementa la división ni la resta. Qué despiste. Voy a añadir la división para que el test pase y luego escribo otro test con una resta para estar obligado a implementarla.

public int Resolve(int a, int b, CalculatorProxy calcProxy)
{
    if (Token == "*")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Multiply, a, b);
    if(Token == "+")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Add, a, b);
    if(Token == "/")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Divide, a, b);
    return 0;
}
[Test]
public void ProcessAcceptanceExpressionWithAllOperators()
{
    Assert.AreEqual(8,
        _parser.ProcessExpression("5 + 4 - 1 * 2 / 2"));
}
public int Resolve(int a, int b, CalculatorProxy calcProxy)
{
    if (Token == "*")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Multiply, a, b);
    else if (Token == "+")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Add, a, b);
    else if (Token == "/")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Divide, a, b);
    else if (Token == "-")
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Substract, a, b);
    return 0;
}

¡Luz verde! ¿Algo por refactorizar? Ciertamente hay dos condicionales repetidas. En la factoría de operadores se pregunta por el token para asignar precedencia al operador y crearlo. En la resolución también se pregunta por el token para invocar al proxy. Si utilizamos polimorfismo podemos eliminar la condicional del método Resolve haciendo que cada operador específico implemente su propia resolución.Esta refactorización de hecho se llama así: reemplazar condicional con polimorfismo:

public abstract class MathOperator
{
    protected int _precedence = 0;
    protected string _token = String.Empty;
    int _index = -1;

    public MathOperator(int precedence)
    {
        _precedence = precedence;
    }

    public int Index
    {
        get { return _index; }
        set { _index = value; }
    }

    public string Token
    {
        get { return _token; }
    }

    public int Precedence
    {
        get { return _precedence; }
    }

    public abstract int Resolve(int a, int b,
                    CalculatorProxy calcProxy);
    }
}

public class MultiplyOperator : MathOperator
{
    public MultiplyOperator()
        :base(2)
    {
        _token = "*";
    }

    public override int Resolve(int a, int b,
                                CalculatorProxy calcProxy)
    {
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Multiply, a, b);
    }
}

public class DivideOperator : MathOperator
{
    public DivideOperator()
        :base(2)
    {
        _token = "/";
    }

    public override int Resolve(int a, int b,
                    CalculatorProxy calcProxy)
    {
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Divide, a, b);
    }
}

public class AddOperator : MathOperator
{
    public AddOperator()
        :base(1)
    {
        _token = "+";
    }

    public override int Resolve(int a, int b,
                                CalculatorProxy calcProxy)
    {
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Add, a, b);
    }
}

public class SubstractOperator : MathOperator
{
    public SubstractOperator()
        :base(1)
    {
        _token = "-";
    }

    public override int Resolve(int a, int b,
                                CalculatorProxy calcProxy)
    {
        return calcProxy.BinaryOperation(
                calcProxy.Calculator.Substract, a, b);
    }
}

public class OperatorFactory
{
    public static MathOperator Create(MathToken token)
    {
        if (token.Token == "*")
            return new MultiplyOperator();
        else if (token.Token == "/")
            return new DivideOperator();
        else if (token.Token == "+")
            return new AddOperator();
        else if (token.Token == "-")
            return new SubstractOperator();

        throw new InvalidOperationException(
            "The given token is not a valid operator");
    }
}

El código queda más claro y la condicional en un único sitio. Parecen muchas líneas pero ha sido una refactorización de tres minutos.

Aunque no hay ningún test que pruebe que el métodoCreatelanza una excepción en caso que el token recibido no sea válido, el compilador me obliga a hacerlo, ya que de lo contrario tendría que devolver un objeto sin sentido. No veo la necesidad de escribir un test para ese caso porque ya tenemos un validador de expresiones y métodos que comprueban si un token es número u operador y ambas cosas están debidamente probadas.

Recordemos que la finalidad de TDD no es alcanzar una cobertura de tests del 100 % sino diseñar acorde a los requisitos mediante los ejemplos.

Repasemos la libreta:

# Aceptación - "(2 + 2)*(3 + 1)", devuelve 16
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "2 + -2", devuelve 0

Pensemos en las operaciones con paréntesis. En cómo resolver el problema. Aumentar la complejidad de la expresión regular que valida las expresiones matemáticas no me parece sostenible. Me da la sensación de que ir por ese camino hará el código más difícil de mantener, demasiado engorroso.

Por otro lado no sabría utilizar expresiones regulares para comprobar que un paréntesis abierto casa con uno cerrado y cosas así. Si nos fijamos bien, el contenido de un paréntesis ha de ser una expresión de las que ya sabemos validar y resolver. Una expresión a su vez puede verse como una lista de tokens que en última instancia contiene un solo elemento que es un número.

Vamos a partir el test de aceptación en unos cuantos tests de granularidad más fina para ir abordando poco a poco la implementación.

# Aceptación - "(2 + 2)*(3 + 1)", devuelve 16
* "(2 + 2)", se traduce en la expresión "2 + 2"
* "((2) + 2)", se traduce en la expresión "2 + 2"
* "(2 + 2", produce una excepción
# Aceptación - "3 / 2", produce ERROR
# Aceptación - "2 + -2", devuelve 0

El primero de los tests:

[Test]
public void GetExpressionsWith1Parenthesis()
{
    List<string> expressions =
        _lexer.GetExpressions("(2 + 2)");
    Assert.AreEqual(1, expressions.Count);
    Assert.AreEqual("2 + 2", expressions[0]);
}

De momento el test está dentro de ParserTests pero ya estoy pensando moverlo a un nuevo conjunto LexerTests. No voy a devolver un resultado fijo directamente sino a dar los primeros pasos en el algoritmo que encuentra expresiones dentro de los paréntesis.

// MathLexer
public List<string> GetExpressions(string expression)
{
    List<string> expressions =
        new List<string>();
    Stack<char>parenthesis = new Stack<char>();
    foreach (char ch in expression)
    {
        if(ch == '(')
        {
            parenthesis.Push(ch);
            expressions.Add(String.Empty);
        }
        else if(ch == ')')
        {
            parenthesis.Pop();
        }
        else
        {
            expressions[expressions.Count -1] +=
                ch.ToString();
        }
    }
    return expressions;
}

Cada vez que se encuentra un paréntesis abierto se crea una nueva expresión. El algoritmo es simple. He utilizado una pila para llevar control de paréntesis abiertos y cerrados en vista de los próximos tests que hay en la libreta, aunque no estoy haciendo uso de ella al final del algoritmo. Eso lo dejaré para un test que lo requiera.

[Test]
public void GetExpressionsWithNestedParenthesis()
{
    List<string> expressions =
        _lexer.GetExpressions("((2) + 2)");
    Assert.AreEqual(1, expressions.Count);
    Assert.AreEqual("2 + 2", expressions[0]);
}

El test falla porque la función está devolviendo dos expresiones, la primera de ellas vacía. Hay que limpiar expresiones vacías:

// MathLexer
public List<string> GetExpressions(string expression)
{
    List<string> expressions =
        new List<string>();
    Stack<char>parenthesis = new Stack<char>();
    foreach (charch in expression)
    {
        if(ch == '(')
        {
            parenthesis.Push(ch);
            expressions.Add(String.Empty);
        }
        else if(ch == ')')
        {
            parenthesis.Pop();
        }
        else
        {
            expressions[expressions.Count -1] +=
            ch.ToString();
        }
    }
    cleanEmptyExpressions(expressions);
    return expressions;
}

private void cleanEmptyExpressions(List<string> expressions)
{
    bool endOfList = false;
    while (!endOfList)
    {
        endOfList =true;
        for (int i = 0; i < expressions.Count; i++)
        {
            if (expressions[i].Length == 0)
            {
                expressions.RemoveAt(i);
                endOfList = false;
                break;
            }
        }
    }
}

Ya tenemos luz verde. Se me acaba de venir a la mente una pregunta. ¿Y si al leer las expresiones se forma una que no empieza por un número?. Ejemplo:

[Test]
public void GetNestedExpressions()
{
    List<string> expressions =
        _lexer.GetExpressions("((2 + 1) + 2)");
    Assert.AreEqual(3, expressions.Count);
    foreach (string exp in expressions)
        if((exp != "2 + 1") &&
            (exp != "+") &&
            (exp != "2")) {
            Assert.Fail("Wrong expression split");
        }
    }
}

El test expresa mi decisión de evitar devolver expresiones del tipo +1 prefiriendo los tokens sueltos, a las expresiones que no tienen sentido matemático por sí mismas. He tenido cuidado de no especificar en las afirmaciones las posiciones de las expresiones dentro del vector de expresiones para no escribir un test frágil. Lo que me interesa es el contenido de las cadenas y no la posición.

// MathLexer
public List<string> GetExpressions(string expression)
{
    List<string> expressions = new List<string>();
    Stack<int>parenthesis = new Stack<int>();
    int index = 0;
    foreach (charch in expression)
    {
        if(ch == '(')
        {
            parenthesis.Push(index);
            index++;
            expressions.Add(String.Empty);
        }
        else if(ch == ')')
        {
            index = parenthesis.Pop();
        }
        else
        {
            expressions[index -1] += ch.ToString();
        }
    }
    cleanEmptyExpressions(expressions);
    splitExpressionsStartingWithOperator(expressions);
    return expressions;
}

private void splitExpressionsStartingWithOperator(
    List<string> expressions)
{
    Regex regex = new Regex(@"^(\s*)[+|\-|/|*](\s+)");
    bool endOfList = false;
    while (!endOfList)
    {
        endOfList =true;
        for (int i = 0; i < expressions.Count; i++) {
            if (regex.IsMatch(expressions[i]))
            {
                string exp = expressions[i];
                exp = exp.Trim();
                string[] nexExps =
                    exp.Split(new char[] { ', '\t' },
                        2, StringSplitOptions.RemoveEmptyEntries);
                    expressions[i] = nexExps[0];
                    expressions.Insert(i + 1, nexExps[1]);
                    endOfList = false;
            }
        }
    }
}

La nueva función busca expresiones que empiecen por un operador y entonces las parte en dos; por un lado el operador y por otro el resto de la expresión. Por ahora no hace nada más.

El código está empezando a ser una maraña. Al escribir esta función me doy cuenta de que probablemente quiera escribir unos cuantos tests unitarios para cubrir otros usos de la misma pero el método es privado y eso limita la granularidad de los tests ya que no tengo acceso directo. Tener acceso desde el test a la función que se quiere probar sin pasar por otras funciones o métodos de entrada acelera la detección y corrección de defectos.

Recordemos que a veces los métodos privados sugieren ser movidos a clases colaboradoras. Vamos a hacer un poco de limpieza:

Anterior 8.6. En resumen
Siguiente Capítulo 10. Fin del proyecto - Test de Integración
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.