TDD workshop

sherlockFew months ago in the office we started a discussion about testing.
The original topic was “how to implement a continuous integration/continuous delivery workflow”, so “how to test” became immediately a requirement and a parallel important topic.

My suggestion was to start coding in TDD of course, but like every office we had a lot of legacy code to deal with.
In order not to be stuck by that, we decided temporarily to focus on new (or almost new) projects and apply it to them.

So the point became to find a way to have all the developers, regardless on the seniority and the project, on the same page about the topic.

We decided that the best approach was to run a lot of workshops to make practice, analyze patterns, real case studies in order to come to a common approach.

The kick off was in February, but the first real workshop was one month later and since then we tried to allocate one hour per week.

If you start to propose TDD in your team/unit/company there is chance to get a lot of objections: “It’s hard”, “we don’t have time”, “it costs more” etcetc.

For these objections already exist a lot of good answers, but I suggest the Uncle Bob videos TDD part one” and TDD part two

Luckily in my case I didn’t need to convince anyone in the management about the pros of this approach, they saw the value at the very first meeting

In the first workshop I decided to start with a practical exercise so I reused the blackjack game to make a point.

The participants to the workshop were senior dev, junior, people familiar with the concept and people that knew only the theory.

For the firsts workshops we agreed to run a cooperative session instead of a pair programming approach, so one laptop was projected on the big screen and all the participants had to alternate at writing or solving a test. All people in the room could suggest and give insights to the person on duty.

To simplify I decided to make the exercise in C# instead of JavaScript because it was better known by the participants and to reduce a bit the scope I focused the session on the dealer.

To get over the “blank page” I wrote the title of the tests, they had “only” to write and solve them in order.

Here a few tests after the first session

        [Test]
        public void It_Should_Draw_A_Card()
        {
            var target = new Dealer();
            var card = target.Draw();
            Assert.GreaterOrEqual(1, card);
        }

        [Test]
        public void It_Should_Draw_A_Card_Between_1_And_10()
        {
            var target = new Dealer();
            var card = target.Draw();
            Assert.GreaterOrEqual(1, card);
            Assert.LessOrEqual(10, card);
        }

        [Test]
        public void It_Should_Draw_A_Specific_Card()
        {
            var target = new Dealer();
            var card = target.Draw(5);
            Assert.AreEqual(5, card);
        }

        [Test]
        public void It_Should_Calculate_1_As_11()
        {
            var target = new Dealer();
            var card = target.Draw(5);
            Assert.AreEqual(11, card);
        }

The workshop went well even if we didn’t go very fast because of a lot of understandable question about the process, but listening to their questions (ie: “what is the point in the first and second test? I’m quite sure that the C# Random function works, is it really important to test it?”) and reading the tests as outsider I realized a mistake I made in my first implementation of the dealer in javascript.

This reminded me why for me it’s really useful trying to explain something to someone: in this way I can rethink, reanalyze and questioning my own assumption on a subject.

Looking at the tests one possible solution to solve them was something like

    public class Dealer
    {
        public int Draw(int? cardValue = null)
        {
            var card = cardValue ?? new Random().Next(1, 10);

            return card;
        }
    }

Then, thanks to C#, I noticed very easily my error in the first release of the dealer.
There is a gold rule about testing “if you have to force the design of the object to make possible to test it, there is somewhere a defect in its design”.

A classic example is when you have to make public a private method only to test it.
There are few cases (maybe) where this behavior is acceptable, but in most of the cases it’s a symptom of a bad design.

The truth was that a dealer able to draw a specific card was something useful to test the logic in the calculation of the score, but from a design perspective it didn’t have any sense.
A dealer that you can force to draw what you want it’s potentially a break in your software’s security.
In the real world you wouldn’t hire a dealer capable of that.

I didn’t see the problem in the first release of the blackjack because I used in javascript the Math.random, but in C# a bell rung using a “new” operator to access at the built-in functionality.

The random functionality even if is in the standard library in both languages is a Service.
Assuming that the random functionality had not been provided by the language and I had to implement by myself, I have no doubt that I would have created an interface and passed it in the constructor as a dependency.

Because in javascript Math.random is provided by the framework and I didn’t need to instantiate an object through a “new” operator, I didn’t see the oddness and I found legitimate using it directly in my object.
In the second version of the blackjack because I needed a deck for other reasons I solved the security’s problem without realizing it.

Working in C#, the awful signature that I had to create for the Draw method and the “new” operator led me immediately to a solution: to wrap the Random service into an custom service in order to inject the dependency into a constructor.

    public interface IRandom
    {
        int Next(int min, int max);
    }

    public class Random: IRandom
    {
        public int Next(int min, int max)
        {
            return new System.Random().Next(min, max);
        }
    }

In this way I could do

    public class Dealer
    {
        private IRandom _random;
        public Dealer(IRandom random)
        {
            _random = random;
        }

        public int Draw()
        {
            var card = _random.Next(1, 10);

            return card;
        }
    }

Then I updated the tests correctly

    [TestFixture]
    public class DealerTest
    {
        private Dealer _target;
        private Mock<IRandom> _mockRandom;

        [SetUp]
        public void Setup()
        {
                _mockRandom = new Mock<IRandom>();
                _target = new Dealer(_mockRandom.Object);
        }

        [Test]
        public void It_Should_Calculate_1_As_11()
        {
                _mockRandom.Setup(r => r.Next(1, 10)).Returns(1);
                var card = _target.Draw();
                Assert.AreEqual(11, card);
        }
    }

I could remove the tests “It_Should_Draw_A_Card_Between_1_And_10” and “It_Should_Draw_A_Specific_Card” because in the new design were meaningless, then mocking the dependency I could drive the return to test the business logic of the dealer also cleaning the Draw method signature.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s