dave^2 = -1

Favour test driving logic over data

2009-11-04T10:07:00.000+11:00

Warning: this post (and this blog as a whole) contains the opinions of someone who does not know what they are doing. I'm a novice at TDD, and while the advice in this post seems to have helped me, following it blindly may completely wreck your way of developing. I'm posting it more so people can take a critical look at it, and after careful consideration can choose to ignore or adopt any of it they see fit. We now return you to your regular, rambling blog post.

It's taken me a long time to figure out this simple guideline. Your mileage may vary, but since I started using this guideline I've found TDD much easier and more effective.

"Favour test driving logic over data"

When given the option, I've found test driving the logic of the SUT, rather than driving out design by writing tests for various permutations of data, guides me towards a nice, flexible design and gives me some robust tests. When I focus on testing the logic of the SUT, I am really getting to the heart of my SUT's single responsibility, and this helps give feedback on the design. When I build up a design using data-specific tests I've found I end up focussing more on driving implementation (rather than the design), and I end up with messy and fragile tests.

So what do I mean by data-based tests? You've probably seen this style of testing in almost every introductory TDD example out there. We start with an empty string, handle one input, handle multiple inputs, handle edge cases, handle exceptional cases etc. Now don't get me wrong -- I'm not saying this is bad. I'm just saying that if I've got an option I'll defer this kind of unit testing for as long as possible, preferring to unit test the logic of my class as explicitly as possible. This means that the classes that perform the actual grunt work of the application are at the very bottom, most concrete, and most specific parts of the design.

Cue long-winded example...

You can see the indirect application of this guideline in my recent Calculator Kata attempts. The first attempt uses a data-based approach, while the second attempt uses a more behavioural approach. Rather than wading through that long and tedious series, let's take a look at this idea using a more focussed example (in a single long and tedious post ;)). Let's say we want to generate a filename that we're going to use to save some results. The file name needs to be based on the mode used and the date. We'll use an abbreviated form of the mode for the filename, and maybe just include the year from the date, and then add a ".txt" extension. Something like "MD_2009.txt".

Using data-based tests for a file name generator

One way to start test driving this code is to start writing a code for a particular case, say, when we use Mode.Gherkin sometime in 2009. We could then test the same mode in 2007 and make sure the file name is still generated properly. We can then add a test for another mode and make sure that the mode abbreviation is correct. We could do a few permutations of this, then write a test for the exceptional case where we are given an unrecognised mode. Let's see some tests:

[TestFixture]
public class FileNameGeneratorFixture {
    private FileNameGenerator _generator;
    
    [SetUp]
    public void SetUp() { _generator = new FileNameGenerator(); }

    private void AssertModeAndDateGiveExpectedFileName(string expectedFileName, Mode mode, DateTime date) {
        var actualFileName = _generator.GetFileName(mode, date);
        Assert.AreEqual(expectedFileName, actualFileName);
    }

    [Test]
    public void ShouldGenerateFileNameForGherkinModeIn2009() {
        var expectedFileName = "GK_2009.txt";
        AssertModeAndDateGiveExpectedFileName(expectedFileName, Mode.Gherkin, new DateTime(2009, 1, 1));
    }

    [Test]
    public void ShouldGenerateFileNameForGherkinModeIn2007() {
        var expectedFileName = "GK_2007.txt";
        AssertModeAndDateGiveExpectedFileName(expectedFileName, Mode.Gherkin, new DateTime(2007, 1, 1));
    }

    [Test]
    public void ShouldGenerateFileNameForSnebel() {
        var expectedFileName = "SN_2007.txt";
        AssertModeAndDateGiveExpectedFileName(expectedFileName, Mode.Snerbel, new DateTime(2007, 1, 1));
    }

    /*... snip some tests for each mode. We may not have to test that it uses the right year for each mode, as we know the logic is the same... /*
    
    [Test]
    public void ShouldThrowAnExceptionOnUnrecognisedMode() {
        var unrecognisedMode = (Mode) 123;
        Assert.Throws<ArgumentOutOfRangeException>(() => _generator.GetFileName(unrecognisedMode, new DateTime()));
    }
}

This encourages an implementation that looks a bit like this:

public class FileNameGenerator {
    public string GetFileName(Mode mode, DateTime dateTime) {
        return GetModePrefix(mode) + "_" + GetYear(dateTime) + ".txt";
    }

    private int GetYear(DateTime dateTime) {
        return dateTime.Year;
    }

    private string GetModePrefix(Mode mode) {
        switch (mode) {
            case Mode.Gherkin: return "GK";
            case Mode.Flebel: return "FL";
            case Mode.Snerbel: return "SN";
            default: throw new ArgumentOutOfRangeException("mode");
        }
    }
}

What is this class really doing?

In this case our data-based tests are dancing around the real intention and logic of the SUT. The actual implementation says it quite clearly: return GetModePrefix(mode) + "_" + GetYear(dateTime) + ".txt";. This is the real logic we should be testing. To do so we'll need to isolate each bit of functionality into its own class. Let's look at how we can refactor to this, but we could easily test drive it that way to begin with, or guide our refactoring with new tests.

As an aside, you'll notice that our tests are giving us some signals that not all is well. We need to write basically the same test for each new mode we add (yes, we could use [RowTest] or whatever equivalent your testing framework has, but the point is the same). The fact we can't isolate the date formatting properly and so are only testing it with one mode is also a bit concerning. We know our current implementation can be tested using a single mode, but when that implementation needs to change we might have to update a whole lot of tests. This ickiness is a sign we've got some design problems, which in this case are OCP and SRP violations.

Introducing dependencies

We have the GetModePrefix() and GetYear() methods on our FileNameGenerator class. The outputs of these functions change based on the data they are given, but the overall logic we are interested in is how our SUT uses these values, irrespective of what data is actually provided. Let's move these two methods to new classes and use them as dependencies for our FileNameGenerator. We can then test our SUT's logic in isolation.

For our first step, let's create empty implementations of our dependencies for our FileNameGenerator. For now we'll just instantiate them in a default constructor so our existing tests won't break by changing constructor signatures.

//In FileNameGenerator.cs
private readonly ModeFormatter _modeFormatter;
private readonly DateFormatter _dateFormatter;

public FileNameGenerator() {
    _modeFormatter = new ModeFormatter();
    _dateFormatter = new DateFormatter();
}

//Empty classes in separate files:
public class ModeFormatter {}
public clas DateFormatter {}

Now we want to move our private GetYear() and GetModePrefix() methods into our new classes. We'll then update the old private methods to delegate to the new methods on our dependencies. We end up with something like this:

public class FileNameGenerator {
    private readonly ModeFormatter _modeFormatter;
    private readonly DateFormatter _dateFormatter;

    /* ... snip to conserve precious pixels ... */

    private int GetYear(DateTime dateTime) {
        //Copied the body of this method to dependency, and delegate to that...
        return _dateFormatter.GetYear(dateTime);
    }

    private string GetModePrefix(Mode mode) {
        //Same thing here...
        return _modeFormatter.GetModePrefix(mode);
    }
}

public class ModeFormatter {
    public string GetModePrefix(Mode mode) {
        switch (mode) {
            case Mode.Gherkin: return "GK";
            case Mode.Flebel: return "FL";
            case Mode.Snerbel: return "SN";
            default: throw new ArgumentOutOfRangeException("mode");
        }
    }
}
public class DateFormatter {
    public int GetYear(DateTime dateTime) {
        return dateTime.Year;
    }    
}

We can run our tests now and check that they all still pass. And they do, so now we can remove the private methods and call the dependencies directly. Our FileNameGenerator now looks nice and simple:

public string GetFileName(Mode mode, DateTime dateTime) {
    return _modeFormatter.GetModePrefix(mode) + "_" + _dateFormatter.GetYear(dateTime) + ".txt"
}

Testing logic over testing data

We now have some code that, according to our unit tests, is doing exactly what the old code did, but has pushed out the bits of behaviour that change as the data changes into some dependencies. This has left our SUT with logic that we can test in isolation from the data. Now writing these tests is going to be a funny sort of refactoring where our production code will be testing our test code. Our test code currently passes when run against our production code, and after changing the test code it should still pass.

First thing we want to do is fake out the responses from our dependencies so we remove the data-related variation. There are a few ways to do this in our beloved, statically typed little language, but one of the easiest that has some nice design implications, but at the cost of a bit more (trivial) code, is to extract interfaces for the dependencies. This way we have a guaranteed separated of logic from the implementation of the dependencies. I'll just point my refactoring tool of choice at the ModeFormatter and DateFormatter and extract interfaces, but you can write the interfaces manually without any trouble:

public interface IModeFormatter {
    string GetModePrefix(Mode mode);
}
public interface IDateFormatter {
    int GetYear(DateTime dateTime);
}

We'll also provide an additional constructor to FileNameGenerator to allow us to inject dependencies as required. This will help us to test the logic of the class, but also gives us a handy way to change the class' behaviour by giving it different dependencies. (Cue Open Closed Principle reference.)

public class FileNameGenerator {
    private readonly IModeFormatter _modeFormatter;
    private readonly IDateFormatter _dateFormatter;

    public FileNameGenerator(IModeFormatter modeFormatter, IDateFormatter dateFormatter) {
        _modeFormatter = modeFormatter;
        _dateFormatter = dateFormatter;
    }

    public FileNameGenerator() : this(new ModeFormatter(), new DateFormatter()) {}
 /* ... snip ... */
}

I've left the default constructor in at this stage and just chained it to the new constructor. That way we can still run our tests and code without further changes. (This is also referred to as "Poor Man's Dependency Injection", probably because it is commonly employed when we can't afford one of the great, free DI containers in the OSS market. :P)

Finally, we're in a position to write some logic-based tests. In fact, we only really need one:

[TestFixture]
public class FileNameGeneratorFixture {
    private FileNameGenerator _generator;
    private Mode mode;
    private string modePrefix;
    private DateTime date;
    private int year;
    
    [SetUp]
    public void SetUp() {
        var modeFormatter = MockRepository.GenerateStub<IModeFormatter>();
        var dateFormatter = MockRepository.GenerateStub<IDateFormatter>();

        mode = Mode.Snerbel;
        modePrefix = "MODE";
        modeFormatter.Stub(x => x.GetModePrefix(mode)).Return(modePrefix);

        date = new DateTime();
        year = 1234;
        dateFormatter.Stub(x => x.GetYear(date)).Return(year);

        _generator = new FileNameGenerator(modeFormatter, dateFormatter);    
    }

    [Test]
    public void ShouldGenerateFileNameUsingModePrefixAndYear() {
        var result = _generator.GetFileName(mode, date);
        Assert.That(result, Is.EqualTo(modePrefix + "_" + year + ".txt"));
    }
}

Normally I use a style that is a bit easier to read, but as far as a basic refactoring goes this reads pretty clear to me. We are now testing exactly what we expect our class to do. But what about all the other tests we had? Well, we simply move those to new fixtures testing the implementations of our dependencies. These will still be data-based tests, but we can simplify the cases and number of tests as we don't need to deal with combinations of data:

[TestFixture]
public class ModeFormatterFixture {
    private ModeFormatter _formatter;
    
    [SetUp]
    public void SetUp() {
        _formatter = new ModeFormatter();
    }

    [Test]
    [TestCase(Mode.Flebel, "FL")]
    [TestCase(Mode.Snerbel, "SN")]
    [TestCase(Mode.Gherkin, "GK")]
    public void PrefixForMode(Mode mode, string expectedPrefix) {
        Assert.That(_formatter.GetModePrefix(mode), Is.EqualTo(expectedPrefix));
    }

    [Test]
    public void ShouldThrowAnExceptionOnUnrecognisedMode() {
        var unrecognisedMode = (Mode)123;
        Assert.Throws<ArgumentOutOfRangeException>(() => _formatter.GetModePrefix(unrecognisedMode));
    }
}

[TestFixture]
public class DateFormatterFixture {
    [Test]
    public void ShouldReturnYearFromDate() {
        var formatter = new DateFormatter();
        var date = new DateTime(2009, 1, 1);
        Assert.That(formatter.GetYear(date), Is.EqualTo(2009));
    }
}

What have we achieved?

We now have the same coverage as we had before, but we aren't trying to test all the different combinations of data, just the basic logic of each class. If we want to add a mode, we can do so without touching our FileNameGenerator or its tests. We also don't have any need to check that the filenames are generated properly when given different dates -- those responsibilities are completely separate. If you extrapolate this to more complex examples, you get code that is easier to change without worrying about breaking loads of tests.

When we are stuck with testing data...

You'll notice our data-specific tests now reside at the bottom layer of our application. Generally as we move from less abstract to more concrete we start having to make compromises for reality, and this is where we need to drop back to data-based tests. And that's fine -- if we keep these parts of the application at the bottom, and the code above is isolated from the implementation via interfaces, then we won't end up depending on the data and so our code will stay easy to change.

Data-based tests can be good candidates for customer-facing / acceptance tests. Our customers will be the ones most likely to care about what prefixes are used for each mode, and having it in a format they can read, and possibly change, can be helpful in getting the requirements correct. When our data-specific code is kept isolated these tests can be very easy to write.

A complementary technique I've found useful for separating logic from variable data is to pass in variable data as a configuration detail. I've found it is very easy to fall into testing data combinations while implementing domain/business rules by inadvertently coupling the implementation of the rule to the way that rule is processed. In this case we can separate out the logic of finding the right rule and executing it, and then inject the required collection of rules into our class which we can test independently. (See the Specification Pattern for an example of how to do this.)

Our ModeFormatter could be a possible candidate for this kind of approach. Rather than having our data-based test for each prefix we could interate over an enumerable of rules that map modes to prefixes, especially if the rules became more complex (for example, if Mode became a [Flags] enum). We could then test that our formatter picks the correct formatting specification and applies it. The mapping itself we may decide not to test, or we could rely on an acceptance test, or we could just write unit tests at that low level of abstraction knowing the code is isolated properly.

Conclusion

This may seem like a silly little example, but looking back over code I've test driven over the last 12 months I've found data-based testing like this has been one of my major sources of troubles, and it's surprisingly easy trap to fall into when you're not keeping an eye out for it. I think the more technical correct expression of this guideline is in terms of SRP, OCP, and Tell Don't Ask, but for me at least the basic approach of preferring test driving logic over data is easy to apply and a pretty easy smell to detect when you're looking out for it.

Next time you find yourself writing tests for a whole lot of combinations of inputs to the one behaviour, have a think about your current level of abstraction, and whether you may be better off isolating the logic from the data variability.

As always, I'd love to hear your thoughts, so please feel free to leave a comment or email me. :)

Keeping a change out of our master repository with Git

2009-10-30T17:23:00.001+11:00

I love Git. I can't believe I ever loved another version control system. Recently I've started using Git branches to keep one particular lot of changes out of my master repository. I wanted to upgrade my current VS solution and projects to VS 2010 Beta 2, use VS 2010 to add features to the code base, then only check in the non-VS 2010 related changes. That way my fellow developers don't have to worry about the slightly different .csproj files and we can all work together happily. :)

Now the changes made by the VS2010 upgrade wizard are very minor -- basically just updating the ToolsVersion attribute to 4.0 and providing a fallback OldToolsVersion element set to 3.5.

So here's what I did. The first step was to create a branch off my master using git checkout -b vs_upgrade. I then ran the VS 2010 upgrade wizard which changed the relevant files, then commited those to my vs_upgrade branch. Second step was to create a branch off my vs_upgrade branch to add my feature (not strictly necessary, but keeps things nicely separated). I can now happily make changes to the code and commit the new feature to my feature branch. The structure now looks something like this:

Now I need to get my feature back into my master branch, minus the VS 2010 upgrade. We can just cherry pick the change we want (I use gitk --all rather than the command line) and apply it to the master branch. Now we can push master to our remote server and everyone has the change, minus the VS 2010 bits. Very cool!

We can rebase our vs_upgrade branch whenever some new changes are applied by other developers to master, and we get back to our first picture. We always have just a single commit containing our VS-related changes (so we don't have to repeat the upgrade), and we can always just cherry pick the changes we want made on top of the upgrade and apply them to our master. I get to play with VS 2010 and the nightly builds of ReSharper 5.0, without having any effect whatsoever on my team mates using VS 2008. The ease with which Git handles this branching and any merging is just awesome! :)

Calculators and a tale of two TDDs - Pt 3: Finishing up our BDD kata

2009-10-22T10:53:00.001+11:00

This is the wrap up of my second attempt at Roy Osherove's String Calculator kata. You can read the details at the original site, but the overall gist is a calculator that has an Add() method. This method takes a string input, adds the numbers in the string together, then returns the result.

In Part 1 of this series we looked at using a traditional TDD approach to the calculator kata. In Part 2 we attempted to use a variation of Behavioural Driven Development (BDD) . In this part we'll finish off our BDD example. So far our implementation handles empty strings, single numbers, and numbers delimited by a comma and/or a newline.

To accomplish this, we have a Calculator class that uses an INumberParser to parse numbers from the string, and then an IAdder to calculate the sum of these numbers.

The final part of the exercise (well, as far as we're going for this series) is to be able to parse expressions that specify a custom delimiter. So the input to our Add() method might be in the form //[delimiter]\n[numbers]. We also need to continue handling the case were there is no delimiter specified.

This post is pretty code heavy, so if you're viewing it in an RSS reader you might find it more readable on my site where the code has syntax highlighting.

Where should customer delimiter logic go?

This seems like a different concern to me -- we are no longer just parsing out numbers, we are parsing out a delimiter message, then parsing out numbers from the remaining string based on that. Where should we add this concern? Well our Calculator itself is looking pretty tight already:

public class Calculator {
    private readonly INumberParser _numberParser;
    private readonly IAdder _adder;

    public Calculator(INumberParser numberParser, IAdder adder) {
        _numberParser = numberParser;
        _adder = adder;
    }

    public int Add(string expression) {
        var numbers = _numberParser.GetNumbers(expression);
        return _adder.Add(numbers);
    }
}

This seems a really clear implementation of what we are trying to achieve: we are getting numbers from an expression and adding them. The current specification for this class also looks pretty neat. I don't really fancy mucking it up by shoehorning in this new concern at this level of abstraction.

Maybe we can push this down into the INumberParser? The intention behind the custom delimiter feature is to provide another way to parse numbers, so this sounds pretty reasonable. Let's try creating a CalculatorExpressionParser that implements INumberParser, which can parse out the delimiter and delegate to our original NumberParser. I'm unsure of exactly what this will look like at this stage (will we have a series of handlers selected using the Specification Pattern? Or decorate our current number parser?), but we'll let our tests/specification drive out the details.

Specifying our CalculatorExpressionParser

Let's start our CalculatorExpressionParserSpec and see where that leads us.

public class When_parsing_expression : ConcernFor<CalculatorExpressionParser> {
    
    [Test]
    public void Should_parse_numbers_using_specified_delimiter() {
        Assert.That(result, Is.EquivalentTo(numbers));
    }

    protected override void Because() {
        result = sut.GetNumbers(expression);
    }
}

We still want the same basic INumberParser behaviour, so the spec isn't telling us much here. The context is where it is going to get a little scary. We want to parse out the delimiter from the expression, then remove the delimiter information from the expression and feed that into our standard NumberParser. Let's start with this:

protected override void Context() {
    char[] delimiter = new[]{';'};
    const string numbersExpression = "1;2;3;4;5";
    expression = "//;\n1;2;3;4;5";
    
    delimiterParser = MockRepository.GenerateStub<IDelimiterParser>();
    delimiterParser
        .Stub(x => x.Parse(expression))
        .Return(new DelimitedNumbersExpression(numbersExpression, delimiter));
  
    /*.... need to use this output somehow... */
}

We've now pushed down the responsibility for parsing out the delimiter and updating the expression to an IDelimiterParser. Because C# functions only support single return values, we'll need a new class to hold these two pieces of information which I'll call DelimitedNumbersExpression. This will just be a DTO, so we may as well code this up now:

public class DelimitedNumbersExpression {
    public char Delimiter { get; private set; }
    public string DelimitedNumbers { get; private set; }

    public DelimitedNumbersExpression(char delimiter, string delimitedNumbers) {
        Delimiter = delimiter;
        DelimitedNumbers = delimitedNumbers;
    }
}

Now we need to configure a NumberParser with this required delimiter, and give it our numbersExpression to parse. This is starting to sound like another responsibility, isn't it? Let's use a factory to create a configured NumberParser for us. We'll update the context like this:

protected override void Context() {
    char[] delimiter = new[]{';'};
    const string numbersExpression = "1;2;3;4;5";
    expression = "//;\n1;2;3;4;5";
    numbers = new [] {1, 2, 3, 4, 5};
    
    delimiterParser = MockRepository.GenerateStub<IDelimiterParser>();
    delimiterParser
        .Stub(x => x.Parse(expression))
        .Return(new DelimitedNumbersExpression(numbersExpression, delimiter));
    numberParserFactory = MockRepository.GenerateStub<INumberParserFactory>();
    numberParser = MockRepository.GenerateStub<INumberParser>();
    numberParserFactory.Stub(x => x.CreateWithDelimiters(delimiter)).Return(numberParser);
    numberParser.Stub(x => x.GetNumbers(numbersExpression)).Return(numbers);
}

protected override CalculatorExpressionParser CreateSubjectUnderTest() {
    return new CalculatorExpressionParser(delimiterParser, numberParserFactory);
}

This is a bit scary, particularly if you haven't used a mocking framework before. :) We've given ourselves another dependency, an INumberParserFactory which has a CreateWithDelimiter() method. We'll then return a stubbed INumberParser, which in turn will process our numbersExpression and return our required result.

Notice how we are testing a lot of things implicitly here. We don't have a test called Should_create_number_parser_from_factory() or similar, although this is in fact tested indirectly via the context (i.e. if it were not setup in the context, our actual test would not pass!). This stops our tests from being too brittle -- we can change how the SUT does its job by modifying the context, while leaving most of the specification (the scenario name, test name, assertion, and the interface used in the Because() method) unchanged.

On the positive side, this is trivial for us to implement:

public class CalculatorExpressionParser : INumberParser {
    private readonly IDelimiterParser _delimiterParser;
    private readonly INumberParserFactory _numberParserFactory;

    public CalculatorExpressionParser(IDelimiterParser delimiterParser, INumberParserFactory numberParserFactory) {
        _delimiterParser = delimiterParser;
        _numberParserFactory = numberParserFactory;
    }

    public IEnumerable<int> GetNumbers(string expression) {
        var delimitedNumbersExpression = _delimiterParser.Parse(expression);
        var numberParser = _numberParserFactory.CreateWithDelimiters(delimitedNumbersExpression.Delimiters);
        return numberParser.GetNumbers(delimitedNumbersExpression.DelimitedNumbers);
    }
}

I'm not 100% happy with this: that context is pretty mean and I think I have the names wrong, but the code is still flowing freely and seems fairly clean so let's press on.

Confession: I actually intially coded this with INumberParserFactory only accepting a single character delimiter, then refactored to support multiple delimiters for the default case of ',' and '\n' (this was a concious decision). I've omitted this step in consideration of your bandwidths costs and in the interest of finishing this post this year. :)

A delimiter parser

Our previous spec required the use of an IDelimiterParser. As I see it the DelimiterParser has to cope with two separate scenarios: when the expression contains a delimiter specifier, and when it doesn't. Let's start with the case where it actually needs to do some work.

public class When_parsing_expression_that_starts_with_a_delimiter_specifier : ConcernFor<DelimiterParser> {
    private DelimitedNumbersExpression result;
    private string expression;
    private string delimitedNumbers;
    private char delimiter;

    [Test]
    public void Should_return_specified_delimiter() {
        Assert.That(result.Delimiters, Is.EquivalentTo(new[] {delimiter}));
    }

    [Test]
    public void Should_return_delimited_numbers_without_delimiter_specifier() {
        Assert.That(result.DelimitedNumbers, Is.EqualTo(delimitedNumbers));
    }

    protected override void Because() {
        result = sut.Parse(expression);
    }

    protected override void Context() {
        delimiter = ';';
        delimitedNumbers = "1;2;3";
        expression = "//" + delimiter + "\n" + delimitedNumbers;
    }

    protected override DelimiterParser CreateSubjectUnderTest()
    {
        return new DelimiterParser();
    }
}

//DelimiterParser.cs
 public DelimitedNumbersExpression Parse(string expression) {
     var delimiter = expression[2];
    var delimitedNumbers = expression.Substring(4);
    return new DelimitedNumbersExpression(delimiter, delimitedNumbers);
 }

I actually did this in two steps, one to pass each test. Our second scenario and passing implementation looks like this:

public class When_parsing_expression_without_delimiter_specifier :ConcernFor<DelimiterParser> {
    private DelimitedNumbersExpression result;
    private char[] defaultDelimiters;
    private string expression;

    [Test]
    public void Should_return_default_delimiters() {
        Assert.That(result.Delimiters, Is.EqualTo(defaultDelimiters));
    }

    [Test]
    public void Should_return_numbers_expression_unchanged() {
     Assert.That(result.DelimitedNumbers, Is.EqualTo(expression));   
    }

    protected override void Because() {
        result = sut.Parse(expression);
    }

    protected override void Context() {
        defaultDelimiters = new[]{',', '\n'};
        expression = "1,2,3,4";
    }

    protected override DelimiterParser CreateSubjectUnderTest() {
        return new DelimiterParser();
    }
}

//DelimiterParser.cs
public class DelimiterParser : IDelimiterParser {
    private readonly char[] defaultDelimiters = new[] {',', '\n'};
    private const string delimiterSpecifier = "//";

    public DelimitedNumbersExpression Parse(string expression) {
        if (!expression.StartsWith(delimiterSpecifier)) { return new DelimitedNumbersExpression(expression, defaultDelimiters);}
        var delimiter = expression[2];
        var delimitedNumbers = expression.Substring(4);
        return new DelimitedNumbersExpression(delimitedNumbers, delimiter);
    }
}

Our tests are now all green, and it looks like we have a good case for some refactoring. Let's use Extract Method to make our DelimiterParser a bit more readable:

public class DelimiterParser : IDelimiterParser {
    private readonly char[] defaultDelimiters = new[] {',', '\n'};
    private const string delimiterSpecifier = "//";

    public DelimitedNumbersExpression Parse(string expression) {
        if (!StartsWithDelimiterSpecifier(expression)) { return ExpressionWithDefaultDelimiters(expression);}
        return ExpressionWithDelimitedNumbersAndCustomDelimiter(expression);
    }

    private DelimitedNumbersExpression ExpressionWithDelimitedNumbersAndCustomDelimiter(string expression) {
        const int positionOfDelimiter = 2;
        const int positionOfFirstNewLine = positionOfDelimiter + 1;
        const int startOfDelimiterNumbers = positionOfFirstNewLine + 1;

        var delimiter = expression[positionOfDelimiter];
        var delimitedNumbers = expression.Substring(startOfDelimiterNumbers);
        return new DelimitedNumbersExpression(delimitedNumbers, delimiter);
    }

    private DelimitedNumbersExpression ExpressionWithDefaultDelimiters(string expression) {
        return new DelimitedNumbersExpression(expression, defaultDelimiters);
    }

    private bool StartsWithDelimiterSpecifier(string expression) {
        return expression.StartsWith(delimiterSpecifier);
    }
}

This is a longer class now, but I feel the Parse() method is much more understandable now. The ugliest bits are kept private.

A trivial factory implementation

Our CalculatorExpresionParser also needed an INumberParserFactory. As this will just be creating a NumberParser via a constructor, I don't see much benefit in unit testing it. You could write tests for it if you like, but I'm happy to assume that .NET constructors work ok. :)

public class NumberParserFactory : INumberParserFactory {
    public INumberParser CreateWithDelimiters(char[] delimiters) {
        return new NumberParser(delimiters);
    }
}

This will require us to refactor our NumberParser to take constructor arguments. Let's update our spec for it:

//NumberParserSepcs.cs, in abstract Concern class:
protected override void Context() {
    delimiters = new[] {',', '\n'};
}

protected override NumberParser CreateSubjectUnderTest() {
    return new NumberParser(delimiters);
}

This also lets us simplify our NumberParser specifications too, as we no longer need to test specific delimiters (',' and '\n'), but just test that it splits on whatever delimiters it has been created with. Let's make this pass:

public class NumberParser : INumberParser {
    public readonly char[] Delimiters;

    public NumberParser(char[] delimiters) { Delimiters = delimiters; }

    public IEnumerable<int> GetNumbers(string expression) {
        if (expression == "") return new int[0];
        return expression.Split(Delimiters).Select(x => int.Parse(x));
    }
}

Please, not another spec! Have mercy!

Guess what? We're done. We don't have any unimplemented interfaces defined in specs. We should probably check this works now though. I'll add a default constructor to our Calculator class to wire up our dependencies (although for a real app we'd use an DI container).

//In Calculator.cs
public Calculator() {
    IDelimiterParser delimiterParser = new DelimiterParser();
    INumberParserFactory numberParserFactory = new NumberParserFactory();
    _numberParser = new CalculatorExpressionParser(delimiterParser, numberParserFactory);
    _adder = new Adder();
}

Just for good measure, I've added a file called IntegrationTests.cs and put in all the tests from our first attempt. And it all passed first go. Isn't that nice? :)

Post-mortem

So how'd this go? We ended up with a much more abstract design than our previous attempt. Because of this abstraction I felt code was easier to change, especially when it came to adding the custom delimiter behaviour. In that case we just picked the level of abstraction that seemed to work, then let the tests guide us. I also feel this level of abstractions helps prevent brittle tests. Because the majority of the test code focuses on the SUT's single responsibility, we only need to adjust the context to add or remove collaborators and change the behaviour.

I found that the BDD-style approach tends to force me to think more about design issues up front, and the tests give me guidance as to how to think through these issues. It makes it very easy to adhere to SOLID principles, and makes it clear when I am violating them.

I also found that when I got closer to the lowest levels of abstraction and I began hitting reality and the .NET framework code, I ended up reverting to a more TDD-style of testing. For example, the NumberParserSpecs test specific permutations of data, and it ends up just checking the use of the .NET String.Split() method and int.Parse(). I've had lots of trouble caused by focussing of testing data like this at the wrong levels of abstraction, so using the BDD-style was great for avoiding this until necessary. The majority of the time I could focus on telling dependencies to do something, not asking them for data and then acting on it.

On the negative side, we have what could reasonably be described as an over-engineered design . With 7 classes and 4 interfaces, perhaps "over-engineered" is an understatement. ;) That said, each class has a trivial implementation. With the possible exception of the DelimiterParser, you can pretty much just inspect each class for correctness. I didn't find that coding additional classes slowed me down at all. Because I got so much guidance from my tests the implementation was fairly quick. If it's just as fast and cheap to over-design, but you get the ability to more easily change your code, is that still over-design?

Of course, these posts have been more about looking at the how each approach influences the design direction, not if that level of design is a good idea. I'm quite sure you could easily end up with an identical design using either approach, but it is how each TDD style forces design decisions at different points that really interests me. I think the most illuminating part of this exercise from my perspective is that I need to work harder on my basic TDD skills -- at applying SOLID and being more aggressive in the refactoring step of the red-green-refactor cycle. Once I learn how to refactor both production and test code better, then I'll hopefully be able to move back and forward between styles as appropriate.

Hope you enjoyed this series. Would love to get your thoughts, so feel free to leave a comment, email me or tweet me @davetchepak.

Calculators and a tale of two TDDs - Pt 2: BDD-style

2009-10-21T13:38:00.001+11:00

This is my second attempt at Roy Osherove's String Calculator kata. You can read the details at the original site, but the overall gist is a calculator that has an Add() method. This method takes a string input, adds the numbers in the string together, then returns the result.

My first attempt at the problem was using a traditional TDD approach. For this attempt I'll be using a variation of Behavioural Driven Development (BDD) as described to me by JP Boodhoo at NBDN. We'll pick up from the point where I have created a new solution containing a single C# project called CalculatorKata.

As this is likely to be quite different to what you've seen before (at least, it was for me), I'll take a bit of time to describe the details of how it works, so this instalment will be longer than the last.

Test infrastructure

To support writing tests in the style I want I'll create a litte bit of test infrastructure:

[TestFixture]
public abstract class ConcernFor<T> {
    protected T sut;

    [SetUp]
    public void SetUp() {
        Context();
        sut = CreateSubjectUnderTest();
        Because();
    }

    protected virtual void Context() {}
    protected abstract T CreateSubjectUnderTest();
    protected virtual void Because() {}
}

This will be a base class for our tests, or specifications if you prefer. Before each test runs our unit testing framework will setup a context, create the subject under test (SUT), then execute the Because() method. This means we'll create our dependencies in Context(), create a subject under test that uses those depedencies, then call the sut in the Because() method. Our test itself will then assert that the right thing happened.

This is a fairly clumsy and simplistic (although reasonably understandable) version of the approach used in JP's developwithpassion.bdd library.

Designing the `Add()` method

Let's start top-down. How's our Calculator.Add() method going to work? We'll start writing a specification in CalculatorSpecs.cs:

public class CalculatorSpecs {
    public class When_given_a_string_to_add : ConcernFor<Calculator> {
        [Test]
        public void Should_return_the_result_of_adding_together_all_numbers_in_the_string() {
            Assert.That(result, Is.EqualTo(sum));
        }
    }
}

At an abstract level this states what I want to happen. When the calculator is given a string to add it should return the sum of all the numbers in that string. The implementation of that assertion is that the result we get is equal to the correct sum. We'll setup the specific instances of these variables later.

Why does this scenario occur? Because the Add() method was called with a given expression. Let's add that to our specification.

protected override void Because() {
 result = sut.Add(expression);
}

Building up a context for this to work

How are we going to get from a string expression to our sum integer? We've written the assertion and the action triggering off this scenario (the Because()), so now we've got to design a context that allows this to happen. Our test title gives us a bit of a hint as to what we need to do: we need to add together all the numbers in the string. Which implies we need to get the numbers out of the string, then sum them. Two responsibilities. So let's push each responsibility out into another object so we don't have to worry about it yet.

First, let's get something that will get some numbers out of our expression:

INumberParser numberParser;
/* ... snip ... */
protected override void Context() {
    numberParser = MockRepository.GenerateStub<INumberParser>();
    numberParser.Stub(x => x.GetNumbers(expression)).Return(numbers);
}

This has required our design to adopt an INumberParser with a GetNumbers() method on it. It is going to return some numbers from an expression. What expression? What numbers? Let's make some up:

INumberParser numberParser;
/* ... snip ... */
protected override void Context() {
    numberParser = MockRepository.GenerateStub<INumberParser>();
    expression = "(some numbers)";
    var numbers = new[] {1, 2, 3, 4};

    numberParser.Stub(x => x.GetNumbers(expression)).Return(numbers);
}

You may prefer to have our expression set to something like "1,2,3,4" so it matches the content of our numbers variable. Comes down to personal preference, but I prefer not to be prejudging exactly how our GetNumbers method is going to work. By putting in garbage then anyone reading this test will know that they have to look at the INumberParser implementation to see exactly how the numbers are getting parsed. Otherwise the parsing requirements could change and then this test would be giving misleading information.

Then we'll need something to add these numbers, right?

INumberParser numberParser;
IAdder adder;
/* ... snip ... */
protected override void Context() {
    numberParser = MockRepository.GenerateStub<INumberParser>();
    adder = MockRepository.GenerateStub<IAdder>();
    expression = "(some numbers)";
    var numbers = new[] {1, 2, 3, 4};

    numberParser.Stub(x => x.GetNumbers(expression)).Return(numbers);
    adder.Stub(x => x.Add(numbers)).Return(sum);
}

This has given us an IAdder interface with an Add() method on it that can sum numbers, as well as a variable name that sounds like a type of snake. When our adder is told to add the numbers we want to return a sum. It doesn't really matter what sum, we're just going to assume that it does its job properly. We haven't initialised our sum variable yet, so let's put in any old thing.

protected override void Context() {
 numberParser = MockRepository.GenerateStub<INumberParser>();
 adder = MockRepository.GenerateStub<IAdder>();
    expression = "(some numbers)";
 var numbers = new[] {1, 2, 3, 4};
 sum = 42;

 numberParser.Stub(x => x.GetNumbers(expression)).Return(numbers);
 adder.Stub(x => x.Add(numbers)).Return(sum);
}

All that's left now is to create our subject under test.

protected override Calculator CreateSubjectUnderTest() {
 return new Calculator(numberParser, adder);
}

Huh?

So what are we doing here? First, we've stated what our class does: when given a string it should add together all the numbers in that string. If we drill down into our context we can see how our class will accomplish this monumental feat. It will use a INumberParser to get numbers from our expression, and then use a IAdder to add those numbers.

So what is Calculator's single responsibility? It is just coordinating its two dependencies. This level of abstraction has just flowed fairly naturally from this way of thinking about and decomposing the problem while writing our test. If you're anything like me this will all look a little foreign, but I've found this style of TDD very addictive.

Those of you that have done mocking before may notice my favourite little side-effect of writing contexts like this: we have references to all the little pieces of data flowing through our class under test. We won't need to resort to any fancy Arg<int>.Matches(...) stuff, we just tell or mock/stub/substitute/test double exactly what to expect and what to return.

In case you are worried that this seems like a lot of work, it took me about 2 minutes to get this out.

Passing our specification

To pass this we switch to Calculator.cs. Our implementation will mirror the context we set up while writing the test:

public class Calculator {
    private readonly INumberParser _numberParser;
    private readonly IAdder _adder;

    public Calculator(INumberParser numberParser, IAdder adder) {
        _numberParser = numberParser;
        _adder = adder;
    }

    public int Add(string expression) {
        var numbers = _numberParser.GetNumbers(expression);
        return _adder.Add(numbers);
    }
}

This passes, and, as I can't see any refactoring to do, I think we're done. Because our class only has a single responsibility (coordinating a couple of dependencies), there aren't a whole lot of scenarios to set up, nor a whole lot of tests to write.

Driving out our `IAdder` implementation

We now have to continuing driving out the bits and pieces that the calculator needs to work. We have an INumberParser and IAdder that need implementations. IAdder sounds pretty straight forward, so let's knock that out of the way.

//AdderSpecs.cs
public class AdderSpecs {
    public class When_adding_numbers : ConcernFor<Adder> {
        private int result;
        private int sum;
        private IEnumerable<int> numbers;

        [Test]
        public void Should_return_the_sum_of_the_numbers() {
            Assert.That(result, Is.EqualTo(sum));
        }

        protected override void Because() {
            result = sut.Add(numbers);
        }

        protected override void Context() {
            numbers = new[] {1, 2, 3, 4};
            sum = 10;
        }

        protected override Adder CreateSubjectUnderTest() {
            return new Adder();    
        }
    }
}

//Adder.cs
public class Adder : IAdder {
    public int Add(IEnumerable<int> numbers) {
        return numbers.Sum();
    }
}

Creating a number parser

Finally, we need to tackle the INumberParser implementation. This one was driven out a requirement at a time, similar to the approach taken for my first attempt at the problem. First I added handling for the empty string case, then a string containing a single number, then multiple numbers separated by commas etc.

public class NumberParserSpecs {
    public abstract class Concern : ConcernFor<NumberParser> {
        protected string numberString;
        protected IEnumerable<int> result;

        protected override void Because() {
            result = sut.GetNumbers(numberString);
        }

        protected override NumberParser CreateSubjectUnderTest() {
            return new NumberParser();
        }
    }

    public class When_parsing_an_empty_string : Concern {

        [Test]
        public void Should_return_no_numbers() {
            Assert.That(result, Is.Empty);
        }

        protected override void Context() {
            numberString = "";
        }
    }

    public class When_parsing_a_string_containing_a_single_number : Concern {
        [Test]
        public void Should_return_the_number() {
            Assert.That(result, Is.EquivalentTo(new[] {1}));
        }

        protected override void Context() {
            numberString = "1";
        }
    }

    public class When_parsing_a_string_with_multiple_numbers_separated_by_commas : Concern {
        [Test]
        public void Should_return_all_the_numbers() {
            Assert.That(result, Is.EquivalentTo(new[] {1,2,3,4,5}));
        }

        protected override void Context() {
            numberString = "1,2,3,4,5";
        }
    }

    public class When_parsing_a_string_with_multiple_numbers_separated_by_newlines : Concern {
        [Test]
        public void Should_return_all_the_numbers() {
            Assert.That(result, Is.EquivalentTo(new[] { 1, 2, 3, 4, 5 }));
        }

        protected override void Context() {
            numberString = "1\n2\n3\n4\n5";
        }
    }
}

The main difference between this and the previous approach is that we are only testing the number parsing here, in isolation of the addition. This should hopefully give us more flexibility in changing the parsing later.

public class NumberParser : INumberParser {
    private static readonly char[] Delimiters = new[] {',', '\n'};

    public IEnumerable<int> GetNumbers(string expression) {
        if (expression == "") return new int[0];
        return expression.Split(Delimiters).Select(x => int.Parse(x));
    }
}

We still have two responsibilities here: splitting the expression and converting each part into an integer. That's a potential refactoring for us, but I'm happy enough with this for now.

What's left?

The remaining feature we have to implement is allowing a custom delimiter to be specified by beginning the expression with "//". This caused me a bit of trouble in the first instalment, so we'll tackle this in the next part of this series so we can look at it in a bit more depth.

Calculators and a tale of two TDDs - Pt 1: a traditional approach

2009-10-21T13:11:00.001+11:00

I've seen TDD practiced a number of different ways. All of them use the basic "red, green, refactor" approach (i.e. write a failing test, pass it, refactor to clean up the code), but differ in the way the tests are written and in the focus of each test. Each way seems to lead the design differently, pushing design decisions at different times, requiring different amounts of refactoring and also focussing on quite different elements of design and implementation. I have a feeling that when practiced by masters each approach would converge to similar levels of awesomeness, but how far the rest of us get with each approach seems to vary greatly by how we naturally tend to approach problems.

To investigate this I thought I'd have a go at a coding exercise, and attempt it several different ways. You can't draw too many conclusions from this as the knowledge I get each time I go through the exercise will impact the later attempts, but I thought it could be an interesting exercise regardless.

The exercise I've picked is Roy Osherove's String Calculator kata. You can read the details at his site, but as it helps to avoid reading ahead I'll just describe it as we go. The overall gist is a calculator that has an Add() method. This method takes a string input, adds the numbers in the string together, then returns the result.

For this first attempt I'll be using a traditional Test Driven Development approach (or at least, traditional TDD as I understand it from reading some early-ish work on the topic by Kent Beck et al.). We'll pick up from the point where I have created a new solution containing a single C# project called CalculatorKata. I have two files: Calculator and CalculatorTests.

First test: empty strings

The first requirement for Add() is that it should return zero when given an empty string. Let's do that:

[TestFixture]
public class CalculatorTests {
    private Calculator calculator;

    [SetUp]
    public void SetUp() {
        calculator = new Calculator();
    }
    
    [Test]
    public void Takes_empty_string_and_returns_zero() {
        Assert.That(calculator.Add(""), Is.EqualTo(0));
    }
}

I've cheated a bit here by extracting a SetUp method in advance, but I'm pretty sure I'm going to need a new calculator for each test. Maybe this is evil. Feel free to point out the problems with this in the comments. This test doesn't pass, so let's pass it:

public class Calculator {
    public int Add(string expression) {
        return 0;
    }
}

It passes. Stay with me, things should start getting more interesting later (I hope...).

Test 2: Adding a single number

The next requirement is for when the string contains a single number. The test and passing implementation is below:

//CalculatorTests.cs
[Test]
public void Takes_a_single_number_and_returns_it() {
    Assert.That(calculator.Add("2"), Is.EqualTo(2));
}

//Calculator.cs
public int Add(string expression) {
    if (expression == "") return 0;
    return int.Parse(expression);
}

Summing multiple numbers

The next requirement is to actually sum multiple numbers separated by commas (','). We'll start of with the easy case: 2 numbers.

//CalculatorTests.cs
[Test]
public void Takes_two_numbers_delimited_by_commas_and_returns_the_sum() {
    Assert.That(calculator.Add("1,2"), Is.EqualTo(3));
}

//Calculator.cs
public int Add(string expression) {
    if (expression == "") return 0;
    return expression.Split(',').Sum(x => int.Parse(x));
}

Now you're quite welcome to debate me on whether this is actually the simplest thing that could possibly work. I could assume from my test that I'll only be dealing with two single digit numbers separated by a comma, get each digit via array access (expression[0] and expression[2]), parse each as an int and return the sum, but that hardly sounds any simpler to me. The current implementation will also pass our next test:

[Test]
public void Takes_multiple_numbers_separated_by_commas_and_returns_the_sum() {
    Assert.That(calculator.Add("1,2,3,4,5,6,7,8,9"), Is.EqualTo(45));
}

The only refactoring I can see is extracting the delimiter (',') into a constant. Once that's done we can go to the next step.

Allow commas and newlines as delimiters

//CalculatorTests.cs
[Test]
public void Takes_numbers_delimited_by_a_newline_and_returns_the_sum() {
    Assert.That(calculator.Add("1\n2\n3"), Is.EqualTo(6));
}

//Calculator.cs
public class Calculator {
    private string[] delimiters = new[] {",", "\n"};
    public int Add(string expression) {
        if (expression == "") return 0;
        return expression.Split(delimiters, StringSplitOptions.None).Sum(x => int.Parse(x));
    }
}

Allow a custom delimiter

The next requirement in the kata is more interesting. As input we can optionally specify a delimiter by using the following format: //[delimiter]\n[numbers]. The test is fairly simple, but the code got very messy:

//CalculatorTest.cs
[Test]
public void Can_set_the_delimiter_at_the_start_of_the_expression() {
 Assert.That(calculator.Add("//;\n1;2;3;4"), Is.EqualTo(10));
}

//Calculator.cs
public class Calculator {
    private const string CustomDelimiterToken = "//";
    private string[] DefaultDelimiters = new[] {",", "\n"};

    public int Add(string expression) {
        if (expression == "") return 0;
        var delimiters = DefaultDelimiters;
        if (expression.StartsWith(CustomDelimiterToken)) {
            var indexOfStartOfCustomDelimiter = CustomDelimiterToken.Length;
            var indexAfterCustomDelimiter = expression.IndexOf("\n");
            var customDelimiter = expression.Substring(indexOfStartOfCustomDelimiter, indexAfterCustomDelimiter - indexOfStartOfCustomDelimiter);
            delimiters = new[] {customDelimiter};
            expression = expression.Substring(indexAfterCustomDelimiter + 1);
            //Console.WriteLine(expression);
        }
        return expression.Split(delimiters, StringSplitOptions.None).Sum(x => int.Parse(x));
    }
}

That's beginning to look pretty intolerable. Our test passes, but this is in desperate need of refactoring. Now I'm pretty sure I've stuffed up the traditional TDD process here: this seems like much too big a step to get this test to pass. This could be an indication that I should have refactored first. Another indication is, as you may have noticed, I left a commented out Console.WriteLine() in there. That was because I had initially stuffed up the string escaping of the newline character, and couldn't find a good way to get inside my implementation to test what the value was at that point. This just feels plain dirty when you resort to that, but pragmatism won that battle and it helped me fix my error. All of this is really screaming out for a nice big refactoring, and I did refactor, but it was mainly tinkering around the edges using Extract Method to make the mess more understandable, rather than cutting through the mess itself.

public class Calculator {
    private const string CustomDelimiterToken = "//";
    private const string NewLine = "\n";
    private string[] DefaultDelimiters = new[] {",", NewLine};

    public int Add(string expression) {
        if (expression == "") return 0;
        var delimiters = DefaultDelimiters;
        if (HasCustomDelimiterSpecified(expression)) {
            delimiters = new[] {GetCustomDelimiter(expression)};
            expression = GetExpressionWithoutCustomDelimiterSpecification(expression);
        }
        return expression.Split(delimiters, StringSplitOptions.None).Sum(x => int.Parse(x));
    }

    private string GetExpressionWithoutCustomDelimiterSpecification(string expression) {
        return expression.Substring(IndexOfFirstNewLine(expression) + 1);
    }

    private string GetCustomDelimiter(string expression) {
        var indexOfStartOfCustomDelimiter = CustomDelimiterToken.Length;
        var indexAfterCustomDelimiter = IndexOfFirstNewLine(expression);
        var lengthOfCustomDelimiter = indexAfterCustomDelimiter - indexOfStartOfCustomDelimiter;
        var customDelimiter = expression.Substring(indexOfStartOfCustomDelimiter, lengthOfCustomDelimiter);
        return customDelimiter;
    }

    private int IndexOfFirstNewLine(string expression) {
        return expression.IndexOf(NewLine);
    }

    private bool HasCustomDelimiterSpecified(string expression) {
        return expression.StartsWith(CustomDelimiterToken);
    }
}

Not horribly impressive, is it? Perhaps a little easier to understand the Add() method itself, but this implementation hardly fills me with joy. The kata is not fully finished yet, but I think this is a decent place to stop and take stock.

Where did I go wrong?

Well, first up I missed the cue for a bigger refactoring. My tests weren't really screaming out a nice direction for me to go though. If you know your SOLID principles you'll have noticed right away that I am violating the Single Responsibility Principle (SRP). My class is doing lots of things: interpretting custom delimiters specified in a string, parsing numbers from the string, and adding the numbers.

Later on I went through and factored out a NumberParser class which took care of all the delimiter stuff and returning the numbers in the string, but that class still needed to be broken down further as well. Then I also needed to update the tests, so the calculator tests only test that it adds the numbers that come back from a mock NumberParser, and then adjust most of the current tests to test that the number parser is doing the parsing properly. Sure, I could leave the current tests as integration tests and forget about each unit, but I've this approach has given me troubles in the past.

Now I'm sure that if I were a better TDDer, refactorer, OOer etc that this would have turned out better. But my main complaint is that my tests aren't really helping me drive out my design. They help me to drive out an implementation that works (for which I am very grateful -- so much better than old school hack and slash :)), but by the time I am getting design feedback and test smells I've already dug myself in a bit of a hole. Luckily I've got tests to haul myself out, but is there a better way? Or do I need to just resign myself to the fact that I'm going to have to run through the gauntlet of SOLID principles and GRASP patterns etc. after each test to see when I need to refactor? But even then, refactor to what? I can start writing tests for the NumberParser, then join the bits back together, but this seems a bit hit-and-miss to me.

I'd love to get your thoughts on this. Anyone that wants to give the first part of the kata a try I'd love to read a blog post or have a chat with you and see how things ended up for you.

Next up I'll try a different flavour of TDD on the same problem.

Using Privoxy to block net nasties and ads in Chrome [Windows]

2009-10-02T23:30:00.000+10:00

I love Google Chrome. It is so sleek, so responsive, and so fast. Problem is I've been using Firefox with AdBlock forever (well, rounded to the nearest eternity), and when I tried to switch to Chrome all the tracking cookies and ads turned my nice neat intraweb into a garish and frightening place. I finally decided to give Privoxy, a web proxy featuring junk filtering and privacy protections, a try.

Getting started with Privoxy

Download and install Privoxy
Set your HTTP proxy in Chrome (which uses the IE/Windows settings) to 127.0.0.1:8118. You can obviously use it from Firefox too if you like.
Enjoy!

Wow, that was tough. :) The out of the box configuration seems to do pretty well. You can also delve into the config files and disable certain filters, allow ads for sites that aren't obnoxious, or switch to only blocking ads on sites you blacklist instead of using Privoxy's pattern matching. Privoxy is meant to be faster and easier than blocking this stuff in your hosts file as it can use patterns and cache lookups rather than running through each line in your host file every time you need a DNS lookup.

But I'm already behind a proxy at work! What do I do?

Glad you asked! You can configure Privoxy to forward to your corporate proxy. Go to your install directory (C:\Program Files\Privoxy for me) and edit the config.txt file (er, maybe back it up first :)). Search for the section called "forward" (section 5.1 on my install), uncomment the forward command and add your proxy:

forward / proxy.example.com:8080

This will forward all URLs to the given proxy and port number. Next search for "accept-intercepted-requests", read the explanation and warnings about the setting, uncomment the command and set it to 1. This will allow any traffic coming back via the corporate proxy to work with Privoxy.

accept-intercepted-requests 1

And with any luck you're now surfing in Chrome-filled bliss! :)

But I'm not always behind another proxy! How can I switch settings?

Oh, good question! I couldn't find any autoproxy.pac style approach for this, so I opted for a fairly basic approach. I created two config files, config.txt.work (with forwarding and accept-intercepted-requests on) and config.txt.nofoward (which is just the default config file for me). I then wrote a little PowerShell script to switch between these (use at your own risk! :)):

$location=$args[0]
$target=$env:ProgramFiles + "\Privoxy\config.txt"
if ($location -eq "work") {
    $source=$target + ".work"
} else {
    $source=$target + ".noforward"
}
cp -force "$source" "$target"

Based on the argument given to the script, this will just copy the required configuration file over Privoxy's config.txt file. I then setup two shortcuts pointing to this script, one to turn on work mode and the other to turn it off. The shortcut target looks something like this:

C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noexit "C:\Users\davesquared\Documents\Applications\PrivoxySwitch\PrivoxySwitch.ps1" work

You don't need the -noexit option, I just had it in for debugging. Note the command ends with the "work" argument, so the other shortcut should have "noforward" or something similar. Finally I set both shortcuts to run as Administrator so it has permission to copy the files. I've got the shortcuts in a location indexed by Launchy, so now I can just activate Launchy, type "switch work", ok the admin prompt, and I'm in work mode.

Is it worth it?

Yes, yes it is. Chrome is so nice. And you can do lots of fancy stuff with Privoxy if you delve into it. Have a quick glance through the Privoxy Quickstart and give it a try! :)

Configuring dependencies' dependencies with StructureMap 2.5.1 [.NET,C#]

2009-09-10T23:39:00.001+10:00

To date I've generally only used the most basic of StructureMap's configuration options. Stuff like init.ForRequestedType<SomeInterface>().TheDefaultIsConcreteType<SomeImplementor>(). But I recently needed to resolve two types which needed different configurations of the same dependency type. Here is how I got it to work, but be aware that I'm a StructureMap newbie so there might be better ways of doing this.

A contrived example

Say we have two types that both require an IController:

public class HelloView {
    public HelloView(IController controller) {}
}

public class HowdyView {
    public HowdyView(IController controller) {}
}

Our IController implementation requires an IStuffDoer:

public class Controller : IController {
    public Controller(IStuffDoer stuffDoer) {}
}

Finally, we have two IStuffDoer implementations:

    public class SayHelloWorldDoer : IStuffDoer {}
    public class SayHowdyWorldDoer : IStuffDoer {}

Configuration using StructureMap

Now we want our HelloView to have a Controller configured with a SayHelloWorldDoer, while our HowdyView controller needs a SayHowdyWorldDoer. Now this is obviously a silly, contrived example, but some semi-decent reasons to do something like this could be configuring an enumerable of Specification instances, or passing through different pipelines to change behaviour of dependencies. For now let's try and wire up our silly example:

public class Bootstrapper {
    public static void WireUpApplication() {
        ObjectFactory.Initialize(init => {
            init.ForConcreteType<HelloView>()
                .Configure.CtorDependency<IController>().Is(
                    x => x.OfConcreteType<Controller>().CtorDependency<IStuffDoer>().Is<SayHelloWorldDoer>()
                 );
            init.ForConcreteType<HowdyView>()
                .Configure.CtorDependency<IController>().Is(
                    x => x.OfConcreteType<Controller>().CtorDependency<IStuffDoer>().Is<SayHowdyWorldDoer>()
                 );
        }
        );
    }
}

Now when we try and resolve a HelloView with a call to ObjectFactory.GetInstance<HelloView>(), StructureMap will inject a Controller configured with a SayHelloWorldDoer for it's IStuffDoer.

Another way to do this is with named instances in StructureMap, but I think the approach above reads reasonably neatly and reveals our intent. It's also worth pointing out that if our wireup logic becomes particularly complicated we may be better off moving parts of it into a factory or builder.

An example of driving design through top-down testing

2009-09-01T23:53:00.003+10:00

My last post on anonymous delegates/lambdas being a source of hidden responsibilities included a test written using the guidelines I picked up from Day 4 of Nothin' but .NET. This prompted Mike to leave me some interesting questions on how we get from responsibility identification to a test and on to the design and implementation. This post is my attempt to explain the discussion and basic approach that my bearded pair and I went through to write the test (as usual, the bearded guy got everything right, any mistakes written in this post are my own). It's not the best example, but it's something that came up as an actual requirement and so has the benefit of realism at the expense of the clarity of a contrivance.

Just to be clear for anyone new to my blog: I have no idea what I'm doing. This was my first attempt at applying some of the stuff I extracted from Nothin'but .NET. It could be completely wrong, cause your computer to spontaneously combust, and/or sell your kittens to a pack of hungry dogs and then gamble the profit away at the track without leaving you a cent.

Starting out

The start of the original post includes a summary of the problem we are trying to solve and the approach we're taking. We just need a PersistenceService that will save an array of ints to a file as serialised binary (something like persistenceService.SaveResults(results, path)). And we're going to start by writing a test for this (i.e. the PersistenceService is out SUT, or Subject Under Test. Yes I know it's meant to be System Under Test, but for unit tests I like thinking of it as our subject). At each step in the process of writing our test we'll be making design decisions about both this concern and also decisions about the concerns one abstraction level beneath our SUT.

I'll try and show one decision at a time to make it easier to follow, but generally when writing these things I'd go back and forth between sections and make changes based on the feedback the test was giving me.

A single responsibility

What's the responsibility of our SUT, the PersistenceService? In the previous section I mentioned it "will save an array of ints to a file as serialised binary". So, being a touch more abstract, our SUT's reason for existence is to save data. Let's put that as our scenario name.

[TestFixture]
public class When_saving_data {}

Now I've seen this name be very long and descriptive. The more descriptive it is, the more design hints this can give you later on. In our case, this is all I can really think of, so let's move on.

Breaking down the responsibility

So we know our SUT's responsibility is to save data. We started off with the more specific requirement that we need to "save an array of ints to a file as serialised binary". What's involved to meet that requirement? We need to serialise an array of ints. We also need to open a file for writing and dump our serialised array into it. We'll also need to dispose of the file once we're done. Now there is no way we can possibly put all that into our SUT and still conform to the Single Responsibility Principle (SRP). So we'll push these responsibilities down into our SUT's dependencies and let them handle it.

Being mindful of our current level of abstraction, our SUT is responsible for saving data. We've thought about all the sub-responsibilities this entails, but what's a neat way to break these up that is abstract enough to keep a simple design, but more specific than our current level of abstraction? Essentially: what is the next level of abstraction down we can use?

For this example, let's decide to break our "save data" responsibility into managing the file stream we're going to use for writing, and writing the serialised data to a stream. Now the managing file stream stuff will need to include opening a writeable stream to a file and disposing our stream when we're done, but that's at a different level of abstraction, so we don't need to worry about that just yet.

We haven't really done much other than thought about the problem at this stage. But this will give us some valuable information we can use to make design decisions as we write the rest of the test.

How does the SUT meet its responsibility?

We've now got a better idea of what is involved in getting our SUT to do what it needs to do. Let's add an assertion/test to our scenario that describes the required behaviour.

[TestFixture]
public class When_saving_data {
    [Test]
    public void Should_write_serialised_results_to_file_stream_from_given_file_path() {        
    }
}

What are we going to assert for this test? Well we have broken down our responsibility and we know we're going to do stuff with a file stream. In C# this looks a bit like this:

using (var stream = File.OpenWrite(path)) {
    //do stuff using stream
}

We're into implementation details here. You could conceivably put this off and use another level of abstraction, but eventually you'll have to touch the framework and you'll need to start dealing with these concrete constraints.

Now in the "do stuff" section we want to write data to that stream. Let's pass in a delegate that takes a stream and does stuff do it. But from the last blog post remember that creating a delegate instance is a creational responsibility. So our SUT will need something that can create this delegate instance. We also know a sub-responsibility we have is to serialise some array data, so what ever is creating our delegate will have to be able to return a delegate that can do that. Let's fill out our assertion based on a design we might like to have to cover all this:

[TestFixture]
public class When_saving_data {
    [Test]
    public void Should_write_serialised_results_to_file_stream_from_given_file_path() {        
        fileStreamer.AssertWasCalled(x => x.Write(path, streamProcessor));
    }
}

Now we've made loads of design decisions here. We've got a fileStreamer that will handle the stream lifecycle stuff, and we're expecting it will have a write method that will take a path, and also a streamProcessor function of type Action<Stream> that will write the serialised data.

Like I said at the start, this example is a bit odd. This is because of the decision we took to have a collaborator responsible for the lifetime of the stream: opening it and disposing it. This has a nice appeal to it, but dividing the responsibilities a different way gives a very different (and in some ways a cleaner) design. (Go ahead! Try it!)

Where did all these object references come from?

So where did we get the fileStreamer, path and streamProcesser that our test referred to? Well, nowhere currently. What we have written is what we would like to have. The details of where we get all this from will go in our context.

Now writing the context is itself a design activity. Almost everything that goes into our context will be something we'll need to drill down into for future tests.

First let's start with why this whole scenario is happening -- it is because our PersistenceService (our SUT) has been told to save some results (I already have a Results class from a previous test).

[SetUp]
public void Context() {
    sut = new PersistenceService();
    sut.SaveResults(new Results(data), path);
}

Now the assertion we wrote previously is checking that a fileStreamer received a call to its Write() method, so we'll need to add one of these to our context. It also needs a path to save it to, so we'll create a fake path we can use for the purpose of our test.

[SetUp]
public void Context() {
    path = "some path"; 
    fileStreamer = MockRepository.GenerateStub<IFileStreamer>();
    sut = new PersistenceService(fileStreamer);
    sut.SaveResults(new Results(data), path);
}

The next bit is a tad trickier: we want a streamProcessor delegate of type Action<Stream> that will be able to serialise data to the stream it is given as an argument. Now because the creation of this delegate is a responsibility we don't want to put this into the SUT itself, so let's pretend we have a class with a factory method capable of doing this. Let's call this class IIntegerArrayDataSerialiser (bad name, but it's what I called it last post). We'll need to be able to call a method on this so that, given some data it will return an Action<Stream>.

[SetUp]
public void Context() {
    streamProcessor = stream => { };
    data = new[] { 1, 2, 3, 4, 5 };
    path = "some path"; 
    fileStreamer = MockRepository.GenerateStub<IFileStreamer>();
    serialiser = MockRepository.GenerateStub<IIntegerArrayDataSerialiser>();
    serialiser.Stub(x => x.GetStreamSerialiser(data)).Return(streamProcessor);

    sut = new PersistenceService(fileStreamer, serialiser);
    sut.SaveResults(new Results(data), path);
}

Let's quickly review the design decisions we have made here. Our SUT will have two dependencies injected, a IFileStreamer for managing the lifetime of a file stream, and a IIntegerArrayDataSerialiser, which will create an Action<Stream> to serialise data to the stream. By stubbing a call to our IIntegerArrayDataSerialiser we have determined it will have a method called GetStreamSerialiser() which takes some data and returns a delegate to write it out to the stream.

Note: We've chosen to violate the OCP here with a reference to a particular serialiser. We could have made the entire thing more generic but it wasn't required. This should be fine provided we are happy to take the hit if this changes in future. With any luck the class will be small enough to refactor easily if this requirement emerges.

Passing our scenario

We obviously need a constructor that takes our two dependencies (Resharper helps here). We also need a SaveResults(Results results, string path) method.

public class PersistenceService {
    IFileStreamer _fileStream;
     IIntegerArrayDataSerialiser _serialiser;

    public PersistenceService(IFileStreamer fileStream, IIntegerArrayDataSerialiser serialiser) {
        _fileStream = fileStream;
        _serialiser = serialiser;
    }

    public void SaveResults(Results results, string filePath) {
        //TODO
    }    
}

Now let's have a look at our assertion, as well as a line within our test context:

//Assertion:
fileStreamer.AssertWasCalled(x => x.Write(path, streamProcessor));

//Context, stubbed IIntegerArrayDataSerialiser call:
serialiser.Stub(x => x.GetStreamSerialiser(data)).Return(streamProcessor);

This means our SUT's SaveResult() method needs to get a stream processor from its IIntegerArrayDataSerialiser, then use it in a call to the IFileStreamer.Write() method.

public void SaveResults(Results results, string filePath) {
    _fileStream.Write(filePath, _serialiser.GetStreamSerialiser(results.GetData()));
}

And we're done!

Wrap up and next steps

Notice how when we go to the trouble to break down our responsibilities our implementation becomes trivial, and the test becomes pretty simple too. We didn't need to dig deep into the capabilities of our mocking framework, as we already had references to all the arguments being used and could easily stub out the call we needed. The hardest bit was breaking down the problem. And this wasn't a particularly nice breakdown (the use of delegates made it a bit uglier than necessary), but even still it made things very simple.

Better yet, we have a clear idea of the next steps to take. We have two interfaces, IFileStreamer and IIntegerArrayDataSerialiser, that do not have implementations. But our test has told us exactly what methods they need so far, and how they are to behave. Our IFileStreamer will need a write method and it should open a file stream, call the stream processor, and dispose of the stream (we can work out how many responsibilities this actually is while writing the test -- we may choose to push some of it out into other dependencies). Our IIntegerArrayDataSerialiser will need a GetStreamSerialiser() method that given the data, will create an Action<Stream> delegate that will serialise the data to the stream (again, we'll design this via the test, but an easy way to do it is to wrap BinaryFormatter from the .NET Framework<).

One thing that probably isn't clear from this post is that both the test and the implementation took only a few minutes to write. The hardest bit is decomposing the problem and figuring out what abstractions to use, but the very act of writing the test helps give you immediate feedback as to how well you are going with this. And better yet, there's no stopping to think "what's next?", we just jump straight to the next lot of tests.

For the record, we ended up with a change in requirements and refactored this approach, including scrapping the stream processor delegate and using something simpler. The good thing about having such small pieces was that this was really easy to do.

I hope this has helped give people some ideas as to how tests can be used to drive out design and keep classes small and focussed.

Coding tiger, hidden responsibility

2009-08-21T22:03:00.001+10:00

During a recent pursuit of the Single Responsibility Principle, I stumbled upon an interesting place a responsibility can hide. Here's our task: we want to write a PersistenceService class that will let us save some Results to a file. These results will be stored as a serialised array of integers (int[]).

I'm going to use a bit of an unusual approach for writing the array out to the file. Commonly we'd have a serialiser class that would open up a file (or ask a dependency to open up the file), write the serialised data, then close and dispose the file. Instead we'll invert control a little. We'll pass in an existing file stream, and the serialiser class will write to it. The type and lifetime of the stream will be managed as an entirely separate responsibility.

Coding tiger

public class PersistenceService {
    private IFileStreamer _fileStream;
    private IIntegerArrayDataSerialiser _serialiser;

    public PersistenceService(IFileStreamer fileStream, IIntegerArrayDataSerialiser serialiser) {
        _fileStream = fileStream;
        _serialiser = serialiser;
    }

    public void SaveResults(Results results, string filePath) {
        _fileStream.Write(filePath, stream => _serialiser.Serialise(results.GetFoos(), stream));
    }
}

Here you'll see we are using an IFileStreamer to manage the lifetime of the stream, and using IIntegerArrayDataSerialiser to manage the serialisation. Our PersistenceService class itself just coordinates those dependencies, which should be its single responsibility. Let's run through the two collaborators before we move on, just so we have a working code sample.

public interface IIntegerArrayDataSerialiser {
    void Serialise(int[] data, Stream stream);
}

public class IntegerArrayDataSerialiser : IIntegerArrayDataSerialiser {
    private IFormatter formatter;

    public IntegerArrayDataSerialiser(IFormatter formatter) {
        this.formatter = formatter;
    }

    public void Serialise(int[] data, Stream stream) {
        formatter.Serialize(stream, data);
    }
}

We'll create our IntegerArrayDataDeserialiser with a BinaryFormatter (an IFormatter from System.Runtime.Serialization.Formatters.Binary in the framework), and we are basically just wrapping this class. Finally, we have our FileStreamer:

public interface IFileStreamer {
    void Write(string path, Action<Stream> streamProcessor);
}

public class FileStreamer : IFileStreamer {
    public void Write(string path, Action<Stream> streamProcessor) {
        using (var stream = File.OpenWrite(path)) {
            streamProcessor(stream);
        }
    }
}

The FileStreamer has the single responsibility of managing the lifetime of the stream used to write the file. It is basically a creational responsibility. And, believe it or not, this all works just fine.

So, have you spotted the hidden responsibility? It's in the PersistenceService. If you have, then nice work! I didn't spot it at all, although a colleague was able to point it out to me later. What I did spot though was a sign of potential trouble, not from the code, but from the tests...

Testing times

How would you test the PersistenceService as implemented? Despite the simplicity of the implementation, it is quite messy. If you haven't noticed the responsibility have a go at writing a test and see what gives you trouble. Here was how I finally got the test to pass:

public class PersistenceServiceFixture {
  [TestFixture]
  public class When_saving_data {
    private PersistenceService sut;
    private FakeFileStreamer fileStreamer;
    private FakeSerialiser serialiser;
    private int[] data;
    private Stream stream;
    private string path;

    [Test]
    public void Should_write_serialised_results_to_file_stream_from_given_file_path() {
        Assert.That(fileStreamer.PathUsed, Is.EqualTo(path));
        Assert.That(serialiser.DataSerialised, Is.SameAs(data));
        Assert.That(serialiser.StreamUsedToSerialise, Is.SameAs(stream));
    }

    [SetUp]
    public void Context() {
        stream = new MemoryStream();
        fileStreamer = new FakeFileStreamer(stream);
        serialiser = new FakeSerialiser();
        data = new[] {1, 2, 3, 4, 5};
        path = "some path";

        sut = new PersistenceService(fileStreamer, serialiser);
        sut.SaveResults(new Results(data), path);
    }

    class FakeFileStreamer : IFileStreamer {
        private Stream stream;
        public string PathUsed;

        public FakeFileStreamer(Stream stream) {
            this.stream = stream;
        }

        public void Write(string path, Action<Stream> streamProcessor) {
            PathUsed = path;
            streamProcessor(stream);
        }
    }

    class FakeSerialiser : IIntegerArrayDataSerialiser {
        public Stream StreamUsedToSerialise;
        public int[] DataSerialised;
        public void Serialise(int[] data, Stream stream) {
            DataSerialised = data;
            StreamUsedToSerialise = stream;
        }
    }
  }
}

I had to manually fake out both dependencies. I needed to put some behaviour in my FakeFileStreamer so that it would call our Action<Stream> function that the PersistenceService passes to it from the IIntegerDataSerialiser. By exposing the arguments used to call our FakeFileStreamer and FakeSerialiser we could then assert on them within our test. Not exactly pretty, is it?

What we'd like to do is to use our trusty mocking framework to generate an IFileStreamer and IIntegerArrayDataSerialiser, stub out a value and assert that the stream was written to.

Unfortunately we can't do that, because of a lurking menace...

Hidden responsibility

Let's take a closer look at the PersistenceService.SaveResults() method:

    public void SaveResults(Results results, string filePath) {
        _fileStream.Write(filePath, stream => _serialiser.Serialise(results.GetFoos(), stream));
    }

This is simply coordinating the class' two dependencies, right? It calls the Write() method of our IFileStreamer, and passes in the Serialise() method of our IIntegerArrayDataSerialiser. That coordination is one responsibility. So what is making this so annoying to test? My bearded colleague was able to walk me through the following steps to reveal (and subsequently fix) the problem. (This post is my translation of his explanation. His explanation was great, any mistakes made in this account are mine. :))

Well, firstly, we'd really like to be able to assert something like this in our test:

fileStreamer.AssertWasCalled(x => x.Write(path, stream => serialiser.Serialise(results.GetFoos(), stream));

That way we could be sure that our PersistenceService was producing the expected call to our fileStreamer, passing in the correct pointer to our serialiser.Serialise() method. So what's stopping us? Well the AssertWasCalled() method on Rhino Mocks will check the arguments for the call. It will get the path right, but we are creating a new Action<Stream> delegate to pass in our serialisation function, so we can't compare it to the one used by the PersistenceService.

Did you notice that? I didn't, despite hours of looking :). We are creating a new delegate. If we had that delegate instance we could compare the arguments in our test. But we don't, because of this line in PersistenceService:

    public void SaveResults(Results results, string filePath) {
        _fileStream.Write(filePath, stream => _serialiser.Serialise(results.GetFoos(), stream));
    }

That simple stream => _serialiser.Serialise(...) lambda declaration is creating a delegate instance. So now PersistenceService isn't just coordinating dependencies, but it is creating a new object! That's two responsibilities! We are violating SRP, and that's what our test is telling us.

Resassigning responsibilities

My bearded colleague suggested we move the responsibility for creating this serialisation delegate. Instead of the PersistenceService creating it from our IIntegerArrayDataSerialiser, he reasoned, why not just get the IIntegerArrayDataSerialiser to create it for us? After all, serialisation is its job, and for serialisation our current design needs an Action<Stream>. (Although it is not really an IIntegerArrayDataSerialiser any more, we should probably rename it to a IIntegerArraySerialisationFactory or something. We'll keep it as is just to keep things simple -- its hard enough to follow code samples on a blog without following renames too.)

public interface IIntegerArrayDataSerialiser {
    Action<Stream> GetStreamSerialiser(int[] data);
}

public class IntegerArrayDataSerialiser : IIntegerArrayDataSerialiser {
    private IFormatter formatter;
    public IntegerArrayDataSerialiser(IFormatter formatter) {
        this.formatter = formatter;
    }

    public Action<Stream> GetStreamSerialiser(int[] data) {
        return stream => formatter.Serialize(stream, data);
    }
}

Notice this class' responsibility is clearer now. Before it was pretty much just a wrapper around IFormatter. Now its responsibility is creational: it is creating a delegate for serialisation from its IFormatter. Let's see how this affects the rest of the code. Our FileStreamer can remain unchanged, so now we just need to update our PersistenceService to call the new factory method on our IIntegerDataArraySerialiser.

public class PersistenceService {
    /* ... snip ... */
    public void SaveResults(Results results, string filePath) {
        _fileStream.Write(filePath, _serialiser.GetStreamSerialiser(results.GetFoos()));
    }
}

Now let's see if this makes everything easier to test.

[TestFixture]
public class When_saving_data {
    private PersistenceService sut;
    private IFileStreamer fileStreamer;
    private IIntegerArrayDataSerialiser serialiser;
    private int[] data;
    private string path;
    private Action<Stream> streamProcessor;

    [Test]
    public void Should_write_serialised_results_to_file_stream_from_given_file_path() {
        fileStreamer.AssertWasCalled(x => x.Write(path, streamProcessor));
    }

    [SetUp]
    public void Context() {
        streamProcessor = stream => { };
        data = new[] { 1, 2, 3, 4, 5 };
        path = "some path"; 
        fileStreamer = MockRepository.GenerateStub<IFileStreamer>();
        serialiser = MockRepository.GenerateStub<IIntegerArrayDataSerialiser>();
        serialiser.Stub(x => x.GetStreamSerialiser(data)).Return(streamProcessor);

        sut = new PersistenceService(fileStreamer, serialiser);
        sut.SaveResults(new Results(data), path);
    }
}

A bit simpler, no? We can now get rid of our hand-coded test doubles and the logic they contained. And now we have access to the Action<Stream> delegate instance that was giving us trouble before. As a result, our assertion (fileStreamer.AssertWasCalled(...)) is very straight forward and is checking that our PersistenceService did exactly what it was meant to.

Now this was mainly done as a coding exercise, so I'm not advocating this design at all, but I do find the realisation that an anonymous delegate or lambda is actually a creational responsibility really useful. It's definitely going to help me divide up responsibilities better in future.

Nothin' but .NET, Sydney 2009: Final thoughts

2009-08-21T21:27:00.001+10:00

It's been almost two weeks since the Sydney edition of Nothin' but .NET finished. I've blogged my thoughts from each day of the course, but the time since the course has given me a chance to reflect on the whole event and has provided me with some much needed perspective. Please excuse me while I rant, but I need to get this off my chest.

Note: this post contains little to no technical content and is filled with disgusting sentimentality. This post may induce feelings of nausea, boredom, or the desire to unsubscribe from this blog. We recommend that, if you choose to read this post, you do so with a vomit bag or bowl in close proximity. Do not ingest this blog post. This post is for external use only. This post is highly flammable. This post may contain traces of nuts. If you are pregnant, or think you might be pregnant, then congratulations! This post may cause hair loss, wind, bloating, itching, convulsions and/or the desire to listen to Rick Astley. Using this post for the disposal of human remains may be a violation of local health ordinances. You have been warned...

A brave new world

By far the most important, exciting, daunting, confronting and demoralising thing that I found out from the course is that I know nothing about what Object Oriented design is really about. I know a couple of design patterns, SOLID, a bit of GRASP, and OO techniques like polymorphism and inheritance, but I've never put it all together in the way JP demonstrated in class.

The average size of each class was around 20 lines of code, including noise like whitespace, braces on new lines, namespace declarations etc. Each class had only one responsibility which was concisely implemented and easy to understand. This made testing really pleasant -- there was no time spent fighting the design, it all just fell into place. And this wasn't just for stuff that JP had done before, this kind of design emerged no matter what we threw at him.

My first reaction was, somewhat embarrassingly, a feeling of complete and utter incompetence. I've been doing OO programming professionally for a decade or so, and all that time I've been doing it wrong! This was quickly followed up by a feeling of demoralisation: now that I'd seen what OO design is meant to look like, I still couldn't pick up the skill to do it myself.

But somewhere behind my fairly pathetic self deprecation I was excited. This was amazing! This was what I'd been desperately trying to work out for myself for years, and it was using all the tools I already knew about!

The best way I can describe it is with a classic DaveSquared contrived analogy (TM). Say you've spent the entirety of your life in one house. Never been outside. You get by ok, but you've always felt that you were missing something. So you've kept tweaking the rooms, repainting the walls, rearranging the furniture. Then one day someone comes along, opens the front door and shows you this entire other world you never knew existed. While the basic laws of physics work the same between your house and this new world, it's full of all these complicated things that fit together in ways you can't even begin to comprehend: people, cars, trains, people, planes, shops, people, towns, countries, commerce, politics, laws, education, planets, galaxies, quantum mechanics, twitter etc. It's confronting, it's daunting, and you feel like a complete idiot for never noticing it before. But it is also brimming with exciting opportunities.

The journey

Being shown that this other world of OO design existed was invaluable, but I still had no idea how to actually work with it. I knew the basic values and concepts, and could understand JP's explanations, but I just couldn't apply it all when it came time to code.

Initially this was a huge source of disappointment for me. My expectation from the course was that I would walk out of it a better developer. But come the end of the final day I found I couldn't do a single thing any better than when I had walked in. It seemed all I was left with was a few bits of knowledge I couldn't use, and the feeling that I was a much worse developer than I initially thought (and, as people who know me can probably confirm, this is coming off a fairly low opinion of myself already! Quite an achievement!). This is quite a realisation to face up to when you've spent around 80 hours with a top notch developer and your company has shelled out a huge amount of money for the privilege.

Part of the reason for this was that JP, as he pointed out several times during the week, is a developer, not a teacher. He simply demonstrates this awesome stuff, and answers any questions you can throw at him. It's kind of up to the attendees to extract as much from the week as they can. But demonstrations alone (IMO) don't really help build you into a better developer. That's sort of left up to you. Another major contributing factor was that I was probably well out of my depth in terms of my existing knowledge.

But I think the most important factor was a matter of misaligned expectations. I don't think I was really meant to come out of the course a better developer. I was meant to be pointed in a direction, given some tips as preparation for my journey, then sent off on my way to become a better developer.

Since the course ended I've had a number of surprises. First up was just how much stuff I managed to learn during the course, which I only realised as I worked through posting summaries of each day (which incidentally is the reason I blog -- it helps me learn stuff and collect my thoughts :)). Sure, I couldn't figure out how to apply much of it, but the sheer amount of really important revelations I got was amazing.

The second surprise took me completely off guard. One week after the course, after the sleep deprivation and system shock were beginning to wear off, I started on a new bit of code at work. And I started writing tests, top-down. And I could divide the responsibilities fairly nicely. The abstractions weren't perfect, but the design was neat. The classes were small. It all just started to flow. All the stuff I failed at during the course had started to fall into place. I could apply some of this stuff! A colleague of mine who had also been on the course started pairing with me, and he started churning out some awesome code. We started talking about the design in terms of the stuff we had learnt, and it was making sense to us.

Now this was just a small win, but a win nonetheless. I know I'm going to keep struggling with this, but less than two weeks after the course I felt I had started to make some progress -- a single, small step -- on my journey to become a better developer.

And it wasn't until that point that I truly understood what JP was saying on Day 1 about competing with and comparing yourself to other developers. He was stressing that you shouldn't look at a developer and think "I want to be able to do that!". By the time you can eventually do "that", the developer you admired has moved on to a better way of doing it. And even if you do overtake him or her, there will always be someone else to try and catch up to. If you look at it like this, then you'll always be behind. What you do will never seem good enough. And that's a fantastic way to sap all the joy out of coding. And let's face it, the reason most of us are developers is because we love to code, so once you lose that you're in trouble.

Instead, JP recommended trying to improve yourself just a little bit each day, and focus on enjoying coding. No comparisons, no competitions. The sum of all these little improvements over the years can be great, but more importantly the enjoyment and the satisfaction you get from continually improving can be life changing.

Now I'm probably one of the least competitive people you're ever likely to meet, but the one thing I do compete mercilessly against is my own expectations for myself. When I felt like I was failing to perform in the course as I expected, that was rough. But now I can see that what JP was really getting at was that I shouldn't expect to be able to do all the stuff he was showing us. I just needed to start the journey.

So my initial disappointment from the course has been turned around now. I was shown this amazing new world of OO design. It's possible I could have stumbled across it myself I guess, but the course pointed out, in fairly dramatic fashion, a direction in which to head. I learnt some really important coding tips that can help me along the way. And, now the course is done, I've finally started my journey -- and it feels awesome. :)

Nothin' but .NET, Sydney 2009: Day 5

2009-08-12T11:43:00.000+10:00

The fifth and final day of Nothin' but .NET kicked off a bit later than normal (once the breakfast chats were over it was closer to 10am, but wound up around 1:45am (I think, my brain was well and truly fried by that point).

Tell, don't ask

We talked for quite a while on this. The idea is to push responsibilities on to the class that owns the data related to that responsibility. The most common violation of this principle is an object acting on data it asked another object for. For example, instead of having if (game.genre == "Platform") { /* do something */ }), we should push the do something code into the game itself. That way we are telling the game to do something, rather than asking it for information and doing it ourselves. Violating tell don't ask strips our objects of behaviour and leads to things like anaemic domain models.

Testing

I think I've rolled most of the testing stuff from the course into previous posts, but I'll rehash some of the stuff that came up on day 5 about using tests to drive design.

Again I saw that the context/SetUp of the scenario/test seems to drive most of the design. The way the context is setup determines the responsibilities and API for the SUT's collaborators and dependencies. By the time they become the SUT and their assertions are being written most of those decisions are made. In that case the only remaining decisions are the design of dependencies' dependencies. :) It is this relationship that really lets us use the tests to drive the design from the top down.

The "because" block is the API generated from the assertion and the context of the higher level component's tests. For example, if a test for our FrontController asserts that command.AssertWasCalled(c => c.Run());, then the "because" blocks of our tests around Command implementations become command.Run();. We get a similar result when we stub return values for dependencies while setting up the test context: we need to write scenarios around those calls.

Here's a pseudo-code example. Say we have a test for an HttpRequestHandler class which asserts that frontController.AssertWasCalled(fc => fc.Process(request)). We can then write tests for the FrontController around that scenario:

When the FrontController is told to process a request:
  It should run the command able to handle this request:
    command.AssertWasCalled(c => c.Run());
  Because:
    sut.Process(request);
  Context:
    request = MockRepository.GenerateStub<Request>();
    command = MockRepository.GenerateStub<Command>();
    commandRegistry = MockRepository.GenerateStub<CommandRegistry>();

    commandRegistry.Stub(c => c.GetCommandFor(request)).Returns(command);
    sut = new FrontController(commandRegistry);

This scenario is telling us that when our FrontController is told to process a request, it needs to Run() a command. This is its sole responsibility. Note how well this responsibility is summarised by the scenario and test name, "When the FrontController is told to process a request, it should run the command able to handle this request". In our context/SetUp method, we've made a design decision to have a CommandRegistry responsible for mapping a request to the right command. We'll then need to drive out the behaviour of what happens when the CommandRegistry.GetCommandForm(Request r) method is called.

I also noticed there a two different styles of TDD: incrementally driving the design vs. incrementally driving the implementation. JP's "simplest thing that makes sense" approach (see Day 3 wrap up) tends to focus the developer on driving the design, whereas sometimes the "simplest thing that works" can lead me to procedural thinking (first it should do this, test, refactor, then it should do that, test, refactor...). I'm sure this is due to me misusing TDD in this manner, but I'm reasonably confident I'm not the only TDD novice to fall into this trap of driving implementation from the tests instead of design.

Other notes from Day 5

With a FrontController architecture Commands are similar to MVC actions.
Pipelines (Pipes and Filters pattern) can be a good way to get into event-driven and message-passing architectures.
Moving away from layered and onion architectures to component layers that are loosely affiliated. The direction of communication can be guided by the Dependency Inversion Principle and specific requirements (like having Query objects accessible from anywhere via their interface, but consumed only below the service layer).
Query object pattern (as used for NHibernate's Criteria and DetachedCriteria). Related to the Specification pattern.
Collaborators for a test can be injected into the SUT, retrieved from another dependency, or retrieved from a Static Gateway (although be careful with that last one).
Don't bother creating interfaces for DTOs.
The Service Layer is responsible for unwrapping DTOs packaged by higher layers. DTOs shouldn't go lower down than that, they are strictly for communication between the Service Layer and higher levels.
Had a whirlwind tour of some Domain Driven Design (DDD) concepts.
Separating data updates using repositories from queries using Query Object pattern. Don't just dump create/read/update/delete functions on a repository. Enforce Command Query Separation (CQS).
Command and Visitor design patterns are really under-used and under-appreciated.
Introducing Pure Fabrications over primitives to help encapsulate behaviour and make writing aggregates and entities easier. An example, instead of using an IDictionary<Product,int> to map products to quantities in a shopping cart, introduce a CartItem fabrication that can be used to track both and encapsulate useful behaviour for the Cart aggregate.
Concept of Shu Ha Ri for describing the stages of learning.
if and for (loops and conditions) are a tad evil. Getting rid of them makes code nice. :)
Problem decomposition is more important than patterns.
Always look for the higher level of abstraction. Step back from the details of the problem, and tackle the abstraction instead.

Nothin' but .NET, Sydney 2009: Day 4

2009-08-10T23:23:00.001+10:00

Day 4 of Nothin' but .NET started before 9am and finished after 2:45am (including eating breakfast and dinner in the conference room while still discussing stuff).

The day's activities

We started out talking about IoC containers, and their role in controlling object creation, lifecycle, autowiring dependencies, and also in dynamic interception for AOP (using Reflection.Emit, RealProxy from System.Runtime.Remoting.Proxies, Dynamic Proxy etc). Then it was up to us to implement a simple container. The class started off trying to convert the poor man's dependency injection approach we had used to quickly hack everything together. Xerx made a great suggestion that we could simply use Func<object> delegates as factory methods, and just map requested types to those methods.

Once we had configured all the mappings using our container and removed all the no arg constructors we had used for poor man's DI, the next challenge was to drive out a fluent interface for application startup. Here I had my first and only success of the week, where I actually managed to test drive out a few classes without doing my normal trick of becoming hopelessly stuck. JP needed to make a few changes to it but it seemed like I ended up fairly close to a reasonable design. I finally felt like I might be making some progress.

We were going to end the day by chatting about and implementing the domain and the service layers, but we started to lose some attendees due to illness and exhaustion, so we decided to defer the domain stuff until the morning (or, technically, later that morning). We did end up having a quick chat about service layer styles as described in Martin Fowler's PoEA, and contrasted the Transaction Script approach (i.e. a big ol' procedural method or script) to keeping a thin service layer over a domain. A couple of us worked 'til the end of class implementing some of the front controller stuff we had skipped from earlier in the course.

In contrast to previous days, after Day 4 I felt like I was finally on track to becoming a better developer. (Spoiler alert: it wasn't to last :-\)

Writing tests

I continued to build on the testing concepts I had started uncovering on previous days, although I still struggled to apply all them. I covered a lot about trying the "simplest thing that makes sense" instead of the "simplest thing that works" in my summary of Day 3, and this continued to be an important theme for Day 4.

I finally started to get an appreciation of the impact which each part of a test definition has on design. The scenario name, test case name and assertion became the fundamental behaviour and purpose for the SUT's existence. The "because" block showed why the SUT was exhibiting this behaviour (a call to a particular method). The context/setup was then used to drive the design of the SUT's collaborators and dependencies and dole out their responsibilities, as required for the SUT to do its job. This then helped us get down to the next level of abstraction.

JP seemed to use the SUT's dependencies, as setup in the test definition, as axes by which the overall design could be varied. By decomposing the problem the SUT is trying to solve into sub-responsibilities, then pushing these responsibilities down into dependencies, the SUT stayed very clean, small and simple. Finding the right abstraction for these responsibilities (in particular, programming to the API you would like them to have) made it easier to design these dependencies once they became the SUT. Any pain, duplication or smells detected while writing tests became a clear sign that we needed to look for a different abstraction around the SUT's dependencies. For example, instead of injecting in a dependency with a required behaviour, we might need to inject a factory or other dependency that would figure out the behaviour needed and return the relevant dependency.

I quizzed JP to try and find out the process he used to make all these design decisions which he seemed to effortlessly drive out while writing the tests. A lot of it seemed to come down to the context he has built up over the years by experience, and (somewhat unfortunately for me) down to an amazing talent for spotting and thinking in abstractions. Still, here is the best approximation of his approach that I could come up with:

What responsibility does this SUT have? This becomes the scenario name.
Decompose this into sub-responsibilities. If the SUT is responsible for running a command, then it cannot also be responsible for creating or locating that command. This becomes a responsibility of the next level down the abstraction chain.
Identify collaborators/dependencies required so we can push these other responsibilities to them, rather than burdening the SUT with doing too much. If the other responsibility is creating a command, we might make a design decision to use a factory to do this.
How should the SUT react under this scenario? The description of this becomes the test title. JP often used long, descriptive titles loaded with design implications. For example, the SUT "should call the run method on the command returned by the command factory". This became a broad overview of the design intention and design decisions made, with the details fleshed out in later steps.
How do I assert this? Write the test case body in one logical assertion, as simply as possible. It should reflect the fundamental purpose of the SUT's existence.
Why does this happen? Write the "because" block, which is basically the method call that triggers the behaviour.
Work out the context/test setup. Setup the collaborators. During this time you'll be making design decisions about the responsibilities and behaviours of the dependencies.

Today's tidbits

Here's a quick round up of some other miscellaneous things I picked up on Day 4:

I need to write more code. Lots of code. Tonnes of code! Anything to get some practice in and get more context for my design decisions. Try tackling the same problem with completely different techniques. Try functional style. Try not using dependencies. Try different patterns. Try writing tests differently. Try having no tests. Regardless of the result it will help grow your context on which to base future design decisions.
Top down design is incredibly powerful for driving down from the required behaviour to the lowest levels of abstraction. Because test-driving each SUT in turn drives out the design of its dependencies, JP ended up with the problem broken down into incredibly simple abstractions. I'm not sure how you could come up with that using bottom-up design, as then your tests don't really give you any feedback on what the design of the higher level code should be.
Three essential abilities for OO design: problem decomposition, finding abstractions, and segregation of responsibilities. I have no idea how to learn the first two, but GRASP can probably help with the third.
Using dependencies as axes to vary the design of the SUT.
Keep to one logical assertion per test.
Test context/SetUp can hold design decisions and indirect assertions (e.g. stubbing a return value which is used in the test assertion is a design decision).
JP said not to think too far ahead, because "it will kill you". Keep at the current level of abstraction, and break the problem down. Don't think "what if?", think "what now?".
Concentrate on writing tests around the "happy" path. This helps to focus on the SUT's single responsibility, and defer exception handling etc. to different levels of abstraction.
Focus on outside-in testing using dependency injection. By passing dependencies in we can assert their state or calls made to them by the SUT, therefore making previously untestable internal state testable.
Static gateways may need direct access to Service Locator (IOC, or abstraction over our container).
Orchestrator pattern, an object that takes care of the sequence of operations in a pipeline.

Nothin' but .NET, Sydney 2009: Day 3

2009-08-08T22:51:00.034+10:00

Day 3 of Nothin' but .NET was a bit more laid back than previous days as we spent most of it in teams trying to apply some of the stuff we had learned. We had an early finish (10:45) in preparation for a longer day on Thursday.

I actually found this day immensely frustrating. After seeing so much cool stuff the previous two days and feeling like I was starting to understand the main concepts, it was completely demoralising to be given a fairly basic problem and utterly fail to make even the vaguest hint of progress with it. The only reason I didn't feel completely incompetent was because of the overwhelming feeling of stupidity I had, and it didn't feel quite right for someone so obviously stupid to think of a big word like "incompetence". :\ :)

This wasn't helped by the fact that as soon as JP started demonstrating how to proceed to the next stage, he made the solution seem so obvious and effortless. At the start of the course JP went to great lengths to encourage us not to compare ourselves to or compete with other developers, and that instead we should just aim to better ourselves one small step at a time. But in this case comparing JP's work to mine wasn't like comparing the work of two developers. It was more like comparing the works of Leonardo da Vinci to those of a small, under-watered cabbage. It was fairly difficult not to notice the difference.

Despite this the under-watered cabbage did manage to pick up a few things from this day. First up I got to see an end-to-end, test-driven development of a Front Controller architecture for processing web requests using Commands. The Front Controller itself was more of a component -- it consisted of several classes all grouped together to perform the front controller related tasks. It's a bit like a "layer", although the architecture wasn't really layered in a traditional sense. It was just a bunch of components working together in a pretty loosely coupled way. Not having a more traditional layered approach seemed to make the design much more flexible.

Note: As a source of examples for the following sections I'll refer to a specific class within the Front Controller component -- the slightly-confusingly named FrontController implementation (confusing because it is just one part of the entire Front Controller component). This class's responsibility is to receive an IncomingRequest (an abstraction of an HTTP get or post) and run a Command that will do whatever this request is asking.

I also picked up a couple of TDD tips. The first was to start with a test case that reflects the simplest, most fundamental description of the subject under test's (SUT's) behaviour, rather than asserting little facts about the SUT. For example, when writing a test for when our FrontController is handling a request, we shouldn't start by asserting it gets a non-null Command from its CommandFactory, or assert that CommandFactory.Create() was called on a mock object. Instead our test was that it "should tell the command that can process the request to process it".

As I started picking up on later in the week, the former is really focussed on the mechanics of an implementation, while the latter is about the required behaviour. By being very descriptive about the behaviour, we end up driving out a lot of design in that one statement (in this case, the design decision is that we need a collaborator that can return a command that can process a request, and that our SUT will run that command). This technique also encourages the use of a very simple, targeted assertion in code, which lets us defer design decisions about the SUT's dependencies until we start writing the context/SetUp method for the test.

This really comes down to the identification and segregation of responsibilities (as per the Single Responsibility Principle (SRP)). This process is really helped by attempting to identify the most abstract responsibilities beneath the SUT, while ensuring the SUT is still responsible for adding some behaviour. Or, put another way, the SUT should have only one small part to play in achieving it's overall reason for existence -- this is its single responsibility. Everything else is deferred to the implementation of its collaborators.

Another TDD technique JP used was doing the "simplest thing that makes sense", instead of the "simplest thing that could possibly work". For the FrontController example, the simplest thing that could work when initially coding it would be injecting a single Command into the FrontController and asserting that the command was run. Then what? Move on to another SUT and leave a very defficient implementation of our FrontController? In this case, we know with absolute certainty that this class will need to process more than one type of Command, so the simplest thing that makes sense is for the FrontController to get a relevant command implementation from a CommandRegistry or similar type, rather than hard-wiring in a single command. This not only gives our FrontController less reasons to change (as per the Open Closed Principle (OCP)), but it also points out the next SUT we can drive out, the CommandRegistry implementation.

Aside: In retrospect, we could potentially get the same design in a more incremental fashion by still doing the simplest thing that will work but being especially diligent during the refactoring step of TDD. Just say our first attempt was to inject a single Command and assert it's run method was called. The test passes, and the next step is to look for refactoring opportunities. We notice the OCP violation, and refactor to introduce the CommandFactory, all under the protective cover of our passing test. I have a suspicion that it might be more reliable to think about the problem in abstract terms and have a SOLID design naturally fall out than to do the simplest thing that could work, and then run it through the gauntlet of SOLID principles, GRASP patterns etc. Still, I find it comforting that if I miss the abstraction up front I still have a chance to get there via refactoring.

Other tidbits

Here is some other stuff that came up during day 3:

A long context/SetUp for a test is a test smell that indicates we're probably doing too much. Push some of it down into other collaborators so we can defer decisions about it.
Creating something is a responsibility. If a SUT needs to create something and act on it, then the creation should probably be pushed out into a Factory. The SUT is then only responsible for using the factory and its output (i.e. mediating between the two types).
When designing and writing tests, focus on abstraction and intention rather than focussing on implementation and mechanics. Yes, I've written this already in this post. No, I probably haven't stressed it enough.
If stubbed values are required for a test to work then these are tested implicitly when the test runs. In our FrontController example where it uses a CommandRegistry to get a Command and call its Run() method, we don't need to explicitly test that CommandRegistry.Create() was called. Instead we can stub out Create() to return a specific Command instance, and assert that its Run() method was called. We don't need to explicitly assert that the factory was used if the test's assertion already depends on it. This is a side-effect of identifying the simplest, fundamental assertion for a SUT rather than thinking about the implementation mechanics.
By constraining ourselves to one ViewModel per View we can use convention over configuration to wire everything up.
Went through the concept of a Service Layer, which is a type of Facade for operating on the domain.
Went over Command Query Separation (CQS)
Covered Separated Interface, where an implementation lives at a lower level than the interface itself. An example is a Query interface which is defined somewhere with visibility from all layers/components, but the implementation within the domain or by the ORM or persistence layer/component. This is why its ok to use NHibernate's criteria interfaces from pretty much anywhere within the application, provided the implementation is abstracted appropriately into a lower layer. The Dependency Inversion Principle (DIP) is used as a guide for applying the Separated Interface pattern (i.e. higher levels should not depend upon lower levels, but instead on interfaces using the Separated Interface pattern).

Nothin' but .NET, Sydney 2009: Day 2

2009-08-05T09:15:00.001+10:00

Day 2 of JP Boodhoo's Nothin' but .NET bootcamp has come and gone. JP's been aiming for an 11pm stop, but we went through until about 12:30am. Which was awesome, because we had started covering some really interesting stuff. Apologies in advance for the rambling nature of this post, but most of it was done after 2am. :-\ I'll try and distil this stuff into some decent posts after the course.

The main highlight I took away from the day was finally identifying the source of and solution to a lot of the problems I have when designing OO systems. Whenever I've tried test-driving a solution from top-down I've commonly found my tests seem to raise more questions than they answer. I'd struggle through writing one test, and have to almost-arbitrarily whack in a number of dependent classes that I'd need to contort in some strange way via mocking to get the assertion to pass.

Turns out that this was on the right track (driving out dependencies), but for the wrong reason. I had been looking for any form of collaborator for my subject under test (SUT) in a vain effort to find something (anything!) to test. It all felt pretty contrived. The reason I was struggling is for the same reason I identified on day 1: I suck at segregation and assignment of responsibilities. When you start thinking of things you want to test on your SUT, they should all relate to one behaviour/concern/responsibility (SRP). Anything else, no matter how trivial it seems, really needs to be pushed into a collaborator. (The GRASP patterns can really help in identifying what these collaborators should be.) This collaborator will most of the time need to be accessed via an interface, not a concrete class (DIP). This let's you drive out the intention behind each collaborator, without needing to fill in the shape, structure or implementation of them.

This approach gives lots of very small, very focussed classes. It also produces very focussed interfaces for each collaborator (ISP). When it's done right it also means that when a responsibility needs to be added, it can generally be done without modifying existing classes, but instead producing another implementation of one of your collaborator interfaces (OCP).

You'll notice I've littered a whole lot of TLAs (Three Letter Acronyms ;)) through those last couple of sentences -- that's my effort to tie these things back to the fundamental SOLID principles. Now I felt I had a really good understanding of SOLID, and I am very careful to consider any code I write in light of those principles, but I'd never taken them to their full, logical conclusion.

Now there is one big downside to the designs that come out of thinking like this. The object model is very, very abstract and complex. You can't just hold the entire model in your head at once. It is not immediately obvious how some input at the top layer of the application works its way through the web of myriad of incredibly simple objects to give you some output at the other end. And you know what? I'm dead certain that this is the absolute entire point of Object Oriented development.

Understanding the entire flow of an operation or application is the point of procedural programming. We know that if this value is x then it will go down this branch of an if condition, then that will call a method with this argument that will check some argument to call some other method... this is done in OO languages all the time. Sure there might be some OO niceties sprinkled around like polymorphism, composition etc but classes and objects can still end up being little more than glorified namespaces for organising functions.

By contrast if you use OO programming to its fullest, you lose that immediately-apparent result you get from a more procedural style, and instead you get a whole bunch of almost-endless abstraction. The point of abstraction is that you don't need to understand the entire model, just the bit you are working on. But the benefit you get is that it becomes trivial to understand that small piece and affect how it works via its collaborators, whose implementations are equally easy to understand once you have stepped down into the next level of abstraction.

Sure, it is hard to maintain context during huge leaps through abstraction layers, but again you don't really need the entire context. It seems to me that I need to let go of that procedural safety net of knowing all the complete paths through my application to truly start doing OO design right. And doing OO right means getting a new safety net -- having each piece of my application being trivial to understand, and almost as trivial to change its behaviour. As an aside, having these trivial components also makes your design very easy to test.

Patterns and principles

We also covered a few more patterns and principles today:

Static Gateway as a static entry point to a DSL/fluent interface.
A simple version of the Event Aggregator pattern for decoupling listeners and publishers. Basically you just have system-wide events published to and subscribed via an Event Aggregator, and then let multiple components respond independently to events of system-wide significance. This also makes it easier to handle exceptions that occur during callbacks than with traditional .NET event handling (which will generally halt an invocation of a MulticastDelegate). Also learned a cool way of using attributes to tie into the Event Aggregator (so members can be decorated to get called on a system-wide event). And found out about the System.ComponentModel.EventHandlerList class.
Collecting Parameter pattern.
Registry pattern for lookups.
Object Mother for creating and setting up unit test data.
The Front Controller pattern, the forerunner (?) to MVC and other separated presentation patterns. The Front Controller object becomes the entry point that maps inputs (e.g. from UI) into Commands, and uses these Commands to coordinate between Models and Views. I've probably got the details wrong, but we'll be doing more on this tomorrow.
Null Object pattern for eliminating null checks, including on basic things like event invocation.
The Highlander Principle: there can only be one. Translated, this means that related methods should all delegate to the method with the biggest number and specificity of parameters. This is particularly important for overridding.
Decorator pattern. I've sometimes been hesitant to decorate classes where it only does something very trivial to the calls it delegates too, but this is really the entire point. Sure you have to reproduce several methods, but the power it gives you to augment behaviour while conforming to OCP has made me love this pattern again. :)
We went through the Dependency Inversion Principle.

Miscellaneous stuff

Some other things I jotted down from today:

You can put generic constraints on delegate definitions (this is pretty obvious in retrospect, but I hadn't thought about it before).
We went through the difference between the Delegate type and the delegate keyword.
The idea of closing a lambda/anonymous method down to a known delegate type.
Const fields are copied to other assemblies when compiled, so updating the original assembly won't change the values in the other assemblies until they are recompiled.
Closures can be replaced by a class with state and then pointing to a function of that class (which is what the compiler generates for closures anyway).
Referring to passing delegates around as "passing behaviours".
Naming test contexts very simply helps to isolate responsibilities. Any behaviour not specified in the tests for a SUT generally means that those concerns are delegated to dependencies.
When writing tests/specs, focus on the happy path where everything goes right. We can then choose to defer exceptions to this path to a dependency, or to flesh out the behaviour in later tests/specs.

Nothin' but .NET, Sydney 2009: Day 1

2009-08-04T00:59:00.001+10:00

Wow. Just finished day one of JP Boodhoo's Nothin' but .NET bootcamp. I've found it pretty tough so far. I can generally follow along ok but start to flounder as soon as we have to pick up the code and start adding to it. The exercises have been pushing me far beyond the level I normally work at, but that's pretty much the idea. :) I don't think it would help much to cover everything we did (you kind of have to go through it), but I'll try and outline the main points that stick out in my memory.

First, generics and delegates. Turns out I know nothing about them. I mean, I know what they are, how they work, and even some ways to use them, but I don't really know them. Generally it's been a LINQ call here, a simple function pointer there, and an occasional generic class or method. Until today I'd never seen generics and delegates used and combined in such a way as to permeate through an entire design as off-handedly as most of us use int and string. JP just slung this stuff together like it was Hello World, while the rest of us hung on for dear life. Each individual part seemed to make perfect sense once I had seen JP's examples, but trying to code it for myself left my undersized brain desperately trying to hold on to information like the type of a method that had a generic type PropertyValue in a class that had a generic type Item that took a Func<Item,PropertyValue> and a Criteria<PropertyValue>.

Hot on the heals of revealing my inadequacy in that area, I also discovered my knowledge of responsibility assignment and problem decomposition is sorely lacking. JP made it look effortless as he stepped through implementing a fluent interface, starting with creating a static method that returned a factory or builder for our DSL, then driving down into separate classes that would handle each phrase in our grammar. I got some good tips on how to get better at this: look back over the GRASP patterns and focus on identifying and separating out responsibilities. One interesting technique I'll have to try out is over-zealously applying the Single Responsibility Principle (SRP), then potentially "de-factoring" some of the code if it is overly abstracted.

We also got some practice identifying a whole raft of design patterns which cropped up while going through the exercises. We had Adapter for adapting a Predicate<T> to a Criteria<T>, Factory for creating Criteria instances, Composite for creating potentially infinite chains of IComparator<T> references, Iterator both as a custom implementation and as built into .NET with the yield keyword, and Decorators for negating Criteria. It was great to see these naturally emerging from the application of the SRP, rather than as a deliberate refactoring toward a pattern.

I also had a long-held suspicion all-but-confirmed today. TDD/BDD can help inform your design decisions, but if you don't know good design principles up front then you are pretty much stuffed. I was sort of holding out hope that TDD can somehow teach good design, but I think I've given up on that. TDD helps point out some inadequacies in design approaches, like not using dependency injection, not adhering to SOLID principles etc., but you really need to know this stuff before TDD becomes really effective. So what makes me think that? Well, today we didn't use TDD at all, and I'm almost positive I would not have come up with the approach we did had I been using TDD. My design-fu just is not strong enough. The goal is clean code that works. This is a result of the good application of design principles. TDD can help you apply the principles, but you need to know them first.

Even the small amount we've done so far (today was apparently the "easy day") has opened my eyes to some areas I really need to improve upon, and given me some ideas on how to start doing so. It's definitely given me lots to think about for the next time I'm tackling a problem, especially around responsibility assignment. Tomorrow we start on the real work, including introducing TDD/BDD. I'm really interested to see the how TDD affects how we go about segregating responsibilities.

Enumerables, LINQ and speed

2009-07-24T15:06:00.002+10:00

Warning: This post contains an optimisation for a specific case that sacrifices readability in favour of performance, and should not be applied prematurely. Readability trumps performance until you've whacked a profiler on your code and determined it is too slow under realistic usage.

I love LINQ. Especially since trying out Python, I find it a lot easier to think in terms of apply functions over iterators, rather than big loops with nested conditions. This has always worked really well for me. At least, it had done until yesterday, when we were tracking down a performance problem experienced when running through lots of buffers full of lots of data. Let's take a look at two options for adding data to an internal list (this isn't a realistic example, it's just for profiling purposes):

private readonly List<ushort> _buffer = new List<ushort>();

public void Add(ushort[] data, int count) {    
    for (var i = 0; i < count; i++) {
        _buffer.Add(data[i]);
    }
}

public void AddLinq(ushort[] data, int count) {
    _buffer.AddRange(data.Take(count));
}

Note: these two methods aren't exactly the same. The second one won't fail if the data array has less than count elements. We could fix this by updating the condition used in the for of the first method, but the performance difference is negligible and it obscures what we are talking about.

The Add method uses simple array traversal to add the first count elements to our internal _buffer list. The AddLinq method does the same thing, but uses the Enumerable.Take(x) extension method defined in System.Linq.

If you don't think too hard about it you might assume these two have similarish performance. If I call both methods with a 10,000,000 element array and take the first 9,999,900 or so of those, I find the LINQ version takes around 370ms, while the non-LINQ version is around 135ms.

Now if this was in the context of a web page loading or for smaller amounts of data then I don't think it's going to bother anyone, but this is in the context of large amounts of data coming in every 20ms or so, so faster is better and taking almost 3 times as long is definitely a worry. Regardless of whether we care about this additional time, where exactly is it coming from?

Well the Take() extension method most likely uses something like this:

public static IEnumerable<T> Take<T>(this IEnumerable<T> data, int numberOfItemsToTake) {
    var itemsTaken = 0;
    foreach (var item in data) {
        if (itemsTaken >= numberOfItemsToTake) yield break;        
        itemsTaken++;
        yield return item;
    }
}

If you're not familiar with how yield return works under-the-hood, have a look at Raymond Chen's discussion on the topic (Part 1, Part 2, and Part 3), and Jon Skeet's article. Basically the compiler generates a class that implements IEnumerable<T> and IEnumerator<T>, and it keeps track of the state of the enumeration between yields from this method. With the overhead of another instance that has it's own internal state machine to maintain, it's never going to be as efficient as using a plain array index for state. Profiling two calls to each method gives the following:

In both cases the actual list insertion of each element is about the same (the Insert and Add calls), around 600ms in total. In the non-LINQ case, this leaves about another 600ms for interating over the list using for and our integer index. In the LINQ case, we have a large amount of time (2,310ms) spent in MoveNext() on our compiler generated iterator (<TakeInterator>d__3a). You'll notice that the slow MoveNext() call delegates to an SZGenericArrayEnumerator<T> which itself is reasonably fast, taking about 800 ms to iterate over the required elements compared with the 600ms of the non-LINQ version. The rest of the overhead is maintain the state machine, jumping between states and checking whether we have taken count number of elements yet. (It's really interesting to step through this stuff with Reflector, try searching for <TakeInterator> with private/internal classes displayed if you want to see what goes on).

We ended up de-LINQing a couple of heavily used bits of code and solving most of the performance issues. Just remember that this is for a very specific scenario, and if you haven't measured it, don't optimise it away. But I think it is useful to have an idea of all the hard work happening under the hood of all that LINQy enumerable goodness.

ReSharper Live Template for Dependency Properties (C#)

2009-06-27T21:38:00.004+10:00

The syntax for declaring WPF dependency properties can get a little tedious to type. I ended up creating a ReSharper Live Template which has been working nicely for me:

public static readonly DependencyProperty $PropertyName$Property = 
    DependencyProperty.Register("$PropertyName$", typeof($PropertyType$), typeof($PropertyOwner$));

public $PropertyType$ $PropertyName$ {
    get { return ($PropertyType$) GetValue($PropertyName$Property); }
    set { SetValue($PropertyName$Property, value); }
}

Then configure the substitutable variables as follows:

PropertyName, no macro, #3 editable
PropertyType, Guess type expected at this point, #2 editable
PropertyOwner, Containing type name, not editable

This lets me type in dp and fill in the blanks. Hope this helps. :)

Occasional trouble mocking generic methods

2009-06-17T00:00:00.000+10:00

I've seen a few people get bitten by an edge case of using generics with mocking frameworks. Let's look at a contrived example and see what's going on. We're using Rhino Mocks 3.5 in this case, but the symptoms described here are not specific to that mocking framework, but rather are related to how generics work in .NET.

public interface IFish{}
public class Eric : IFish{}
public interface IAquarium {
    IEnumerable<T> FindEverythingLike<T>(T thisThing);
}

[Test]
public void LookingForALicenceForMyPetFishEric() {
    var petFish = new Eric();
    var expectedErics = new[] {petFish};

    IAquarium stubAquarium = MockRepository.GenerateStub<IAquarium>();
    stubAquarium
        .Stub(aquarium => aquarium.FindEverythingLike(Arg<IFish>.Is.Anything))
        .Return(expectedErics);

    var actualErics = stubAquarium.FindEverythingLike(petFish);
    Assert.That(actualErics, Is.EqualTo(expectedErics));
}

Here we're asking our stubbed IAquarium to return an expectedErics array whenever it gets any IFish as an argument to the generic FindEverythingLike<T>() method. We then pass our petFish to the method and get the actual enumerable returned from our stub. So what does this assertion give us?

MockingGenerics.LookingForALicenceForMyPetFishEric : FailedNUnit.Framework.AssertionException:   Expected: < <Workshop.Tests.Eric> >
  But was:  null

Oops, our stubbed value hasn't been returned. Our petFish is an instance of class Eric, which implements IFish, so why isn't our stub returning our expectedErics?

The reason for this is that we are actually stubbing out FindEverythingLike<IFish>(), but are calling FindEverythingLike<Eric>(). While we declare this as a single generic method, the CLR actually calls these as two completely different method instances. This is obscured a bit by the fact that type inference is used to determine which method instance is called. If we don't rely on type inference we can get the test to pass:

    var actualErics = stubAquarium.FindEverythingLike<IFish>(petFish);
    Assert.That(actualErics, Is.EqualTo(expectedErics));

In the real examples where I've seen errors like this we generally have more collaborators involved and the problem becomes harder to detect. It's not a common case, requiring us passing different static types to a generic method where at first glance it looks reasonable to expect our call to resolve to the one method.

Just to belabour the point for one more example, let's just confirm that it is the static type of our reference that determines which method instance is called, rather than the actual instance type:

[Test]
public void GenericsAndMockingCanBeTroublesome() {
    IAquarium mockAquarium = MockRepository.GenerateMock<IAquarium>();    
    Eric petFish = new Eric();
    IFish sameFish = petFish;

    mockAquarium.FindEverythingLike(petFish);
    mockAquarium.AssertWasCalled(aquarium => aquarium.FindEverythingLike(sameFish));
}

This case fails, as we are calling mockAquarium.FindEverythingLike<Eric>(), but asserting that FindEverythingLike<IFish>() was called. Even though it is the same instance, the declared types of each reference at compile time are the ones that are used to dispatch to our generic method instance.

Hope this helps save someone a couple of minutes of frustration. :)

Scenario-based unit testing post in PDF format

2009-06-11T23:10:00.002+10:00

My last post on Moving to scenario-based unit testing in .NET was pretty long, even for me. I've just gone and done a rough conversion of it to PDF, so if you felt the topic was vaguely interesting but were put off by having to scroll through loads of my aesthetically-challenged blog screens, then feel free to give the PDF a go. I feel it would have really helped me had I read it a year or so ago, so I thought I'd put it in a more readable format to try and encourage someone else to actually read it. :)

Unfortunately the conversion process didn't improve the actual content of the post, so you'll still have to bear with my insufferable ramblings if you decide to subject yourself to it. ;)

Download as PDF: MovingToScenarioBasedTesting.pdf

Moving to scenario-based unit testing in .NET

2009-06-10T22:29:00.006+10:00

This post is pretty long, even for me. You can download it as a PDF file (roughly converted) which might be a bit easier to read. Unfortunately the conversion process didn't improve the actual content of the post, so you'll still have to bear with my insufferable ramblings if you decide to subject yourself to it. ;)

Download as PDF: MovingToScenarioBasedTesting.pdf

From my observations most developers (myself included) start writing tests using one testcase class per class under test. Due to an unfortunate attribute naming choice, NUnit users may know this as "fixture per class", and implement it as one class marked [TestFixture] holding all the tests that relate to one production class.

I think the reason this approach is so common is because introductory testing and TDD examples tend to focus on smallish utilities like stacks or basic calculators, where fixture setups and object interactions tend not to get out of hand. In my experience, taking the knowledge learned during from basic examples and trying to apply them to Real Life^TM ends in pain, frustration, nausea, alcoholism and, in rare cases, spontaneous human combustion. Even worse, it may cause developers to abandon writing unit tests altogether!

There are many alternatives for organising tests (testcase class per fixture, feature, method, etc.), but the one I've found easiest to adopt and use is a testcase class per scenario. We've been using this at work across two teams for a few months now, and while not perfect it does seem to make writing, reading and maintaining tests relatively easy. In fact, I'm 99% sure that learning to write tests in this way initially (rather than the usual testcase class per class way) would avoid most of the problems we face when first trying to adopt unit testing and test driven development.

The aim of this post is to describe what on earth I'm talking about, as well as how to move from testcase class per class to scenario-based testcase classes, and also outline some of the unresolved problems we've run into with this approach.

A note on terminology

I'm probably drastically misusing terminology here when I talk about scenario-based testing. The way I'm using the term "scenario" seems quite different to scenario-based testing as described in Wikipedia, but I don't know a better name for it. The word "scenario" is used a lot when talking about Behaviour Driven Development (BDD) as well, but I wouldn't be so presumptuous to think this approach qualifies as BDD. It doesn't really seem to match the Dan North or Dave Astels definitions. Instead, I'd appreciate it if we ignore the terminology and just focus the basic ideas -- you can substitute whatever terminology you like! :)

The example

Let's look at the traditional way of introducing testing concepts -- a contrived example. Say we've been contracted by KAOS, an private company with a strong focus on research and development. Their latest project is something they casually refer to as The Doomsday Device. KAOS, being an agile sort of organisation, have given us some simple stories:

The screen should display a welcome message to the agent using the Doomsday Device.
The user can click a button to fire the Doomsday Device
After firing, the user cannot click the button again until the Doomsday Device has finished its firing sequence

Seems fairly easy. We'll use a Model View Presenter style approach so we can test this logic without actually firing the device. Siegfried, the product owner, has assured us that the consequence of any bugs could be quite dire. He stresses "Vee are KAOS agents -- vee don't write bugs here!".

Testcase class per class

We start off by creating a DoomsDayDevicePresenterFixture, and then begin writing tests and filling in passing implementations. Here is what our fixture looks like when we're done:

/* Fixture per class example */
[TestFixture]
public class DoomsDayDevicePresenterFixture {
    private IDoomsDayDeviceView stubView;
    private IDoomsDayDevice stubDoomsDayDevice;

    [SetUp]
    public void SetUp() {
        stubView = MockRepository.GenerateStub<IDoomsDayDeviceView>();
        stubDoomsDayDevice = MockRepository.GenerateStub<IDoomsDayDevice>();
        new DoomsDayDevicePresenter(this.stubView, this.stubDoomsDayDevice);
    }

    private void RaiseLoadedEventOnView() {
        stubView.Raise(view => view.Loaded += null, this, EventArgs.Empty);
    }
    private void RaiseFireButtonPressedOnView() {
        stubView.Raise(view => view.FireButtonPressed += null, this, EventArgs.Empty);
    }
    private void RaiseFiringCompletedOnDevice() {
        stubDoomsDayDevice.Raise(device => device.FiringCompleted += null, this, EventArgs.Empty);
    }

    [Test]
    public void ShouldWelcomeEvilAgentWhenLoaded() {
        const String agentsName = "Siegfried";
        stubDoomsDayDevice
            .Stub(device => device.CurrentOperatorsName())
            .Return(agentsName);
        RaiseLoadedEventOnView();
        Assert.That(stubView.WelcomeMessage, Is.EqualTo("Welcome KAOS Agent " + agentsName + ". What would you like to destroy today?"));
    }

    [Test]
    public void ShouldEnableFireButtonWhenViewLoaded() {
        RaiseLoadedEventOnView();
        Assert.That(stubView.FireButtonEnabled, Is.True);
    }    

    [Test]
    public void FireButtonShouldFireDoomsDayDevice() {
        RaiseLoadedEventOnView();
        RaiseFireButtonPressedOnView();
        stubDoomsDayDevice.AssertWasCalled(device => device.Fire());
    }

    [Test]
    public void ShouldDisableFireButtonWhileFiring() {
        RaiseLoadedEventOnView();
        RaiseFireButtonPressedOnView();
        Assert.That(stubView.FireButtonEnabled, Is.False);
    }

    [Test]
    public void ShouldEnableFireButtonWhenFiringCompleted() {
        RaiseLoadedEventOnView();
        RaiseFireButtonPressedOnView();
        RaiseFiringCompletedOnDevice();
        Assert.That(stubView.FireButtonEnabled, Is.True);
    }    
}

Our tests all pass. Hooray!

What's wrong with this?

What's wrong with this testcase class? Well, nothing really I guess. It's manageable enough for now. However when we start adding features we may find the lack of cohesion between the tests causes us pain (at least it always has for me. I'm quite happy to acknowledge I could just be doing it wrong).

The main source of the cohesion problem is the setup used for each test. The testcase class has its own setup method, but the first few lines of each test performs some additional setup. In somes cases, such as the ShouldDisableFireButtonWhileFiring() and FireButtonShouldFireDoomsDayDevice(), this setup code is repeated. These two tests are really asserting two facts about the same situation (firing the device), but you can't easily see that from the test code. Test code can be a great way of communicating how your code works, but this lack of cohesion in the setups used can make it really difficult to parse out this information.

Having setup information all over the place can make the test code hard to maintain. Changes like requiring a dependency to provide new data at a different time can cause large numbers of tests to break, especially when they've ended up with setups that don't truly reflect the contexts they use. Well factored tests can be a great enabler of change. Fragile tests have the opposite effect.

One thing you can't see from this example, and to me one of the most important drawbacks, is the impact of this approach on test driven development. Most people I know who have learned/tried to learn TDD (especially when learning without a mentor) have done some of the examples, decided to apply it to a real project, sat down to write the first test and... have absolutely no idea where to start. Should we first test the presenter has wired up the correct events on the view? Should we construct a presenter and see if its null (don't laugh, I've seen it recommended!)? Knowing what tests to write to drive development was probably my greatest hurdle in learning TDD -- it is natural to keep thinking in terms of how we want objects and methods to work, rather than what we want the design to do, which robs you of the design benefits TDD can provide.

Testcase class per fixture?

The setup required for a series of tests is generally called the test fixture -- basically, all the stuff that needs to be in place for the assertions in a test to apply and run successfully. To make managing these tests easier we could break them out into a series of testcase classes per fixture. But then we end up with another problem -- our fixtures overlap. Let's think this through for a moment and see if we can find out why this could pose a problem.

All the tests start with a call to RaiseLoadedEventOnView(), so we could just whack that in the setup and have a fixture for all our tests the rely on the loaded view. Except for the ShouldWelcomeEvilAgentWhenLoaded(), which needs to stub out a value before we raise the loaded event. We could stub the value in the setup method as well, but then we lose the benefit of knowing which functionality relies on having that value available. Following this path leads to an increase in the amount of setup code, most of which doesn't apply to all the tests. This can end up obscuring what we are actually testing.

If we were going to go by fixtures, what would we name them? We have one fixture that stubs an agent name and uses a loaded view, another that just needs a loaded view, two that share a fixture where we have a loaded view and a pressed button, and another that has a loaded view, pressed button, and a firing completed event. Based on fixture alone it's hard to come up with good names here, which is generally a sign that we have the wrong class break down.

When I first looked at moving from testcase class per class, grouping tests by fixture was my first stop. I found I had trouble finding clean slices through the fixtures that would neatly group everything. Even when I thought I had it right, my next test would require a change to a fixture that invalidated the breakdown I had chosen. It seemed like I was getting closer to a good approach, but it was still missing something. It wasn't until some people much smarter than me (although less bloggish :)) got together and came up with a better solution that everything started to click into place.

Testcase class per scenario

The change in thinking was to stop worrying about specific fixtures and setups and to group things by scenario. Bit anti-climatic I know, but the change in thought process seemed to fix the majority of the problems we were facing. By concentrating on scenario, and how our class under test should behave in that scenario, the fixtures naturally became more cohesive. The delineation between scenario-specific fixture setup and test logic became obvious. Fixtures and tests were easy to name, and trivial to write. For the first time I no longer struggled to come up with what to test next. Instead I just picked a scenario and started asserting facts about how my class should behave in that scenario and what state it should have.

Of course, this really just ends up as another way of breaking things down by fixture, which I've spent the entirety of the last section complaining about. But I've found the change in thinking indispensable in getting this to work. It's very hard to establish up front exactly what fixtures you're going to need (which results in a lot of churning of test code), but it is generally fairly obvious what scenarios your objects need to work under. In a way, the nice fixture break down is a nice side-effect that comes out of having well-structure scenarios.

I'm intending to post later on how to start from scratch using this approach, but seeing we've come this far lets look at the steps we can use to break our current testcase class into scenarios. Hopefully this will illustrate why I like this approach so much.

Refactoring toward scenario-based tests

The first step is to identify some scenarios in our existing testcase class. Any name referring to a time or event is a hint about the scenario the test relates to. The word "when" is a dead give-away. Scanning through our current tests I can see two that relate to when the view is loaded (ShouldWelcomeEvilAgentWhenLoaded() and ShouldEnableFireButtonWhenViewLoaded()). We also have ShouldDisableFireButtonWhileFiring(). When is the device firing? Looking at the code in that test, it is when the fire button is pressed. The FireButtonShouldFireDoomsDayDevice() also seems to relate to this scenario. And lastly we have ShouldEnableFireButtonWhenFiringCompleted() -- so when firing completed sounds like a good scenario for that one. (You don't have to identify all the scenarios up front, just enough to get you started.)

The second step is to pick one of the identified scenarios and create a testcase class for it. We can then proceed in a couple of ways. One way is to start moving tests one at a time into our new file that relate to the new scenario. Another is to copy and paste the entirety of the old testcase class into our new file, and start removing things that don't relate to the scenario. We repeat this until all our old test cases are in scenarios.

The final step is to factor out the duplication between our testcase scenarios into helper methods in a common base class or two. You can probably get away with doing this as you go, but I'd advise against doing it too prematurely. Wait until the code in two or three scenarios is screaming to be consolidated, and then do it. Otherwise you can wind up extracting commonalities that aren't really there, and we go back to losing cohesion between our setups and tests.

Let's have a look at the when view is loaded scenario. I'll create a new subfolder in our test project called DoomsDayDevicePresenterScenarios, then create a WhenViewLoaded.cs class. I'm quite a fan of underscore-overload (cue shameless post plug), so I'd prefer to use When_view_loaded, but I'll try and skip potentially heated debates for now. :)

The code below is how it looks after refactoring out some duplication:

namespace DaveSquared.Kaos.Tests.DoomsDayDevicePresenterScenarios {
    public partial class DoomsDayDevicePresenterScenario {
        [TestFixture]
        public class WhenViewLoaded : ScenarioBase {
            const string AgentsName = "Siegfried";

            [SetUp]
            public void SetUp() {
                CreateDoomsDayDevicePresenterAndDependencies();
                stubDoomsDayDevice.Stub(device => device.CurrentOperatorsName()).Return(AgentsName);
                RaiseLoadedEventOnView();
            }

            [Test]
            public void ShouldWelcomeEvilAgent() {
                Assert.That(stubView.WelcomeMessage, Is.EqualTo("Welcome KAOS Agent " + AgentsName + ". What would you like to destroy today?"));
            }

            [Test]
            public void ShouldEnableFireButton() {
                Assert.That(stubView.FireButtonEnabled, Is.True);
            }
        }
    }
}

Look how beautifully simple those tests are -- both one line assertions. And it reads nicely too: when view loaded, should welcome evil agent, and should enable fire button. You'll notice I've committed one of the sins I mentioned in my rantings about testcase class per fixture -- I've stubbed out a value in our setup that doesn't relate to all our tests. There are a couple of reasons that I don't care in this case. First, our setup focuses on what is required for this scenario, and the current agent's name happens to be important to this scenario, even if not to all the tests. I'm still getting good cohesion vibes. Second, I can use constants and properties with decent names to make sure the expected state of the scenario is clear. Thirdly, it just doesn't seem to cause me any troubles in real life, unlike when I tried breaking things down by fixture alone.

You may have noticed a weird partial class thingoe happening here. This was suggested by a colleague to make scenarios more discoverable from within Resharper. If all scenarios related to our presenter are inner classes of the DoomsDayDevicePresenterScenario partial class, then Resharper's "go to type" command (Ctrl + T on my configuration) will let us select the partial class, then list all the scenarios for us to jump to. It adds some code noise, and isn't strictly necessary, but does make navigating around your tests easier.

Let's move on to our next scenario.

public partial class DoomsDayDevicePresenterScenario {
    [TestFixture]
    public class WhenFireButtonPressed : ScenarioBase {
        [SetUp]
        public void SetUp() {
            CreateDoomsDayDevicePresenterAndDependencies();
            RaiseLoadedEventOnView();
            RaiseFireButtonPressedOnView();
        }

        [Test]
        public void ShouldFireDevice() {
            stubDoomsDayDevice.AssertWasCalled(device => device.Fire());
        }

        [Test]
        public void ShouldDisableFireButton() {
            Assert.That(stubView.FireButtonEnabled, Is.False);
        }
    }
}

Compare this to how one of these tests used to look.

/* plus [SetUp] method code */
[Test]
public void FireButtonShouldFireDoomsDayDevice() {
    RaiseLoadedEventOnView();
    RaiseFireButtonPressedOnView();
    stubDoomsDayDevice.AssertWasCalled(device => device.Fire());
}

Even though this is a trivial example, the new version is much clearer and cleaner. You'll have to take my word for it, but this applies even as you get to more complicated, real code. You tend to push more into the scenario setup (which is exactly what the setup is for) and the tests themselves stay trivial.

Let's look at the final scenario, and at our base class which we have used to keep duplication in check (although not eliminated, as we'll discuss later).

[TestFixture]
public class WhenFiringCompleted : ScenarioBase {
    [SetUp]
    public void SetUp() {
        CreateDoomsDayDevicePresenterAndDependencies();
        RaiseLoadedEventOnView();
        RaiseFireButtonPressedOnView();
        RaiseFiringCompletedOnDevice();
    }

    [Test]
    public void ShouldEnableFireButton() {
        Assert.That(stubView.FireButtonEnabled, Is.True);
    }
}

public class ScenarioBase {
    protected IDoomsDayDeviceView stubView;
    protected IDoomsDayDevice stubDoomsDayDevice;

    protected void CreateDoomsDayDevicePresenterAndDependencies() {
        stubView = MockRepository.GenerateStub<IDoomsDayDeviceView>();
        stubDoomsDayDevice = MockRepository.GenerateStub<IDoomsDayDevice>();
        new DoomsDayDevicePresenter(this.stubView, this.stubDoomsDayDevice);
    }
    protected void RaiseLoadedEventOnView() {
        stubView.Raise(view => view.Loaded += null, this, EventArgs.Empty);
    }
    protected void RaiseFireButtonPressedOnView() {
        stubView.Raise(view => view.FireButtonPressed += null, this, EventArgs.Empty);
    }
    protected void RaiseFiringCompletedOnDevice() {
        stubDoomsDayDevice.Raise(device => device.FiringCompleted += null, this, EventArgs.Empty);
    }
}

Issues with this approach

Duplication in setup methods

All the [SetUp] methods in our scenarios have some duplication -- creating the subject under test and raising basic events. Why don't we factor all this into the base class? There's a few points to consider here. First, do we really want to hide this duplicated setup code? Scott Bellware questions this under the subheading Reuse: Friend or Foe in an article on BDD for CoDe Magazine:

"Specification code is intended to document the behaviors of the system, and this often means leaving duplicated code in-place to support the learnability of the specs... If you do move common context code to a base class, do so with care for how you’re impacting the learning experience of the code"

Having the setup steps repeated to a degree in each scenario (although with the logic behind each step encapsulated as methods in the base class) is great for documenting the scenario, and also helpful with scenario's with fixtures that diverge from the more standard cases (an example of a divergent fixture is one that tests exceptions thrown from a constructor, so we can't have a constructed object setup already).

On the flip side, developers have good reason to be wary of duplication, having Don't Repeat Yourself drummed into them from the moment they are first exposed to inheritance. You can do some really clever things in terms of context base classes. One of the cleverest of these I've seen is JP Boodhoo's developwithpassion.bdd approach, which has the downside of looking completely foreign to people who speak C#, but has the upside of being really nice once you get used to it.

If the duplication bothers you, you can have a general context base class with an existing [SetUp] method that provides hooks like BeforeSetup(), AfterDependenciesCreated() etc. This lets your scenarios hook into whichever parts of the setup chain it needs to and do things like stub out calls. The real trick comes when you want to chain together contexts, which is something JP's approach gives you, and something which you get for free with tools like rspec (using nested blocks).

This is something the purist in me is still questioning, but in practice we've just been leaving the duplication in and have had little to no problems with it, mainly because the duplication is contained to a small, related area, and because the logic itself is neatly encapsulated in a base class.

What goes into the base class?

Anything that can conceivably apply to every scenario. Fields for dependencies, methods to create a subject under test, common operations like raising events etc. You can eliminate some of this by using an automocker, but this can hide design smells like your SUT having too many dependencies.

Tests that apply to multiple scenarios

Sometimes you'll come across a test that should really apply across all the scenarios, an invariant if you will. There's a few approaches for dealing with this. You can put the test in the base class, and it will automatically run against all inheriting scenarios. I hate this approach -- it means you can get multiple failures from one problem, and I don't like the lack of clarity you get with tests being pulled in mysteriously from a base class. You can also create a specific scenario for these things, calling it Always, or if you like the when-convention, WhenEver :).

I really don't like either of these approaches. I've become increasingly suspicious of this actually being a test smell. If you have logic that doesn't depend on a specific scenario, then this could be a cue to extract this into another class to isolate it properly for testing. You can then check the subject under test uses the other class correctly, without worrying about the behaviour changing across multiple scenarios.

We've used all three approaches, with my preference being for isolating the behaviour properly. The WhenEver style approach feels a bit dirty but hasn't caused much grief. The inherited test thing has had mixed results.

Using in conjunction with other organisational methods

Sometimes you end up sprouting classes or writing utilities that don't seem to have a home or fit a particular scenario. In these cases falling back to lumping everything in the one fixture can work just fine. As this post tried to show, it's pretty easy to move to scenario-based testcase classes in future once some scenarios start emerging or when the single fixture starts giving you grief.

Conclusion

Writing tests around scenarios is a way organising your tests which, in my opinion, makes test code easier to write, helps identify common fixtures, and improves the readability and maintainability of tests. Writing testcase classes per scenario ends up breaking down tests by common fixture, but focusing on the scenario makes this breakdown occur more naturally and more maintainably. It is also pretty straight forward to refactor existing tests into this format.

If anyone actually made it this far, I'd love to hear any comments you have on this, both so I can get valuable feedback, and also to lavish you with compliments for actually getting through this epic :). This technique has really dramatically improved how I practice TDD and approach testing, so hopefully it will also help someone else out there.

Dynamic disappointment

2009-06-03T00:35:00.004+10:00

I've been eagerly awaiting .NET 4 and the new dynamic feature in C#, but after taking the beta for a spin I've run in to a major disappointment. Let's take dynamic for a quick spin and see what's got me so devastated.

public class Dynamic : DynamicObject {
    Dictionary<String, object> members = new Dictionary<string, object>();
    public override bool TrySetMember(SetMemberBinder binder, object value) {            
        members[binder.Name] = value;
        return true;
    }
    public override bool TryGetMember(GetMemberBinder binder, out object result) {
        return members.TryGetValue(binder.Name, out result);
    }
}

[TestFixture]
public class Disappointment {
    [Test]
    public void CanCreateAndCallAMethod() {
        dynamic anObject = new Dynamic();
        anObject.AMethod = new Func<int>(() => 1);
        Assert.That(anObject.AMethod(), Is.EqualTo(1));
    }
}

Here we've created a sub-class of DynamicObject, which lets us play in the world of dynamic lookups from C#. We override TrySetMember(...) and TryGetMember(...) to use a dictionary as a backing store for members. Our test assigns a method called AMethod to a dynamic object at run time, then executes it. It passes! Awesome!

If it walks and quacks like a duck, too bad!

Let's declare an ICanAdd interface, as well as a class that uses objects that support the ICanAdd interface to, well, add stuff.

public interface ICanAdd {
    int Add(int a, int b);
}

public class SomethingThatAdds {
    private ICanAdd adder;
    public SomethingThatAdds(ICanAdd adder) {
        this.adder = adder;
    }
    public int FirstNumber { get; set; }
    public int SecondNumber { get; set; }
    public int AddNumbers() {
        return adder.Add(FirstNumber, SecondNumber);
    }
}

We can add this method to our dynamic object so that it supports the same operations as the ICanAdd interface:

[Test]
public void CanCreateADynamicAdder() {
    dynamic adder = new Dynamic();
    adder.Add = new Func<int, int, int>((first, second) => first + second);
    Assert.That(adder.Add(1, 3), Is.EqualTo(4));
}

This works fine, but when we try to combine static and dynamic worlds we run into problems:

[Test]
public void CannotUseDynamicAdderForAnythingUseful() {
    dynamic adder = new Dynamic();
    adder.Add = new Func<int, int, int>((first, second) => first + second);
    var somethingThatCanAdd = new SomethingThatAdds(adder); /* Fails here at runtime */
    somethingThatCanAdd.FirstNumber = 10;
    somethingThatCanAdd.SecondNumber = 20;
    Assert.That(somethingThatCanAdd.AddNumbers(), Is.EqualTo(30));
}

This compiles, but at runtime we get the test failing with the following RuntimeBinderException:

DaveSquared.DynamicDisappointment.Disappointment.CannotUseDynamicAdderForAnythingUseful:
Microsoft.CSharp.RuntimeBinder.RuntimeBinderException : 
The best overloaded method match for 'DaveSquared.DynamicDisappointment.SomethingThatAdds.SomethingThatAdds(DaveSquared.DynamicDisappointment.ICanAdd)' has some invalid arguments
  at CallSite.Target(Closure , CallSite , Type , Object )
  at System.Dynamic.UpdateDelegates.UpdateAndExecute2[T0,T1,TRet](CallSite site, T0 arg0, T1 arg1)
  at DaveSquared.DynamicDisappointment.Disappointment.CannotUseDynamicAdderForAnythingUseful() ...

The exception is fairly clear -- the C# RuntimeBinder is trying to call the SomethingThatAdds(ICanAdd) constructor, but we've given it a dynamic instance instead. Based on my fairly primitive understanding of this stuff, in order to integrate dynamic lookups into the statically typed CLR, dynamic is actually implemented as a static type. So even though we're using dynamic member lookups at runtime, we still need to abide by the type system and pass methods the static types they expect.

If we modify our original SomethingThatAdds class to explicitly accept the dynamic type then our last test passes:

public class SomethingThatAdds {
    private dynamic adder;
    public SomethingThatAdds(dynamic adder) {
        this.adder = adder;
    }
    /* ... snip ... */
}

I'm aware I'm probably expecting too much, but having to explicitly modify our code in order to make this kind of use of the dynamic feature for duck typing is, well, disappointing. It would be great to see something like Jon Skeet's dynamic<T> idea get into the final release so we can get the best of both worlds. :)

Setting up Git difftool on Windows

2009-05-20T10:56:00.000+10:00

After installing Windows 7 I had trouble getting git diff to work properly with diffmerge. In fact, any external tool I tried to define would hang PowerShell once it exited. Unfortunately in the time I had to look at it I couldn't figure out if this was due to a new version of PowerShell with Windows 7, the new version of MSys Git I had installed, or something strange after switching to x64 architecture. What I did have time for was to find a workaround using git difftool, which is a new command added to Git as of version 1.6.3.

The workaround is essential just a tweak of the steps we used previously to get git diff working, so its probably easiest to have a quick skim through that before going through this.

Configuration tweaks

First alteration to the previous approach was to create a difftool wrapper for Diffmerge. I called this git-diff-diffmerge-wrapper.sh and put it in my Git/cmd directory (which is also on my PATH).

#!/bin/sh
"C:/Program Files (x86)/SourceGear/DiffMerge/DiffMerge.exe" "$1" "$2" | cat

Next step was to update .gitconfig to disable the previously added external tool setup for git diff, and configure git difftool in its place.

[diff]
 #external = git-diff-wrapper.sh
 tool = diffmerge

[difftool "diffmerge"]
 cmd = git-diff-diffmerge-wrapper.sh "$LOCAL" "$REMOTE"
 
# ... snipping rest of .gitconfig ... #

I've left the external tool commented out (prefixed by #). This syntax is more in line with the way the we configured the git mergetool last time.

Using `git difftool`

Now whenever we type git diff we will get the standard, textual git difference listing. If we want to use the externally configured tool, we need to type git difftool (followed by the usual range of diff options) and follow the prompts instead. I actually like having the ability to switch between a quick diff and firing up an external tool. With this approach we can also configure multiple diff tools and pick whichever one is our favourite at the moment (or just try out a new diff tool). The command also closely parallels that used to bring up an external tool for merging (git mergetool).

I wish I knew what caused my original issue, but at least I've got my graphical diff tools back for Git. :)

Basic .NET builds using Rake

2009-04-02T00:10:00.005+11:00

I was starting up a new hobby project the other night and thought I'd try using Rake to script my build (instead of pure MSBuild, Nant, or just plain VS). Now Ruby and I generally don't get on too well, so I was really surprised with how easy I found it to setup, and even more surprised with how much I enjoyed it. :)

Prerequisites

Um, Ruby. The installer I used gave me Ruby with Rake included. We'll also need a simple .NET solution. I created one with a structure that looked a bit like this:

DaveSquared.SampleProject

build

rakefile.rb

DaveSquared.SampleProject.Tests
DaveSquared.SampleProject.Web
DaveSquared.SampleProject.sln

tools

NUnit

The SLN file contains two projects, DaveSquared.SampleProject.Tests and .Web. The .Tests project references the .Web project, as well as the NUnit framework buried somewhere in the tools/NUnit directory. Both projects where configured to build to the build/output directory (so .Tests will build to build/output/DaveSquared.SampleProject.Tests).

We can now setup our rakefile.rb, which I've plonked into the build directory. We'll run the build from this directory, so we can specify all our paths relative to this build file.

Basic build and test run

After reading Dave Laribee's post on OMG Rake!, and Mark Guzman's excellent post on Building .NET projects with rake, I then proceeded to ruin all their good work by patching bits and pieces of their posts together and got this:

require 'rake/clean'

DOT_NET_PATH = "#{ENV["SystemRoot"]}\\Microsoft.NET\\Framework\\v3.5"
NUNIT_EXE = "../tools/Nunit/bin/nunit-console.exe"
SOURCE_PATH = "../src"
OUTPUT_PATH = "output"
CONFIG = "Debug"
 
CLEAN.include(OUTPUT_PATH)

task :default => ["clean", "build:all"]
 
namespace :build do
  
  task :all => [:compile, :test]
      
  desc "Build solutions using MSBuild"
  task :compile do
    solutions = FileList["#{SOURCE_PATH}/**/*.sln"]
    solutions.each do |solution|
      sh "#{DOT_NET_PATH}/msbuild.exe /p:Configuration=#{CONFIG} #{solution}"
    end
  end
   
  desc "Runs tests with NUnit"
  task :test => [:compile] do
    tests = FileList["#{OUTPUT_PATH}/**/*.Tests.dll"].exclude(/obj\//)
    sh "#{NUNIT_EXE} #{tests} /nologo /xml=#{OUTPUT_PATH}/TestResults.xml"
  end
  
end

Now the good thing about this is that I don't think you need to know much (any? I know virtually none) Ruby to understand what is going on here, or even to make basic modifications to the tasks (although it might be a struggle if you haven't used build tools like make or nant before). But you can bring the full power of the language to bear when you need it. Let's have a quick step through the main parts of the file.

The first line imports rake/clean, which lets us use CLEAN.include(OUTPUT_PATH) to tidy up for us. We've then got loads of constants to specify various paths: the location of .NET tools like msbuild, and the relative paths to NUnit, our source, and our output. Our :default task is set to run clean, then build:all (the => syntax translates to depends on in make terms, so to run the default task rake will make sure its dependencies are run).

If we drop into the :build namespace, we have :all, :compile and :test tasks defined. To :compile, we use the wonderful FileList class built into rake to get all *.sln files in our source directory, then shell out to msbuild to take care of the hardwork of compiling everything. The :test task relies on convention, by finding all *.Tests.dll files and running them through NUnit. We also make sure that :test won't run until :compile has run by setting :compile as a dependency.

Finally, our :all task compiles and tests the build.

Running our rake build

By dropping into our build directory from the command line, just typing rake will pickup our rakefile.rb and execute our default task, which will clean, compile and test our build. We can also run a task at a time, say rake clean, or rake build:test (the :test task is prefixed by build because of its namespace).

There's obviously tonnes we could do to make this nicer (like using Mark's .NET tasks, and/or removing the hard-coded Debug configuration), but hopefully this gives people a quick way to start getting into rake and ruby for building .NET projects.

Explicitly test what you are trying to test

2009-04-02T00:09:00.000+11:00

Had a problem today that initially took me by surprise. The problem was that this test passed:

[Test]
public void EmptyBlocksShouldBeEqual() {
    var first = new Block();
    var second = new Block();
    Assert.That(first, Is.EqualTo(second));
}

Why was this a problem? Because Block is a reference type and I had not overridden Equals(Object o), so we should fall back to the default Object.Equals(Object o) method which compares object references. The references are clearly different, so this test should fail.

To figure out why this test passes we need two pieces of information. First we need to know how Block is defined:

public class Block : IEnumerable<FrameParameters>  { /* ... snip ... */ }

Second, we need to know that the NUnit EqualConstraint implementation has specific handling for IEnumerable classes. So when I called Assert.That(first, Is.EqualTo(second));, this was just comparing the values returned via enumerating both Block instances. This really wasn't what I wanted -- I was trying to test the Equals() implementation. Here's the corrected test:

[Test]
public void EmptyBlocksShouldBeEqual() {
    var first = new Block();
    var second = new Block();
    Assert.That(first.Equals(second));
}

The moral of the story: make sure you explicitly test what you are trying to test. :)

Too LINQy?

2009-03-19T23:30:00.002+11:00

This post contains a tale of two methods. Well, more precisely, a tale of one method implemented in two different ways -- once using LINQ-based functional-fu, and once using old school procedural code.

Recently I was pairing on a task that required us to keep a running average of multiple sets of samples that came through our app. Unfortunately it was more than likely that some sets of samples would be different lengths (depending on exactly when sampling stopped). In these situations where we were missing samples the requirements were to leave the averages for the missing samples unchanged. When we had extra samples, we should use them as the new average at that position. Here's some tests that hopefully show what we were trying to do:

[Test]
public void ShouldAverageFirstTwoLotsOfSamples() {
    var firstSamples = new float[] { 1, 2, 3 };
    var secondSamples = new float[] { 3, 4, 5 };
    var expectedAverages = new float[] {2, 3, 4};
    _averager.AddSamples(firstSamples);
    _averager.AddSamples(secondSamples);
    Assert.That(_averager.GetAverages(), Is.EqualTo(expectedAverages));
}  

[Test]
public void ShouldAddExtraSamplesToAverages() {
    var firstSamples = new float[] { 1, 2, 3 };
    var secondSamples = new float[] { 3, 4, 5, 2 };
    var expectedAverages = new float[] { 2, 3, 4, 2 };    
    /* ... snip ... */
}

[Test]
public void ShouldHandleShorterNumberOfSamples() {
    var firstSamples = new float[] { 1, 2, 3 };
    var secondSamples = new float[] { 3, 4 };
    var expectedAverages = new float[] { 2, 3, 3};
    /* ... snip ... */
}

A LINQ implementation

After a brief flurry of for looping, we decided to muck around with LINQ to filter and transform the sets of data in a pseudo-functional kind of way.

public class AverageCalculator {
    private float[] _averages = new float[0];
    private uint _numberOfAverages;    

    public void AddSamples(float[] samples) {
        _numberOfAverages++;
        var numberOfNewSamples = samples.Length;
        var numberOfSamplesInLastAverage = _averages.Length;
        var leftOverSamples = samples.Skip(numberOfSamplesInLastAverage);
        var leftOverAverages = _averages.Skip(numberOfNewSamples);

        _averages = _averages
            .Take(numberOfNewSamples)
            .Select(
                (average, sampleIndex) => CalculateNewAverage(average, samples[sampleIndex], _numberOfAverages)
            )
            .Concat(leftOverAverages)
            .Concat(leftOverSamples)
            .ToArray();
    }

    private float CalculateNewAverage(float oldAverage, float newSample, uint totalSamples) {
        return oldAverage + (newSample - oldAverage) / totalSamples;
    }

    public float[] GetAverages() {
        return _averages;
    }
}

Stepping through the logic, we take a maximum of numberOfNewSamples from the running _averages, then calculate the new averages based on each new sample. To handle the possibility of mismatched array sizes, we concatenate any left over items from each array. In reality, one of these arrays of left overs will be empty (depending on which array is larger).

My first thought once the tests went green was "wow that's evil!", but compared with the procedural approach we started with, this one really began to grow on me. It was surprisingly easy to write, but I was concerned about its readability (initially we had the local variables in AddSamples(...) inlined, but we extracted them out to try and make it more readable). We decided to test out the procedural equivalent and see if that was any clearer.

A procedural implementation

public void AddSamples(float[] samples) {
    _numberOfAverages++;

    var largestArray = (samples.Length >= _averages.Length) ? samples : _averages;
    var smallestArray = (samples.Length >= _averages.Length) ? _averages : samples;

    var newAverages = new float[largestArray.Length];
    for (int i = 0; i < newAverages.Length; i++) {
        newAverages[i] = (i < smallestArray.Length) 
                            ? CalculateNewAverage(_averages[i], samples[i], _numberOfAverages) 
                            : largestArray[i];
    }

    _averages = newAverages;
}

The logic used here is to find which array is largest, and to create a new array of that size. We loop through every possible index, calculating the average until all of the smallest array is used, then append the left overs from the largest array. This seems quite neat to me, although I should mention that this is a refactored, sanitised version (as is the LINQ version). The initial implementation was more verbose and the logic less clear, and it somehow managed to take longer to get it to a state where the tests all passed.

Who's right?

Which approach do you like best? It probably comes down to how much imperative vs. functional programming you've done. (Or you hate both versions of the method, in which case please leave a comment with the correct approach. :)). Imperative programming concentrates on telling the computer how to do something, while functional is more about telling the computer what to do. For example, our LINQ version starts with some data and specifies what transformations we want to make to it. Our second version of the code focuses more on the mechanics -- create an array, loop, check the bounds etc.

The second version's focus on implementation makes it fairly easy to mentally trace through how it works, but how clear is the intention behind the implementation? The LINQ version probably takes a bit more effort to understand how it works (especially as the first exposure most people have to programming tends to be to imperative-style control structures like IF, FOR, WHILE and even GOTO), but what it is doing might be a little clearer.

Overall, I kind of prefer the LINQ version for its faint hint of functional elegance, but on the other hand the procedural version is just so darn familiar and comfortable to read for a C#/Java/C person like me. I'd love to hear any thoughts you have on these approaches, and how you are handling the encroachment of functional concepts into our formerly purely-procedural C# language.

dave^2 = -1

Favour test driving logic over data

Cue long-winded example...

Using data-based tests for a file name generator

What is this class really doing?

Introducing dependencies

Testing logic over testing data

What have we achieved?

When we are stuck with testing data...

Conclusion

Keeping a change out of our master repository with Git

Calculators and a tale of two TDDs - Pt 3: Finishing up our BDD kata

Where should customer delimiter logic go?

Specifying our CalculatorExpressionParser

A delimiter parser

A trivial factory implementation

Please, not another spec! Have mercy!

Post-mortem

Calculators and a tale of two TDDs - Pt 2: BDD-style

Test infrastructure

Designing the Add() method

Building up a context for this to work

Huh?

Passing our specification

Driving out our IAdder implementation

Creating a number parser

What's left?

Calculators and a tale of two TDDs - Pt 1: a traditional approach

First test: empty strings

Test 2: Adding a single number

Summing multiple numbers

Allow commas and newlines as delimiters

Allow a custom delimiter

Where did I go wrong?

Using Privoxy to block net nasties and ads in Chrome [Windows]

Getting started with Privoxy

But I'm already behind a proxy at work! What do I do?

But I'm not always behind another proxy! How can I switch settings?

Is it worth it?

Configuring dependencies' dependencies with StructureMap 2.5.1 [.NET,C#]

A contrived example

Configuration using StructureMap

An example of driving design through top-down testing

Starting out

A single responsibility

Breaking down the responsibility

How does the SUT meet its responsibility?

Where did all these object references come from?

Passing our scenario

Wrap up and next steps

Coding tiger, hidden responsibility

Coding tiger

Testing times

Hidden responsibility

Resassigning responsibilities

Nothin' but .NET, Sydney 2009: Final thoughts

A brave new world

The journey

Nothin' but .NET, Sydney 2009: Day 5

Tell, don't ask

Testing

Other notes from Day 5

Nothin' but .NET, Sydney 2009: Day 4

The day's activities

Writing tests

Today's tidbits

Nothin' but .NET, Sydney 2009: Day 3

Other tidbits

Nothin' but .NET, Sydney 2009: Day 2

Patterns and principles

Miscellaneous stuff

Nothin' but .NET, Sydney 2009: Day 1

Enumerables, LINQ and speed

ReSharper Live Template for Dependency Properties (C#)

Occasional trouble mocking generic methods

Scenario-based unit testing post in PDF format

Moving to scenario-based unit testing in .NET

A note on terminology

The example

Testcase class per class

Designing the `Add()` method

Driving out our `IAdder` implementation

Using `git difftool`