Refactoring Legacy Code In Practice – Iteration 3

After building the Golden Master in the previous iteration we are sure that we wont introduce any bugs during our refactoring phase: we built a safety net that will prevent us form falling into the regression trap.

The next refactoring I’m going to practice is Subclass to test followed by replace inheritance with delegation. As you can see here subclass to test is a powerful technique to break dependencies and test only the method we are interested in refactoring , but it’s also considered by some an anti-pattern because the complexity of the code becomes higher and higher making it difficult to apply refactoring techniques later on.

Testing the questions’ initialization

In this exercise I decided to refactor the askQuestion method of the Game class.

The first step in building the test suite is to create a class that inherits from the Game class in order to be able to reach it’s internals without actually making them public.

private class GameForTests : Game
{
}

To test the askQuestions I can tests that it pops the correct questions from the correct category and to make my life a little easier I decided to spy on the values contained by the questions’ linked lists, so the next step was to promote the linked lists to protected and expose them in the test class.

private class GameForTests : Game
{
    public LinkedList<string> PopQuestions { get { return popQuestions; } }
    public LinkedList<string> RockQuestions { get { return rockQuestions; } }
    public LinkedList<string> ScienceQuestions { get { return scienceQuestions; } }
    public LinkedList<string> SportsQuestions { get { return sportsQuestions; } }
}
public class Game
{
// ...
    protected LinkedList<string> popQuestions = new LinkedList<string>();
    protected LinkedList<string> scienceQuestions = new LinkedList<string>();
    protected LinkedList<string> sportsQuestions = new LinkedList<string>();
    protected LinkedList<string> rockQuestions = new LinkedList<string>();
// ...

And then my first test checks that the questions get correctly initialized in the constructor

[Test]

public void BoardInizializzedCorrectly()
{
    var game = new GameForTests();
    Assert.AreEqual(50, game.PopQuestions.Count);
    Assert.AreEqual(50, game.RockQuestions.Count);
    Assert.AreEqual(50, game.ScienceQuestions.Count);
    Assert.AreEqual(50, game.SportsQuestions.Count);

    for (int i = 0; i < 50; i++)
    {
        Assert.AreEqual("Pop Question " + i, game.PopQuestions.First());
        Assert.AreEqual("Rock Question " + i, game.RockQuestions.First());
        Assert.AreEqual("Science Question " + i, game.ScienceQuestions.First());
        Assert.AreEqual("Sports Question " + i, game.SportsQuestions.First());
        game.PopQuestions.RemoveFirst();
        game.RockQuestions.RemoveFirst();
        game.ScienceQuestions.RemoveFirst();
        game.SportsQuestions.RemoveFirst();
    }
}

Why should I care?

To test the askQuestion method it would have been enough to stub out the lists instead of checking that they get correctly initialized.  I’m doing this extra work because I’m aiming at extracting a class that represents the deck of cards and testing that the lists get initialized correctly will help me testing that the deck class gets correctly initialized too.

Second step: test askQuestion’s behaviour

Looking closer at the method askQuestion I notice that it have  one indirect input in the form of a method call to currentCategory, one indirect output as it writes one question to the console and some side effects when it pops the questions from the linked lists.

protected void askQuestion()
{
    if (currentCategory() == "Pop")
    {
        Console.WriteLine(popQuestions.First());
        popQuestions.RemoveFirst();
    }
    if (currentCategory() == "Science")
    {
        Console.WriteLine(scienceQuestions.First());
        scienceQuestions.RemoveFirst();
    }
    if (currentCategory() == "Sports")
    {
        Console.WriteLine(sportsQuestions.First());
        sportsQuestions.RemoveFirst();
    }
    if (currentCategory() == "Rock")
    {
        Console.WriteLine(rockQuestions.First());
        rockQuestions.RemoveFirst();
    }
}

My goal with the test is to check that for any possible indirect input, the indirect output and the side effect are what I expect them to be: for example if currentCategory returns “Rock”, then the method should write the next rockQuestion to  the console and then pop it out of the list.

To do so I want to control the values returned by currentCategory so, first of all I add a public property to GameForTests that I can set during my tests with the value I want currentCategory to return, then I promote the later to protected virtual in order to override it and make it return the value of the new field:

protected override string currentCategory()
{
    return CategoryStub;
}

public string CategoryStub { get; set; }

Then I build a test that asks one question of each category checking that the question gets popped from the linked list and that it’s sent to the console

[Test]

public void DeckReturnsQuestionForCategory()
{
    // change the output stream of console to test
    // the indirect output
    var stream = new MemoryStream();
    var writer = new StreamWriter(stream);
    var reader = new StreamReader(stream);
    Console.SetOut(writer);

    // Initialize the game
    var game = new GameForTests();

    // stub the category and ask the question
    game.CategoryStub = "Rock";
    game.AskQuestion();
    
    // check that the first question was popped out of the list
    Assert.AreEqual(49, game.RockQuestions.Count);
    Assert.IsFalse(game.RockQuestions.Any(x => x.Equals("Rock Question 0")));

    Assert.AreEqual(50, game.PopQuestions.Count);    
    Assert.AreEqual(50, game.ScienceQuestions.Count);
    Assert.AreEqual(50, game.SportsQuestions.Count);

// same thing for other categories
           
    writer.Flush();
    stream.Position = 0;
    // Check that the indirect output is correct
    Assert.AreEqual("Rock Question 0", reader.ReadLine());


    stream.Close();
}

Preparing to extract the class

Now that we have the method tested through “subclass to test”, we want to extract the tested code as a dependency in order to separate the miss-placed behavior. the next move then is to prepare the code to extract a class clearly isolating the parts that could go in the extracted class from those that need to remain.

I start by moving the `Console.WriteLine` calls out of the condition blocks so that the askQuestion first evaluate a temporary string that then gets passed to the indirect output.

protected void askQuestion()
{
    string question = string.Empty;
    if (currentCategory() == "Pop")
    {
        question = popQuestions.First();
        popQuestions.RemoveFirst();
    }
    if (currentCategory() == "Science")
    {
        question = scienceQuestions.First();
        scienceQuestions.RemoveFirst();
    }
    if (currentCategory() == "Sports")
    {
        question = sportsQuestions.First();
        sportsQuestions.RemoveFirst();
    }
    if (currentCategory() == "Rock")
    {
        question = rockQuestions.First();
        rockQuestions.RemoveFirst();
    }
    Console.WriteLine(question);
}

Then I change the questions fields to readonly properties that expose a private fields;

protected LinkedList<string> popQuestions { get { return _popQuestions; } }
protected LinkedList<string> scienceQuestions { get { return _scienceQuestions; } }
protected LinkedList<string> sportsQuestions { get { return _sportsQuestions; } }
protected LinkedList<string> rockQuestions { get { return _rockQuestions; } }

private readonly LinkedList<string> _popQuestions = new LinkedList<string>();
private readonly LinkedList<string> _scienceQuestions = new LinkedList<string>();
private readonly LinkedList<string> _sportsQuestions = new LinkedList<string>();
private readonly LinkedList<string> _rockQuestions = new LinkedList<string>();

And the body of the constructor becomes a call to a method that initializes the deck.

All my tests still pass so I’m good to go;

Extract the Parent class

I then change the Game class so it inherits form a parent class called, you guessed it, GameParent. The purpose of this move is to move all the logic for asking a question out of the game class so it will depend only on a call to a parent method. I still don’t need to change my test.

public class GameParent
{
    protected LinkedList<string> popQuestions { get { return _popQuestions; } }
    protected LinkedList<string> scienceQuestions { get { return _scienceQuestions; } }
    protected LinkedList<string> sportsQuestions { get { return _sportsQuestions; } }
    protected LinkedList<string> rockQuestions { get { return _rockQuestions; } }

    private readonly LinkedList<string> _popQuestions = new LinkedList<string>();
    private readonly LinkedList<string> _scienceQuestions = new LinkedList<string>();
    private readonly LinkedList<string> _sportsQuestions = new LinkedList<string>();
    private readonly LinkedList<string> _rockQuestions = new LinkedList<string>();

    public GameParent()
    {
        InitDeck();
    }

    private void InitDeck()
    {
        for (int i = 0; i < 50; i++)
        {
            _popQuestions.AddLast("Pop Question " + i);
            _scienceQuestions.AddLast(("Science Question " + i));
            _sportsQuestions.AddLast(("Sports Question " + i));
            _rockQuestions.AddLast(createRockQuestion(i));
        }
    }
    public String createRockQuestion(int index)
    {
        return "Rock Question " + index;
    }
    protected string Question(string category)
    {
        string question = string.Empty;
        if (category == "Pop")
        {
            question = popQuestions.First();
            popQuestions.RemoveFirst();
        }
        if (category == "Science")
        {
            question = scienceQuestions.First();
            scienceQuestions.RemoveFirst();
        }
        if (category == "Sports")
        {
            question = sportsQuestions.First();
            sportsQuestions.RemoveFirst();
        }
        if (category == "Rock")
        {
            question = rockQuestions.First();
            rockQuestions.RemoveFirst();
        }
        return question;
    }

}

Yon may notice that it has a protected method called Question that gets a category and returns the question to be asked: This method doesn’t have any indirect inputs nor it has indirect outputs, those remain in the askQuestion method which is now super simple: it retrieves the category, then gets the question form the parent and then sends it to the console.

protected void askQuestion()
{
    var category = currentCategory();
    var question = Question(category);
    Console.WriteLine(question);
}

Extract the dependency

Now that the code for building a deck and retrieving questions form it is isolated inside the parent class we can apply the Replace inheritance with delegation refactoring to break the parent-child dependency.

Making this change requires to first change some tests in order to have them pointing directly to the parent class. This isn’t a breaking change but the Game class looses it’s coverage so I’m also forced to write a new test to be sure that the behavior of the askQuestion method doesn’t change.

I first rename the parent from GameParent to CardDeck and build a child class to test:

private class CardDeckForTests : CardDeck
{
    public LinkedList<string> PopQuestions { get { return popQuestions; } }
    public LinkedList<string> RockQuestions { get { return rockQuestions; } }
    public LinkedList<string> ScienceQuestions { get { return scienceQuestions; } }
    public LinkedList<string> SportsQuestions { get { return sportsQuestions; } }


    public string AskQuestion(string category)
    {
        return Question(category);
    }

}

then change the test methods to test the new CardDeckForTests.

[Test]

public void DeckInizializzedCorrectly()
{
    var deck = new CardDeckForTests();
    Assert.AreEqual(50, deck.PopQuestions.Count);
    Assert.AreEqual(50, deck.RockQuestions.Count);
    Assert.AreEqual(50, deck.ScienceQuestions.Count);
    Assert.AreEqual(50, deck.SportsQuestions.Count);

    for (int i = 0; i < 50; i++)
    {
        Assert.AreEqual("Pop Question " + i, deck.PopQuestions.First());
        Assert.AreEqual("Rock Question " + i, deck.RockQuestions.First());
        Assert.AreEqual("Science Question " + i, deck.ScienceQuestions.First());
        Assert.AreEqual("Sports Question " + i, deck.SportsQuestions.First());
        deck.PopQuestions.RemoveFirst();
        deck.RockQuestions.RemoveFirst();
        deck.ScienceQuestions.RemoveFirst();
        deck.SportsQuestions.RemoveFirst();
    }
}


[Test]
public void DeckReturnsQuestionForCategory()
{
    var deck = new CardDeckForTests();
            
    Assert.AreEqual("Rock Question 0", deck.AskQuestion("Rock"));
    Assert.AreEqual(50, deck.PopQuestions.Count);
    Assert.AreEqual(49, deck.RockQuestions.Count);
    Assert.AreEqual(50, deck.ScienceQuestions.Count);
    Assert.AreEqual(50, deck.SportsQuestions.Count);

    // Other Categories
}

As you can see this test doesn’t need to redefine the Console’s output stream as the Question method returns a string and doesn’t have indirect outputs. The new test that I have to write checks that, for any given category, the askQuestion in the Game class sends the question to the console like before:

[Test]
public void GameAsksQuestionForCategory()
{
    var stream = new MemoryStream();
    var writer = new StreamWriter(stream);
    var reader = new StreamReader(stream);
    Console.SetOut(writer);

    var game = new GameForTests();
    game.CategoryStub = "Rock";
    game.AskQuestion();
    game.CategoryStub = "Pop";
    game.AskQuestion();
    game.CategoryStub = "Science";
    game.AskQuestion();
    game.CategoryStub = "Sports";
    game.AskQuestion();

    writer.Flush();
    stream.Position = 0;
    Assert.AreEqual("Rock Question 0", reader.ReadLine());
    Assert.AreEqual("Pop Question 0", reader.ReadLine());
    Assert.AreEqual("Science Question 0", reader.ReadLine());
    Assert.AreEqual("Sports Question 0", reader.ReadLine());

    stream.Close();
}

now that I’m happy with the status of my tests I’m ready to break the inheritance. Now the game class doesn’t inherits from anyone and it has a CardDeck as a private field that gets assigned in the constructor and the ask Question simply calls the Question method of the CardDeck field and it has no clue of how the questions are stored internally in the CardDeck class.

public class Game
{
    private readonly CardDeck _deck;
    public Game(CardDeck deck)
    {
        _deck = deck;
    }
    // ...
    protected void askQuestion()
    {
        var category = currentCategory();
        var question = _deck.Question(category);
        Console.WriteLine(question);
    }
    // ...
}1

Conclusion

Even if a little bit longer than just asking to ReSharper to extract a class, proceeding with this refactoring gave me the opportunity to make every step in a save and clean way without suddenly breaking the test and having to fix them later. The only time the test got broken was when I needed to change the structure of the code so heavily that I decided to change the test to reflect the behavior that I’m expecting from the change.

What struck me was that at every change it was crystal clear to me what was the next step to be taken in order to improve the code. This doesn’t happen a lot when refactoring code at work when you have less time and have to take shortcuts.

If you liked this article and want more Subscribe here and you wont miss any article.

Refactoring Legacy Code In Practice – Iteration 1

If you have been comdamned to work on old, ugly, unmantained and untested code, then you probably feel the urge from time to time to clean up a little bit and make your life easier. Let’s face it, cleaning up some mess in the code is always fun, until it’s not: until you have to work in the night to fix some bug you introduced in a core feature just because you wanted to clean up the code.

You know there’s a huge risk in every change you make, but you want to work with better code, so you have two options: you either clean it or you leave your job! Leaving a job is slow and painful and you have no guaranties that you are going to land a job which offers better code to work on, so your only choice is to clean up.

But, where to start? How can you minimize the risks? well, we have written a lot about how to handle legacy code (have a look at our Blog series page) and there are some great books that you can read (go to our Resources page). But what if you want to practice and experiment in a safe environment without the risk of breaking your production code? With some luck you can find some User Group near to you that hosts a Legacy Code Retreat but if you are nowhere near a developers meetup like I am you can always find some ugly source code and try to practice by yourself the same excercices they do in the code retreats. And what better code can you use for this purpose than  one that was specifically designed to be legacy: just go here, download it and start refactoring.

That’s exactly what I did and I wanted to share the process with you in this series of post so you can compare your approach to that of a fellow programmer.

First iteration

Like every legacy project that’s worth its name, there is not enough information about the code you are going to touch. So the first iteration is about discovering how bad its state is and what’t the whole purpose of the code. If you had time you could try to apply some refactoring techniques trying to minimize the risk of adding bugs. After 45 minutes you had to stop and delete everything you have done so far.

After some analysis I discovered that this game is basically a simplified implementation of the Trivial Pursuit. It’s formed by two classes the Game class that has all the business logic of the game and the GameRunner that it’s basically the main entry point of the program. For some obscure reason the game loop is located in the GameRunner.

After starting the game, the game runner adds three players to the game and then starts the game loop that can be described by these steps:

  1. a player rolls a standard 6 faces dice
  2. based on the result and the player’s current status the player moves or remains in the same spot
  3. A question is asked to the player which responds to the question with 1/9 percent change of getting it wrong
  4. the game continues until there is a winner: the first one to reach 6 points.

I still haven’t been able to understand how does the player gains it’s points.

The code

Exploring the code gives you headache: you cannot understand if it was so badly crafted written for the purpose of the exercises or if JB asked a fifteen years old kid who is trying to learn to program to write it. I have tried to build examples of bad code and was never able to create something so beautifully ugly!

By the way let’s have a look at the code:

The game runner starts instantiating a Game object and then it calls and add method with something that looks like player names.

Game aGame = new Game();

aGame.add("Chet");
aGame.add("Pat");
aGame.add("Sue");

The method add in fact adds the player name to a list of strings called players then, it looks like it places the player in the first box with zero points, then it returns true (why returning true? what does it means?).

public bool add(String playerName)
{
    players.Add(playerName);
    places[howManyPlayers()] = 0;
    purses[howManyPlayers()] = 0;
    inPenaltyBox[howManyPlayers()] = false;

    Console.WriteLine(playerName + " was added");
    Console.WriteLine("They are player number " + players.Count);
    return true;
}

Back to the GameRunner class. In the game loop a random number between 1 and 6 is picked and then this number is passed to the roll method. Here there is some duplication and some business logic difficult understand, for example: if inPenaltyBox for the current player is true and the random number (roll) is not even (why on earth check that something is not even? wasn’t it the same checking that it was odd?) a variable called isGettingOutOfPenaltyBox becomes true.

if (inPenaltyBox[currentPlayer])
{
    if (roll % 2 != 0)
    {
        isGettingOutOfPenaltyBox = true;

        Console.WriteLine(players[currentPlayer] + " is getting out of the penalty box");
        places[currentPlayer] = places[currentPlayer] + roll;
        if (places[currentPlayer] > 11) places[currentPlayer] = places[currentPlayer] - 12;

        Console.WriteLine(players[currentPlayer]
                + "'s new location is "
                + places[currentPlayer]);
        Console.WriteLine("The category is " + currentCategory());
        askQuestion();
    }
    else
    {
        Console.WriteLine(players[currentPlayer] + " is not getting out of the penalty box");
        isGettingOutOfPenaltyBox = false;
    }

The askQuestion method checks which category is the player in and pops a question from the question from the queue (smells like bug… what happens if you run out of questions of a certain category? a you get a nice little exception!).

if (currentCategory() == "Pop")
{
    Console.WriteLine(popQuestions.First());
    popQuestions.RemoveFirst();
}

The currentCategory method looks like a really malformed array of strings, but you cannot be sure until you know know how many places there are in the game and which one are actually of “Rock” category, are there some bonus places not considered in this method?

private String currentCategory()
{
    if (places[currentPlayer] == 0) return "Pop";
    if (places[currentPlayer] == 4) return "Pop";
    if (places[currentPlayer] == 8) return "Pop";
    if (places[currentPlayer] == 1) return "Science";
    if (places[currentPlayer] == 5) return "Science";
    if (places[currentPlayer] == 9) return "Science";
    if (places[currentPlayer] == 2) return "Sports";
    if (places[currentPlayer] == 6) return "Sports";
    if (places[currentPlayer] == 10) return "Sports";
    return "Rock";
}

 Refactoring

The first thing I would like to address is the players management, there should be a Player class that handles most of the actions on the player, like it’s points or if it’s in a penalty round. Another improvement could be to isolate is the Board itself in a class that knows places and categories for each one. If you then extract a set of classes that represent the deck of cards and move the game loop inside the game class.

The problem is that these entities are spread across multiple methods and extracting requires changing a lot of places, so it would be better to first have a comprehensive suite of automated tests.

Simple things first

There’s not enough time left so I look for the simplest method to start doing some real refactoring.  I don’t really like currentCategory, there’s a lot of  duplicated code, and a ton of ifs.

private String currentCategory()
{
    if (places[currentPlayer] == 0) return "Pop";
    if (places[currentPlayer] == 4) return "Pop";
    if (places[currentPlayer] == 8) return "Pop";
    if (places[currentPlayer] == 1) return "Science";
    if (places[currentPlayer] == 5) return "Science";
    if (places[currentPlayer] == 9) return "Science";
    if (places[currentPlayer] == 2) return "Sports";
    if (places[currentPlayer] == 6) return "Sports";
    if (places[currentPlayer] == 10) return "Sports";
    return "Rock";
}

The first thing to do is to build the test fixture and to do so I apply subclass to test and promote the method from “private” to “protected virtual” to be able to access it from a child class.

protected string CurrentCategory()

Then I can subclass the Game class and expose the protected method with a public one so I can call it in my tests and the I can start building the test suite.

private class GameForTest : UglyTrivia.Game
{
    public string PublicCurrentCategory()
    {
        return CurrentCategory();
    }

    public int Place
    {
        set { Places[0] = value; }
    }
}

Using subclass to test in often considered an anti-pattern for various reasons one been that with it you test the internal of a class instead of shifting to a higher level of abstraction but in this context is permitted because we just want to preserve the functionality while changing the code and we don’t care if we have to get rid of the test later on.

Building the tests

After having exposed CurrentCategory as a public method we can stub also the places array in order to have fine control over which place a given player is in (in our case player 0). In our case this gives us some advantages like not having roll the dice to move the player arround, just set it’s position do the desired value:

[TestCase(0)]
[TestCase(4)]
[TestCase(8)]
public void CategoryPop(int place)
{
    // we need to set differnt values for places[currentPlayer]
    var game = new GameForTest {Place = place};

    var currentCategory = game.PublicCurrentCategory();
    // I expect it to be

    Assert.AreEqual("Pop", currentCategory);
}

[TestCase(1)]
[TestCase(5)]
[TestCase(9)]
public void CategoryScience(int place)
{
    var game = new GameForTest {Place = place};
    var currentCategory = game.PublicCurrentCategory();
    Assert.AreEqual("Science", currentCategory);
}

[TestCase(2)]
[TestCase(6)]
[TestCase(10)]
public void CategorySports(int place)
{
    var game = new GameForTest {Place = place};
    var currentCategory = game.PublicCurrentCategory();
    Assert.AreEqual("Sports", currentCategory);
}

[TestCase(3)]
[TestCase(7)]
[TestCase(11)]
public void CategoryRock(int place)
{
    var game = new GameForTest {Place = place};
    var currentCategory = game.PublicCurrentCategory();
    Assert.AreEqual("Rock", currentCategory);
}

I wrote four simple parametrised tests, one for each category, and the test’s parameter is the place of which I want to check it’s category. As you can see the tests assert that there are 12 positions and that positions 3, 7 and 11 are Rock category: this is an assumption based on what I’ve learned inspecting the code.

Ok, now that the method is covered I’m free to try different things. First of all I substituted the ifs with a much simpler switch statement, but the conditionals are still there. The tests pass. Then I replaced the switch with an array of strings and promoted it to become a private field of the Game class. The tests still pass. With one more step I convert the array of strings in an array of enum because I don’t like magic numbers; The method still have to return a string because if i change the return I’ll have to make changes in a lot of different places in the code. Again, the tests still pass.

private readonly Category[] _categories = {
        Category.Pop,
        Category.Science,
        Category.Sports,
        Category.Rock,
        Category.Pop,
        Category.Science,
        Category.Sports,
        Category.Rock,
        Category.Pop,
        Category.Science,
        Category.Sports,
        Category.Rock
    };

protected string CurrentCategory()
{
    var place = Places[_currentPlayer];
    return _categories[place].ToString();
}

End of the iteration

I ran out of time after this refactoring, but there is no doubt that my understanding of the code has increased a lot and that the little method I chose to refactor is a lot easier to understand now that there are no conditionals.

If you want to practice your refactoring muscles download the project at JB’s repository, or you can download it form mine where I divided every iteration in a different branch so you can explore what I did.

In the next iteration I’m going characterize the behavior of the Game class building a Golden Master, subscribe if you don’t want to miss it!

How To Test Your Production Code: Using Fake Objects

In the last post we talked about Mock Objects and what are the difference between them and Spies. We have almost completed our journey through the all the different types of Test Doubles, in this post we are going to introduce the last one: the Fake Object. 

There are sometimes where the need of isolation comes from the fact that your tests are running too slow because you are accessing services that need to read or write to the disk or make a network call. In this cases, the best way to improve the test is to use a Test Double that only mimics the same functionality as the service you are currently using. Of course, you need a way to swap the objects during the test, but the techniques to be sure that you can inject your Test Doubles are going to be discussed in a future post.

Let’s make an example

Let’s pretend that you are developing a method that prepares different kind of SMS messages and then sends them using a service that returns an id for the message so you can query it back and check the processing status of the message. The service also stores all sent messages in a queue for later use and the queue has to be manually emptied.

class SmsManager
{
  public IMessageService SMSService { get; set; }
  public void SendSMS()
  {
    string message = PrepareSMS();
    Guid key = SMSService.Send(message);

    while (!SMSService.Sent(key))
    {
      Thread.Sleep(100);
    }
    SMSService.Delete(key);
  }

  private string PrepareSMS()
  {
    return String.Empty;
  }
}

Maybe we can configure the service to avoid sending the SMS message while we are in test, but we still have to wait for the whole round trip to happen, causing a sensible delay in getting the results from the test, and if we have to run lot’s of tests  those delays will start to add up. After a while, you end up with tests that take you minutes to run and you stop running them often.

Uncle Bob always talks about how he and his team developed FitNesse without needing a database. But how do they tested the functionalities that dealt with persistent data? well, they isolated the persistence layer and they faked it with an in-memory representation of it.

This is what Fake Objects are for: you swap slow external dependencies of your SUT with the Fake Object and you use them in your test as if they were real.

Can It Be THAT Simple?

Well, it depends. The problem is that Fake Object must have the same functionality as the real one, at least all the functionality the SUT uses, but implemented in a simpler way; for example, our fake SMS service could be implemented like this:


public interface IMessageService
{
  Guid Send(string message);
  bool Sent(Guid key);
  void Delete(Guid key);
}

public class FakeSmsService : IMessageService
{
  Dictionary<Guid, string> inmemorydb = new Dictionary<Guid, string>();
  public Guid Send(string message)
  {
    var guid = new Guid();
    inmemorydb.Add(guid, message);
    return guid;
  }
  public bool Sent(Guid key)
  {
    return inmemorydb.ContainsKey(key);
  }
  public void Delete(Guid key)
  {
    inmemorydb.Remove(key);
  }

}

Internally the fake service retains a dictionary of key-value pairs where it saves all the messages that the method Send receives. Even though it’s pretty simple, this implementation is powerful because we can trust that the SUT will believe that it’s using the SmsService that makes the HTTP calls and all that, but in fact, all the operations stay in the memory of the running process, so it’s waaay faster.

Could you give an example of a Fake Object recently used?

In one recent project we used Effort to isolate us from the database.

We were using Entity Framework code first to create the database and then access it, and we discovered this Effort that was an Entity Framework Provider: it implemented the same interfaces and functionality of EF but it did it in an in-memory database so we didn’t have to worry about cleaning the DB in the TearDown phase of our test, we simply destroyed it and created it again, and it was super-fast.

What to do next?

Find a simple functionality on the code you are maintaining that uses an external service and start implementing a fake object based on that functionality; then use it in a test suite to cover your method.  If the service is huge and with lots of functionality, don’t get overwhelmed, start small, just a simple implementation that helps you test the method you decided to cover.

 

After reading this series of posts you have no more excuses, now set up your environment and start testing the heck out of your code!

Most importantly help a friend by explaining him the difference between mocks and spies and if he doesn’t understand, send him the link to this post.

How To Test You Production Code: Using Mock Objects

By using Test Spies we can capture all the indirect outputs of our software to check that it is working correctly. Often times, making the assertions for all the outputs in the testing function leads to lot’s of duplicated code that after a while starts to hinder the purpose of the tests. Sometimes, we would like to do our assertions in the context of the running code and not in the context of the test suite.

Think for example that one of the indirect outputs of your SUT is a disposable object. How you verify that it is in the correct state when it get’s injected to your dependency if it gets destroyed as soon as the method you are testing exits?

Let’s try to understand this problem with an example: let’s say we want to verify that the method CreateAndConfigureDisposable passes the correct object to DependencyObject:


public interface IScanner
{
  void SetStream(FileStream fileStream);
}

public class FileLoader
{
  private readonly IScanner _scanner;

  public FileLoader(IScanner scanner)
  {
    _scanner = scanner;
  }
  public void LoadStreamAndPosition(string path)
  {
    using (var s = new FileStream(path, FileMode.Create))
    {
      s.Position = 500;
      _scanner.SetStream(s);
    }
  }
}

We could write a test method using a Test Spy and it will look like this:

public class ScannerSpy : IScanner
{
  public FileStream SpyStream { get; set; }
  public void SetStream(FileStream stream)
  {
    SpyStream = stream;
  }
}
[Test]
public void TestDisposableOutput1()
{
  // Arrange
  var scannerSpy = new ScannerSpy();
  var loader = new FileLoader(scannerSpy);
  // Act
  loader.LoadStreamAndPosition(@"c:\teststream.txt");
  // Assert
  Assert.AreEqual(500, scannerSpy.SpyStream.Position);
}

this test looks correct, we configure the SUT, we pass in the TestSpy, we exercise the method we want to test and the we do the assertions. The problem is that as soon as we exit the using block inside LoadStreamAndPosition the disposable object gets, well, disposed and trying to access its state will cause an ObjectDisposedException.

The best way to test this code is to do the assertions in the context of the running code that we want to test. This is where Mock Objects come into play: you put all the assertions inside the Test Double’s method that receives the indirect output.

public class ScannerMock : IScanner
{
  public long Position { get; set; }
  public void SetStream(FileStream stream)
  {
    Assert.AreEqual(Position, stream.Position);
  }
}

With this new Test Double the assertions run inside the using block while the DisposableObject still exists. By introducing the Mock Object the test logic flow changes quite a bit, the assertion step is missing and is substituted by a step for configuring the Test Double.

[Test]
public void TestDisposableOutput2()
{
    // Arrange
    var scannerSpy = new ScannerMock();
    scannerSpy.Position = 500;
    // Act
    var loader = new FileLoader(scannerSpy);
    loader.LoadStreamAndPosition(pathToTestStream);
}

So… what’s the big difference then between Spy Objects and Mock Objects

With a Mock Object you mock an object and with a Spy you spy on it 😉

Just kidding, I read a similar explanation while researching this topic.

The real explanation is that spies are used to collect information about the behavior of the code and then do all the assertions in the testing context while mocks may collect some information (like how many times a method gets called) but all the assertion are done in the context of the running code.

This distinction is important for a few reasons of which this are the two most important in my opinion:Some indirect outputs may not exists outside of the SUT context;

  • Some indirect outputs may not exists outside of the SUT context;
  • The code you are trying to test may swallow the exceptions generated by the assertions;

The first one is a case when you are forced to use a mock object because you need to do the assertions while the code is still running; we have already discussed this problem so let’s check the second one.

Let’s pretend that

public void LoadStreamAndPosition2(string path)
{
  try
  {
    var s = new FileStream(path, FileMode.CreateNew);
    s.Position = 500;
    _scanner.SetStream(s);
  }
  catch
  {
    //Gotta catch 'em all!
  }
}

In this case, we instantiate a non-disposable object that will live after the method LoadStreamAndPosition exists; We still could use a Mock Object if it wasn’t for the Pokémon Exception Handling that is going to catch the exceptions generated by the testing framework hiding all the failing assertions and making us think that the test run correctly.

How To Test Your Production Code: Test Spy

By using Dummy Objects and Test Stubs, we are able to control the inputs of the System Under Test to exercise every part of it during our tests. But of course, not every method that we want to test is going to have one just output or is going to change only the internals of the class that is under test. Most methods have side effects and are going to call other libraries or methods of other objects and we want to be sure that those method calls happen the right amount of times and with the correct parameters.

Let’s suppose we have a method that prepares an invoice when someone check’s out at a hotel. That method is part of a Registration class that takes in a Room and a Guest object and the BookingInformation with all the information about the booking period, like check-in and check-out dates number of people etc.

public class Registration
{
  private Room _room;
  private Guest _guest;
  private BookingInformation _booking;
  private readonly IPrinter _printer;
  public Registration
          (Room room, Guest guest,
          BookingInformation booking, IPrinter printer)
  {
    _booking = booking;
    _guest = guest;
    _room = room;
    _printer = printer;
  }
  public void CheckOut()
  {
    Dictionary<string, string> invoiceInformation =
        PrepareInvoiceData();
    _printer.Print(invoiceInformation);
  }

  private Dictionary<string, string> PrepareInvoiceData()
  {
    ////
  }

}

 

Then, when the method checkout gets called, it prepares a dictionary of strings and passes it to a library that prints the invoice. This dictionary an indirect output of our method and we want to check if it’s properly set, while also avoiding calling the real method that sends the file to the printer.

We notice that the a printer object is passed in the constructor of the class and that it implements the interface IPrinter. That means we could pass a special implementation of that interface and log the calls and the inputs passed to its methods.

public class PrinterSpy : IPrinter
{
  public int PrintCallCount = 0;
  public Dictionary<string, string> InvoiceParameters;
  public void Print(Dictionary<string, string> parameters)
  {
    InvoiceParameters = parameters;
    PrintCallCount++;
  }
}

In particular we want to capture the dictionary that is passed to the print method and expose it as a public field to do the assertions. We could also check that we are calling the method just once, or that we are not calling it for when an error occurs.

[Test]
public void CheckPrintMethodIsCalledWithTheCorrectConfiguration()
{
  // Arrange
  var guest = new Guest
  {
    Name = "GuestName",
    Surname = "GuestSurname",
    Id = 1,
    Phone = 111111111,
    Address = "SomeAddress",
    City = "SomeCity",
    Country = "SomeCountry"
  };
  var room = new Room
  {
    Number = 1,
    Type = RoomTypes.Suite,
    Price = 1
  };
  var booking = new BookingInformation
  {
    CheckInDate = new DateTime(2016, 1, 1, 14, 0, 0),
    CheckOutDate = new DateTime(2016, 1, 2, 10, 0, 0),
    NumberPpl = 1,
  };

  // Act
  var printer = new PrinterSpy();
  var registration = new Registration(room, guest, booking, printer);
  registration.CheckOut();

  // Assert
  var output = printer.InvoiceParameters;
  Assert.AreEqual(1, printer.PrintCallCount);
  Assert.AreEqual("GuestSurname GuestName",
                    output["GuestName"]);
  Assert.AreEqual("111-111-1111", output["GuestPhone"]);
  Assert.AreEqual("SomeAddress, SomeCity (SomeCountry)",
                  output["GuestAddress"]);
  Assert.AreEqual("01/01/2016", output["CheckInDate"]);
  Assert.AreEqual("02/01/2016", output["CheckOutDate"]);
  Assert.AreEqual("Suite", output["RoomType"]);
  Assert.AreEqual("1 Day", output["NumberOfDays"]);
  Assert.AreEqual("1.00 €", output["TotalCost"]);
}

As you can see, in this test we pass to the Registration constructor the Room, the Guest and the BookingInformation objects properly configured and then we pass also a our Test Double that will get called and will record it’s inputs. After the method CheckOut gets called we have complete access to the information that it prepares and then passes over to the Print method. The SUT (System Under Test) doesn’t know that it’s talking to a test double so if the test passes we are sure that the method will work correctly in production when it will pass the dictionary to the object that will in fact print the invoice.

It’s not always that easy!

Sure, if you are working with some legacy code, chances are that your class doesn’t have the dependency that receives the indirect output nicely injected in the constructor. You could have something like this:

public class Registration2
{
  private Room _room;
  private Guest _guest;
  private BookingInformation _booking;

  public Registration2
    (Room room, Guest guest,
      BookingInformation <span class="hiddenGrammarError" pre=""><span class="hiddenGrammarError" pre="">booking)
  {
    _booking</span></span> = booking;
    _guest = guest;
    _room = room;
  }
  public void CheckOut()
  {
    Dictionary<string, string> invoiceInformation =
      PrepareInvoiceData();
    /////////////////////////////////////////////
    Printer printer =
      new Printer( /* PUT HUGE LIST OF PARAMETERS HERE */);
    /////////////////////////////////////////////
    printer.Print(invoiceInformation);
  }

  private Dictionary<string, string> PrepareInvoiceData()
  {
    ////
    /// 
    return null;
  }

}

How could you test the indirect output now?

Depending on the code base and how confident you are in refactoring it you could try different strategies like extracting the ugly part of your code to a protected method and then extend the class with a special that you use class just for testing. Another way to address this problem is to isolate your code wrapping the external dependency in a class that you can override like this:

internal interface IPrintWarpper
{
  void Print(Dictionary<string, string> invoiceInformation);
}
internal class PrintWrapper : IPrintWarpper
{
  private Printer printer;

  public PrintWrapper()
  {
    Printer printer =
      new Printer( /* PUT HUGE LIST OF PARAMETERS HERE */);
  }
  public void Print(Dictionary<string, string> invoiceInformation)
  {
    printer.Print(invoiceInformation);
  }
}

Now you have isolated the dependency and have an interface that you can implement with your Test Spy

Now it’s your turn

Take a small function that has just one well-defined indirect output extract it and test the behavior YOUR code.

Also, don’t forget to share this post with your friends that have problems testing their code!

How to Test your Production Code: Using Test Stubs

In our last post we explored how to use Dummy Objects in order to test our code when we are dealing with objects that are difficult to create. One important restriction on the usage of this kind of test double is that we have to avoid touching the dummy object during our tests. We can easily check that LoadInvoicesByDate throws when from is higher that threshold:

public string WarningMessage;
public IList<Invoice> Invoices;
public void LoadInvoicesByDate(DateTime from, DateTime to, DateTime threshold, IDbFactory dbFactory)
{
  if (from > threshold)
    throw new DateOverThresholdException("from");

  var count = dbFactory.CountInvoices(from, to);
  if (count == 0)
  {
    WarningMessage = "No Invoices in given rage";
    return;
  }
  Invoices = dbFactory.GetInvoices(from, to);
}

But what happens if we have to pass the first ‘if’ and check that the text of WarningMessage is updated when count equals zero?

In this case the variable count is said to be an indirect input of the code we want to test and in order to control it’s value we need to instantiate a special version of dbFactory that we have under our control. Of course we don’t want to use the real instance of DbFactory because who knows the state of the database… we could never get a count that equals 0. So in our test we are going to need a Test Double called, you guessed it, Test Stub.

The Test Stub will provide you with the indirect input you need no matter what how it gets called. Let’s explain with a test that checks  that the WarningMessage field is set when dbFactory.CountInvoices returns zero:

private class DbFactoryStub : IDbFactory
{
  public int CountInvoices(DateTime from, DateTime to)
  {
    return 0;
  }
  // other IDbFactory methods
}
[Test]
public void WarningMessageWhenCountIsZero()
{
  IDbFactoy dbFactory = new DbFactoryStub();
  invoicesManager.LoadInvoicesByDate(
      DateTime.Now, DateTime.Now, DateTime.Now,
      dbFactory);
  Assert.AreEqual("No Invoices in given rage",
                  invoicesManager.WarningMessage);
}

What’s happening here?

First I create a new class that implements IDbFactory and set the return value of CountInvoices to zero, which is exactly the return value we want to have. With this test double we are forcing our code through a path that could have been difficult to simulate with the real implementation.

Moreover, we can generalize a bit our dummy class and set the wanted return values in the tests

private class DbFactoryDummy : IDbFactory
{
  public int DummyInvoicesCount;
  public IList<Invoice> DummyInvoices;
  public int CountInvoices(DateTime from, DateTime to)
  {
    return DummyInvoicesCount;
  }
  public IList<Invoice> GetInvoices(DateTime from, DateTime to)
  {
    return DummyInvoices;
  }
}

[Test]
public void WarningMessageWhenCountIsZero()
{
  IDbFactoy dbFactory = new DbFactoryDummy();
  dbFactory.DummyInvoicesCount = 0;
  invoicesManager.LoadInvoicesByDate(
      DateTime.Now, DateTime.Now, DateTime.Now,
      dbFactory);
  Assert.AreEqual("No Invoices in given rage",
                  invoicesManager.WarningMessage);
}

[Test]
public void InvoicesSetGetInvoicesReturnedValue()
{
  IDbFactoy dbFactory = new DbFactoryStub();
  dbFactory.DummyInvoicesCount = 1;
  dbFactory.DummyInvoices = new List<Invoice> { new Invoice() };
  invoicesManager.LoadInvoicesByDate(
      DateTime.Now, DateTime.Now, DateTime.Now,
      dbFactory);
  Assert.AreEqual(string.Empty,
                  invoicesManager.WarningMessage);
  Assert.AreEqual(dbFactory.DummyInvoices,
                  invoicesManager.Invoices);
}

With this change to DbFactoryDummy we are able to configure its return values while testing in order to use the same dummy class for different test cases.

Remember that our goal isn’t that of testing the behaviour of the particular implementation of DbFactory but how our code behaves when stimulated with different indirect inputs.

What if we want to test the indirect outputs of the system? for that we use Mock Objects, that will be explained in the next post.

How to Test your Production Code: Using Dummy Objects

Let’s pretend we want to add a method to a class. We, of course, are going implement the method using TDD but we have a problem, to create an instance of the class we need to pass in a super-difficult-to-build object. We are sure that our implementation won’t touch this object in any way, like if it smelled funny, but the class needs an’instance of that object. What can we do?

To understand what can be done let’s look at this test:

[Test]
public void GetInvoiceAsPdf(){
    // some pretty nasty code to intanciate the dbFacade object
    var dbFacade = new DbFacade( /* bleh */);
    // ....
    // A lot of properties that need to be setup for dbFacade to function

    var formatter = new InvoiceFormatter(dbFacade);
    // The test code
}

All the horrible code needed to instantiate a working instance of dbFacade is going to obscure the real purpose of the test and make the tests fragile because if something in the procedure of constructing the object changes, a lot of tests are going to fail.

But, do we really need a working instance of DbFacade? we said earlier that we are not going to use it in any way in our new method, so for what we know we could completely avoid it in our tests and just pass a null to the constructor of InvoiceFormatter .

[Test]
public void GetInvoiceAsPdf(){
    var formatter = new InvoiceFormatter(null);
    // The test code
}

If the constructor of InvoiceFormatter doen’t check for nulls this is a perfectly acceptable strategy and the great thing is that it cleans up the test function. Of course, if we are dealing with a constructor that doens’t accept passing nulls, we have to try to find a different solution.

One solution can be to check if we can instantiate a DbFactory in a simpler way, say with the default constructor and without setting any properties. This won’t result in a working instance of the class but it will be enought for our tests.

[Test]
public void GetInvoiceAsPdf(){
    // using default constructor to build a dummy object
    var dbFacade = new DummyDbFacade();
    var formatter = new InvoiceFormatter(dbFacade);
    // The test code
}

Remember, we don’t need a working DbFacade because we are not going to use it in our tests.

If DbFacade does not expose a default constructor we could inherit from it (if it’s not sealed) and provide one with a dummy class.

private class DummyDbFacade : DbFacade
{
    public DummyDbFacade() {}
}
[Test]
public void GetInvoiceAsPdf(){
    // using default constructor to build a dummy object
    var dbFacade = new DummyDbFacade();
    var formatter = new InvoiceFormatter(dbFacade);
    // The test code
}

Or we could implement the same interface as DbFacade if it implements an interface and InvoiceFormatter requires the same one and not a concrete class.

private class DummyDbFacade : IDbFacade
{
    public DummyDbFacade() {}
// empty methods if IDbFacade
}
[Test]
public void GetInvoiceAsPdf(){
    // using default constructor to build a dummy object
    var dbFacade = new DummyDbFacade();
    var formatter = new InvoiceFormatter(dbFacade);
    // The test code
}

Another use of a dummy object is when we are covering a method already implemented and we only need to provide the values for the path that we want to test. For example the method LoadInvoicesByDate receives an IDbFactory

public void LoadInvoicesByDate (DateTime from, DateTime to, DateTime threshold, IDbFactory dbFactory){
    if(from > threshold)
        throw new DateOverThresholdException("from");

    // load all invoices in the timespan given using dbFactory
}

If we are going to test that a DateOverThresholdException is thrown when from is higher than threshold we are free to pass a null as an argument for dbFactory.

To summarize, Dummies are used to put the code that you are going to test in a testable condition whenever you don’t need to read any information form them or call their functions. For that you have stubs, spies, mocks and fakes…

How to Test your Production Code: Everything You Need To Know About Test Doubles

One common objection that I get while explaining TDD to my colleagues is that they deal with objects that are too complex to be tested in an automated suite. They either have methods that are too complex (too many interactions with other libraries or classes) or objects too difficult to instantiate (constructors that call to databases or that need to set up a huge graph of dependent objects).

Sometimes the objection is that the program uses some method that reaches to a database or an external service to read (or worse to persist) some information. This normally happens when the developer is working with some legacy code that doesn’t have any real separation between what is the business logic of the software product and what are the external resources that it uses (databases, web services, etc).

Of course, writing automated tests to verify code that calls to a database to save some information can be painful for several reasons:

  • Databases are slow: the test would run slow and you will not be willing to run them often;
  • After every test you have to clean the database: this is important because tests should be atomic a reproducible, if the database contains old data, the tests could produce false results. Of course this result in even slower tests;
  • Your software may be using a database (or service) shared between different applications or different developers: if you are working with a shared database your tests may not  be reproducible, you cannot clean the records because you could be deleting other people data. Sharing a database between developers makes your code untestable without some sort of separation from it;

To be able to test your code you have to be isolated from the parts that are difficult to test. By using other objects with the same interface as the one that we want to avoid using.

We have all seen videos of the crash test done on cars to certify their safety. Obviously it wouldn’t be acceptable to do the crash tests with real humans (luckily the time when living humans were used to test katanas is long gone) so they use a dummy that is loaded with sensors to measure the effects that the crash would have on real people.

The software counterpart of these dummies are called Test Doubles and are used in the same fashion: during the tests they are put in those places where it wouldn’t be acceptable or easy to use real production objects. We use Test Doubles to put the software in the condition that we want to test.

Let’s try to understand this better with an example:

public bool Contains(string searchString)
{
    try
    {
        // networkHandler is an instance of an object
        // that makes request to a uri
        List<string> fromNetwork = networkHandler.GetAsList(uri);
        // some interesting code
        return true;
    }
    catch(WebException e)
    {
        return false;
    }
}

The method Contains uses an instance of the class NetworkHandler to load a list of strings from somewhere; this class is defined like this:

public class NetworkHandler
{
     public virtual List<string> GetAsList(string uri)
     {
          // some code that connects to the uri passed as parameter
          // and then parses the result and returns a List of strings
     }
}

Using the real NetworkHandler there’s no easy way to test that our method returns false when a WebException gets thrown by the method GetAsList. The best way to test this behaviour is to replace in our tests the instance of NetworkHandler with a test double which only purpose is to throw a WebException when the method GetAsList gets called.

public class NetworkHandlerStubThrowNetWorkException : NetworkHandler
{
     public override List<string> GetAsList(string uri)
     {
          throw new NetworkException();
     }
}

Types of Test Doubles

The Test Doubles are divided into different categories depending on their usage in the testing suite. Returning to our dummy analogy, a mannequin is a different Double than a fully featured Crash Dummy, both have the resemblance of a human but they are used in totally different scenarios. In the software world we have:

Each of which has it own purpose in helping us isolating the code that we want to test from the one that not; In later posts we will explain each type more in-depth, giving  also some usage examples.

The Importance of TDD: So, what’s TDD?

creek-1149058_1280

Remember when in the first post of the series I said that we should write the requirements as tests before writing the code that implements it? but it’s important?

As you may have noticed, during the implementation of the requirements in the last post I always wrote part of the requirement before writing the implementation to make it pass, and then refactored the code a little bit by trying to generalize the algorithm and make it clearer to the reader. I was able to change the code and improve it because I Knew there was no way I could break something because the tests would warn me otherwise. Then, after I was satisfied with the refactoring I started again with another piece of requirement.

To put it in programming terms I was more or less following this algorithm:

public bool IsImplementationComplete(List requirements)
{
    if(!requirements.Any())
        return true;

    Requirement current = requirements.First();
    requirements.Remove(current);
    TDDStep(current);
    return IsImplementationComplete(requirements);
}
public void TddStep(current)
{
    WriteTestFor(current);
    WriteImplementationFor(current);
    Refactor();
}

TDD stands for Test Driven  Development and was first introduced by Kent Beck in his game changing book Extreme Programming Explained. This technique is similar to Test  First in the sense that you translate in tests your requirements before writing any implementation for them. By following this methodology you guide the development and the design (to some TDD means Test Driven Design) of your program with the help of the feedback that you get from the tests.

Like any methodologies it has some rules you have to adhere in order to be able to say that you actually practice TDD. These are Three Rules of TDD (follow the link for a very good explanation of the rules) :

  • Never write implementation without tests that prove that it’s correct;
  • Write the minimum amount of test that makes the test suite fail;
  • Write the simplest implementation to make the test pass;

By following this rules you enter in a short feedback loop in which you are always sure of the behavior of the program you are writing. You can either have all you implementation working or just one failing test and, as Bob Martin’s article says, almost never run the debugger.

Red-Green-Refactor

This feedback loop is commonly described as the Red Green Refractor loop because you write a test that have no implementation and which obviously fails (Red), then you write just the smallest implementation to make it pass (Green) and then, when you know the program is working, you start to Refactor and start all over again.

At every refactoring step you take you  can run your test and verify that your application is still working the way it was before starting the refactoring phase. And if suddenly you get an error, you know it was introduced by your last change because you never made too many changes without testing them. This means you are just a few seconds away from the last change you did and you remember exactly what you did and why, and in the worst case you can just undo the change loosing just a few seconds of work, instead of hours.

Advantages

When you work following the red green refactor loop you find yourself constantly concentrated in implementing the next requirement, the next, and the next again. It’s easier to enter in a state of Flow when you could continue writing code without interruption. Personally I find it addicting, when I program following TDD I need to use the pomodoro technique to remind me to take a break every 25 minutes or so.

The flow state is facilitated by the fact that, as I said before, you rarely launch the debugger. Instead, you keep checking all your code after every change you make.

Another great advantage is that by following TDD you end up with a pretty easy to understand and easy to verify documentation of your code. Think about the last time you had to maintain a piece of code: you probably had to go through a bunch of methods trying to understand. If you working in TDD has the nice side effect of leaving a trail of tests that exercise your implementation in every possible way so, if you need to understand how a method works you just need to look up the tests that verify that method.

Last but not least is that by having all your code tested, you are protected form introducing bugs in working software. Picture this, how many times have you changed a piece of functionality and found out later on, most probably when your code is already in production, that you broke some other feature of your program? This wouldn’t happen if all your code was covered by a comprehensive suite of tests, because you would discover the regression right after you launch your tests.

So to summarize the main advantages of using this methodology are:

  • You stay more concentrated on your work;
  • You almost stop using the debugger;
  • Tests become a living documentation that help you understand other people code;
  • High test coverage means regression free changes;

What other advantages do YOU know or think TDD can have? what common objections do you have or have encountered while talking about it with your peers? let us know in the comments and well cover them in a future post.

All this is great but…

If you are thinking that you can’t start using TDD because you work with complex objects or classes you should read the next article of the series when we will cover what Test Doubles are so stay tuned

Daniele Pozzobon

Daniele has more that ten years in the software development industry building solutions to automate processes, helping his clients manage legacy code and teaching them how to become more agile by working from the ground up.

He likes spend his free time with his wife and two kids and when they go to sleep improving his skills by learning new languages and methodologies.

The Importance of TDD: A simple example

In this post we build a practical example using TDD. If you are interested in playing a little bit with the project or you what to complete the requirement as we suggest at the end of the post the files can be downloaded here

In the previous post on TDD we introduced some of the advantages of having an automated test suite, let’s recap some of them before going a little deeper into some more practical concepts:

  1. First, the computer tells you when you complete your implementation; it can be the most complex pice of requirement, but you don’t have to remember it or try to execute the code in your head to verify if it’s OK, you just run the tests and if it’s green you completed the implementation;
  2. You end up with a set of regression tests that warn you when you introduce a bug while implementing some other requirement. As soon as you run the tests, they verify all the implementation even the one you wrote hundreds of iterations ago;
  3. You gain a lot of time by not having to run tests manually, actually, you stop spending a lot of time in debug mode;

But how do we get all this magic? well, let’s have a look at the implementation of the requirement that you saw in the previous post:

  • All Approved documents can be read by all registered users which are in the Readers group of higher
  • Documents which are in the Draft state can be read by Editors, Administrators or by delegated users
  • Draft documents can be deleted only by Administrators whom cannot Approve them, the only users able to approve the documents are the Editors excluding the Author of the document

This is an actual requirement I got from a client a while ago, it is simple enough to be implemented in a blog post, but it has enough edge cases and can be risky to implement without properly testing the implementation. When we received the requirement, we implemented it in a TDD fashion and after the implementation was finished we did some manual tests. The result was impressive, we couldn’t find any bugs in our implementation  and, while the automated test run in seconds, we spent half a day to test all the requirements manually.

In this post I’ll focus only on the implementation of the method that checks whether one user can read a document or not (the first two bullet points) and I’ll leave to you the last requirement of the feature.

Let’s get started

First let me introduce the domain objects:

The rules state some specification on how every User can access a list of Documents. The User class is defined as following:

public class User
{
    public int Id { get; set; }
    public string Name { get; set; }
    public string Surname { get; set; }
    public Role Role { get; set; }
}

And every user is assigned to a specific Role:

public enum Role
{
    Guest,
    Reader,
    Editor,
    Master,
    Administrator
}

Every document is represented by the class Document and every document can have a list of users assigned as delegates:

public class Document
{
    public int Id{get;set;}
    public string Name{get;set;}
    public DateTime CreatedDate{get;set;}
    public DateTime ApprovedDate{get;set;}
    public string Content{get;set;}
    public List Delegated{get;set;}
    public List Attachments{get;set;}
    public User Author{get;set;}
    public User ModifiedBy{get;set;}
    public User Approver{get;set;}
    public Status ApprovalState { get; set; }
}

As you can notice, the document can also be in a defined approval state:

public enum Status
{
    Draft,
    Approved,
    Removed,
    Pending
}

On with the implementation

First Step

After defining the domain objects needed to implement the requirements we can start with the implementation by writing the first test. We know that a user that is assigned to the Reader role can read a document in the approved state so, let’s write this as a Test:

[Test]
public void ApprovedDocumentCanBeReadByReader()
{
    var document = new Document {ApprovalState = Status.Approved};
    var user = new User {Role = Role.Reader};

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
} 

Let’s deconstruct what’s happening:

  1. First I create a Document and set its ApprovalState to Status.Approved
  2. Then I create a User and assign it to the Editor Role
  3. Then I create a new Instance of the class AuthorizationChecker
  4. Then I call the method CanUserReadDocument passing the user and the document created earlier
  5. Later I check if the result is true or not

This code obviously doesn’t compile because the class AuthorizationChecker and it’s method CanUserReadDocument don’t exists; so I proceed to create them. But what should be the return of the method? Well, because I don’t have any other tests than the one I just wrote I’ll make it return the only value that will make the test pass so I can move on with the tests.

public class AuthorizationChecker
{
    public bool CanUserReadDocument(User user, Document document)
    {
        return true;
    }
} 

Now the test run and PASSES

Second Step

What the heck is this you say? well, this isn’t me being stupid, I know there have to be some complex algorithm to compute the result of the function, but for now I have just translated one little part of the requirement and writing a check like

    return user.Role == Role.Reader &amp;amp;amp;amp;&amp;amp;amp;amp; document.ApprovalState == Status.Approved;

would be a waste of time;
Let’s see what happens if I write some more tests:

[Test]
public void ApprovedDocumentCanBeReadByEditor()
{
    var document = new Document {ApprovalState = Status.Approved};
    var user = new User {Role = Role.Editor};

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
} 

With this tests my simple implementation still passes. And of course with this:

[Test]
public void ApprovedDocumentCanBeReadByMaster()
{
    var document = new Document {ApprovalState = Status.Master};
    var user = new User {Role = Role.Editor};

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
} 

And this:

[Test]
public void ApprovedDocumentCanBeReadByAdministrator()
{
    var document = new Document {ApprovalState = Status.Approved};
    var user = new User {Role = Role.Administrator};

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
} 

Did you notice any pattern? I have the same test repeated over and over and only some values change; I could use a very useful feature of NUnit to clean the tests a little bit: Parametrized Tests.

[TestCase(Role.Reader)]
[TestCase(Role.Editor)]
[TestCase(Role.Master)]
[TestCase(Role.Administrator)]
public void ApprovedDocumentCanBeReadByReaderOrHigher(Role role)
{
    var document = new Document {ApprovalState = Status.Approved};
    var user = new User {Role = role};

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
}

The TestCase attribute tells the testing framework to run the tests four times, passing every time a different value for role resulting in four different tests.

Third Step

Now I can write another test for the role that cannot read Approved documents: the Guest user.

[Test]
public void ApprovedDocumentsCannotBeReadByGuests()
{
    var document = new Document { ApprovalState = Status.Approved };
    var user = new User { Role = Role.Guest };

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsFalse(canUserReadDocument);
}

Now this tests obviously fails, And I have finally the need to change my previous implementation of CanUserReadDocument. Again, let me make the simplest change possible:

public bool CanUserReadDocument(User user, Document document)
{
    if (user.Role == Role.Guest &amp;amp;amp;&amp;amp;amp; document.ApprovalState == Status.Approved)
        return false;
    return true;
}

The tests are green again and I’m a Happy Coder!!! Without breaking my tests I could rewrite the method like this:

public bool CanUserReadDocument(User user, Document document)
{
    return document.ApprovalState == Status.Approved &amp;amp;amp;&amp;amp;amp; user.Role != Role.Guest;
}

And then I can be more explicit with what is it I’m try to implement

public bool CanUserReadDocument(User user, Document document)
{
    return IsApproved(document) &amp;amp;amp;&amp;amp;amp; CanReadApprovedDocuments(user);
}

private static bool IsApproved(Document document)
{
    return document.ApprovalState == Status.Approved;
}

private static bool CanReadApprovedDocuments(User user)
{
    return user.Role != Role.Guest;
}

My tests still pass, still a Happy Coder!!!

All possible tests for the first bullet point have been written and I’m sure that the implementation is complete.

Fourth Step

The second bullet point states that:

  • Documents which are in the Draft state can be read by Editors, Administrators or by delegated users

So, let’s write a test that specifies this requirement:

[TestCase(Role.Administrator)]
[TestCase(Role.Editor)]
public void DraftCanBeReadByEditorsAndAdministrators(Role role)
{
    var document = new Document { ApprovalState = Status.Draft };

    var user = new User { Role = role };

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
}

As before, this test fails because it’s not implemented yet, so let’s make the simplest implementation possible:

public bool CanUserReadDocument(User user, Document document)
{
    if (document.ApprovalState == Status.Draft)
        return true;
    return IsApproved(document) &amp;amp;amp;&amp;amp;amp; CanReadApprovedDocuments(user);
}

This will suffice to make the test pass, but checking that Guest, Reader and Master cannot read Draft document still fails

[TestCase(Role.Guest)]
[TestCase(Role.Reader)]
[TestCase(Role.Master)]
public void DraftCannotBeReadByGuestReaderMaster(Role role)
{
    var document = new Document { ApprovalState = Status.Draft };
    var user = new User { Role = role };

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsFalse(canUserReadDocument);
}

And to make it pass we can change the implementation like this:

public bool CanUserReadDocument(User user, Document document)
{
    if (document.ApprovalState == Status.Draft &amp;amp;amp;&amp;amp;amp; (user.Role == Role.Editor || user.Role == Role.Administrator))
        return true;
    return IsApproved(document) &amp;amp;amp;&amp;amp;amp; CanReadApprovedDocuments(user);
}

Pretty simple, right? Now, that the test passes I’ll refactor it a little bit:

public class AuthorizationChecker
{
    public bool CanUserReadDocument(User user, Document document)
    {
        return
            IsDraft(document) &amp;amp;amp;&amp;amp;amp; CanReadDraftDocument(user) ||
            IsApproved(document) &amp;amp;amp;&amp;amp;amp; CanReadApprovedDocuments(user);
    }

    private static bool CanReadDraftDocument(User user)
    {
        return (user.Role == Role.Editor || user.Role == Role.Administrator);
    }

    private static bool IsDraft(Document document)
    {
        return document.ApprovalState == Status.Draft;
    }

    private static bool IsApproved(Document document)
    {
        return document.ApprovalState == Status.Approved;
    }

    private static bool CanReadApprovedDocuments(User user)
    {
        return user.Role != Role.Guest;
    }
}

Fifth Step

Now, let me add a delegated user to a document and verify that it can read the document if the document is draft:

[Test]
public void DraftDocumentsCanBeReadByDelegatedUsers()
{
    var user = new User() { Id = 1 };
    var document = new Document{ApprovalState = Status.Draft};
    document.Delegated = new List&amp;amp;lt;User&amp;amp;gt; {user};

    var auth = new AuthorizationChecker();
    var canUserReadDocument = auth.CanUserReadDocument(user, document);

    Assert.IsTrue(canUserReadDocument);
}

Now we know we can modify the private method CanReadDraftDocument to check whether there’s the user is part of the delegates:

public class AuthorizationChecker
{
    public bool CanUserReadDocument(User user, Document document)
    {
        return
            IsDraft(document) &amp;amp;amp;&amp;amp;amp; CanReadDraftDocument(user,document) ||
            IsApproved(document) &amp;amp;amp;&amp;amp;amp; CanReadApprovedDocuments(user);
    }

    private static bool CanReadDraftDocument(User user,Document document)
    {
        return
            document.Delegated != null &amp;amp;amp;&amp;amp;amp; document.Delegated.Any(x =&amp;amp;gt; x.Id == user.Id) ||
            user.Role == Role.Editor || user.Role == Role.Administrator;
    }

    private static bool IsDraft(Document document)
    {
        return document.ApprovalState == Status.Draft;
    }

    private static bool IsApproved(Document document)
    {
        return document.ApprovalState == Status.Approved;
    }

    private static bool CanReadApprovedDocuments(User user)
    {
        return user.Role != Role.Guest;
    }
}

Sixth Step

Relax!

Now that all the requirement is implemented and fully tested. You know it will work in production and you don’t need to run it in debug mode. Now you can go on with your next requirement.

Try to complete the feature implementing the third bullet point

  • Draft documents can be deleted only by Administrators whom cannot Approve them, the only users able to approve the documents are the Editors excluding the Author of the document

Let me know what aha moments did you get while working on it.