Showing posts with label refactor. Show all posts
Showing posts with label refactor. Show all posts

Sunday, 16 October 2011

Wiping the state clean: static stateful classes in NUnit tests

I’m automating some tests that are currently run manually in a batch script that spins up the same executable multiple times, with varying options. As always, I’d like to get the tests started without modifying existing code, and preferably without adding any new code.

Unfortunately the application poses a few problems. The entry class has a curiously-implemented singleton behaviour to ensure that only one is running in a process. It also keeps a static ref to a set of command line options, but doesn’t give the client any way of clearing those options to restart processing. I also need to inject some behaviour that would allow the test to sense that the correct thing had been done. The class looks a bit like this:

namespace AppDomainInNUnit
{
    internal class BaseApplication
    {
        public static BaseApplication Instance = null;

        public BaseApplication()
        {
            if(Instance == null)
            {
                Instance = this;
            }
            else
            {
                throw new InvalidOperationException("ctor fail");
            }
    }

    class Application : BaseApplication
    {
        static int Main(string[] args)
        {
            var app = new Application();
            return app.Run();
        }

        int Run()
        {
            _options.Add("option", "added");
            foreach(var option in _options.Keys)
                Console.WriteLine("{0} {1}", option, _options[option]);
            return 0;
        }

        public static Action<string> PostFinalStatus = s => Console.WriteLine(s);

        private static readonly Dictionary<string, string> _options = new Dictionary<string, string>();
    }
}

I could add methods to the existing code to clear the options and to handle the singleton behaviour, but that means changes. In principle there’s no need, as I can instantiate the classes I need in a separate AppDomain, and unload that AppDomain (and dump any static state) when I’m done.

To get this under test I need to do several things. First, I need to create a facade for the Application class that can be marshalled across AppDomain boundaries. Second, I need to consistently create and unload AppDomains in NUnit tests – I want to unload at the end of each test to dump the class’ static state. Finally, I want to inject some behaviour for the PostFinalStatus delegate that will allow the test to sense the state posted by the class.

So, first I need a facade for Application. This is partly because I need to create a class that can be marshalled across AppDomain boundaries. To use marshal-by-reference semantics, and have the instance execute in the new AppDomain, it’ll need to inherit MarshalByRefObject. Unfortunately I need to break the no-changes constraint, and add an internal hook to allow the facade to kick the Application class off.

The TestMain bootstrap class is a basically a copy of the existing Main( … ):

        internal static int TestMain(string[] args)
        {
            return Main(args);
        }

Then I have a simple Facade:

    public class Facade : MarshalByRefObject
    {
        public Facade()
        {
            Console.WriteLine("Facade default ctor in {0}", 
                        Thread.GetDomain().FriendlyName);
        }

        public void Run(string[] args)
        {
            Console.WriteLine("Calling to {0}.", 
                        Thread.GetDomain().FriendlyName);
            
            Application.TestMain(args);
        }
    }

Creating an AppDomain in an NUnit test is fairly straightforward, although it confused me for a while until I realised that the tests are run under a different application – in my case either the JetBrains TestRunner, or nunit-console. In either case the code is not actually available under the executable’s application base directory. The solution is to explicitly set the application base directory for the new test AppDomain:

        [Test]
        public void Test()
        {
            var callingDomain = Thread.GetDomain();
            var callingDomainName = callingDomain.FriendlyName;
            
            Console.WriteLine("{0}\n{1}\n{2}", 
                callingDomainName, 
                callingDomain.SetupInformation.ApplicationBase, 
                callingDomain.SetupInformation.PrivateBinPath);

            var domain = AppDomain.CreateDomain("test-domain", null, null);
            
            Console.WriteLine("{0}\n{1}\n{2}", 
                domain.FriendlyName, 
                domain.SetupInformation.ApplicationBase, 
                domain.SetupInformation.PrivateBinPath);
            
            AppDomain.Unload(domain);

            var setup = new AppDomainSetup() { 
                ApplicationBase = callingDomain.SetupInformation.ApplicationBase };
            
            domain = AppDomain.CreateDomain("test-domain", null, setup);
            
            Console.WriteLine("{0}\n{1}\n{2}", 
                domain.FriendlyName, 
                domain.SetupInformation.ApplicationBase, 
                domain.SetupInformation.PrivateBinPath);

            var assembly = Assembly.GetExecutingAssembly().CodeBase;

            var facade = domain.CreateInstanceFromAndUnwrap(
                assembly, 
                "AppDomainInNUnit.Facade") as Facade;
            
            facade.Run(new [] { "some", "args" });

            AppDomain.Unload(domain);
        }

There’s a lot of cruft in there, but the intent is to show which AppDomain the class is actually being instantiated in, for clarity.

Finally, I want my tests to be informed of the Application’s final status when it posts it. This is tricky; I’ve been able to do it by passing in a delegate that sets some property data on the test AppDomain, then querying the AppDomain property data during the test assert stage. First the facade needs a method that allows me to set the post behaviour for Application:

        public void SetPostBehaviour(Action<string> behaviour)
        {
            Application.PostFinalStatus = behaviour;
        }

Then I need to pass a delegate with the right behaviour in during the test, and assert on the value of that data before unloading the test AppDomain:

            ...
            var assembly = Assembly.GetExecutingAssembly().CodeBase;

            var facade = domain.CreateInstanceFromAndUnwrap(
                assembly, 
                "AppDomainInNUnit.Facade") as Facade;
            
            var posted = false;
            facade.SetPostBehaviour(s => Thread.GetDomain().SetData("posted", true));
            
            facade.Run(new [] { "some", "args" });

            Assert.IsTrue((bool)domain.GetData("posted"));
            ...

So, it's possible to work around these static classes without too many intrusive changes to the existing code. Generally:

  1. To instantiate and execute a class in another AppDomain the class should derive from MarshalByRefObject (see Richter, CLR via C#). You’ll get a proxy to the instance in the new domain.
  2. Don’t tag with Serializable instead of deriving from MarshalByRefObject – you’ll get a copy of the instance in the other domain, and will still run in the original domain.
  3. You can, but you shouldn’t invoke static methods or fields on your class that derives from MarshalRefByObject. It won’t give you the effect you expect: all static calls are made in the calling domain, not the separate domain. To make calls on static methods you need to create a facade or adapter that can be marshalled across the domain boundary, and have that make the static calls for you.
  4. In the various CreateInstance… methods on AppDomain remember to use a fully qualified type name for the class you want to instantiate. This generally isn’t clear from examples like that in Richter, who dump all their classes in the global namespace (tsk).
  5. If you’re creating an AppDomain in an NUnit test, remember that you might need to set the ApplicationBase for the domain to the directory holding your test code.
  6. I wanted to get AppDomain.ExecuteAssembly(string, string[]) working, which would mean that I wouldn’t need a TestMain hook in the existing code. I’ve not managed to get it working yet, though. In any event, I’d still need a facade that I could marshal across the boundary in order to set the status post behaviour.

Friday, 7 October 2011

Using static classes in tests

This is a followup from my previous post on mocking out static classes and injecting behaviour.

The disadvantage to injecting test specific behaviour into a static class is that if you’re not careful you can leak behaviour from test to test. This certainly happens in NUnit, in which a static class will be instantiated once for a test session – not once per test instance.

Take these psuedo-tests for example:

[Test]
public void FirstTest()
{
    IO.Abstractions.Path.CombineImpl = (path1, path2) => "some\\default\\path";

    Assert.AreEqual("some\\default\\path", IO.Abstractions.Path.Combine("some", "path"));
}

[Test]
public void SecondTest()
{
    // assume we're using the default implementation -- System.IO.Path.Combine

    Assert.AreEqual("some\\path", IO.Abstractions.Path.Combine("some", "path"));
}

FirstTest will succeed. If FirstTest runs first, then SecondTest will fail; if SecondTest runs first, SecondTest will succeed. the problem is that FirstTest sets the behaviour for Path.Combine and SecondTest doesn't override it.

The trick is to ensure you reset the behaviour in test setup and teardown. You can just reset to default behaviour in either:

[SetUp}
public void SetUp()
{
    IO.Abstractions.Path.CombineImpl = System.IO.Path.Combine;
}

or you might want to set the behaviour to null, and just set the defaults on teardown:

[SetUp}
public void SetUp()
{
    IO.Abstractions.Path.CombineImpl = null;
}

[TearDown}
public void TearDown()
{
    IO.Abstractions.Path.CombineImpl = System.IO.Path.Combine;
}

This latter approach has the advantage that -- if you null out all possible behaviour across the static classes used -- you immediately get to sense which methods are being hit in any given test, and where in the code under test it's being hit from.

Sunday, 25 September 2011

Injecting behaviour for static method calls (filesystem mocking redux)

This is a direct evolution of my last post, which dealt with decisions faced in testing code that hits the filesystem. The solution I posted was a bit clunky, and inconsistent; this is better, and works generally if you’re faced with a static method whose behaviour you want to change but can’t, immediately.

A lot of the following process has been informed by Michael Feathers’ “Working Effectively With Legacy Code”. Go read it if you haven’t.

So, imagine you have this idealized legacy code:

using System.IO;
using System.Xml.Linq;

namespace AbstractionExamples
{
    public static class TradeFileLoader
    {
        public static ITrade Load(string rootFolder, string file)
        {
            ITrade trade = null;
            var filePath = Path.Combine(rootFolder, file + ".trade");
            if (File.Exists(filePath))
            {
                trade = Trade.Load(filePath);
            }
            return trade;
        }
    }

    public class Trade : ITrade
    {
        public static ITrade Load(string file)
        {
            var xml = XDocument.Load(file);
            return new Trade(xml);
        }

        public Trade(XDocument xml) { /* ... */ }
    }
}

I want to get it under test, changing as little of the existing code as possible in the process.

The first thing to look at is the use of Path, and then File, as they can both be treated the same way. Path and File are static, and I don’t have the code; this means I can't just derive from them. What I want to do is create a class that provides a pass-through to the existing IO code, without modifying any parameters or behaviour in flight by default.

So, I mimic the existing Path class in a new namespace. In the new class I provide delegates for desired behaviour that defaults to the original System.IO behaviour. First I write a few simple tests like:

    [TestFixture]
    public class Tests
    {
        [Test]
        public void PathTest()
        {
            var path = @"root\file";

            var actual = IO.Abstractions.Path.Combine("root", "file");

            Assert.AreEqual(path, actual);
        }
    }

and then the new classes:

using System;
using System.IO;

namespace IO.Abstractions
{
    public static class Path
    {
        public static Func<string, string, string> CombineImpl = Path.Combine;

        public static string Combine(string first, string second)
        {
            return CombineImpl(first, second);
        }
    }

    public static class File
    {
        public static Func<string, bool> ExistsImpl = File.Exists;

        public static bool Exists(string file)
        {
            return ExistsImpl(file);
        }
    }
}

Now I can introduce these new classes along this seam by just swapping the namespace:

//using System.IO;  // no longer needed
using System.Xml.Linq;
using IO.Abstractions;

namespace AbstractionExamples
{
    public static class TradeFileLoader
    {
        public static ITrade Load(string rootFolder, string file)
        {
            ITrade trade = null;
            var filePath = Path.Combine(rootFolder, file + ".trade");
            if (File.Exists(filePath))
            {
                trade = Trade.Load(filePath);
            }
            return trade;
        }
    }

    public class Trade : ITrade
    {
        public static ITrade Load(string file)
        {
            var xml = XDocument.Load(file);
            return new Trade(xml);
        }

        public Trade(XDocument xml) { /* ... */ }
    }
}

The Trade-related changes are slightly more complicated – because we want to replace XDocument. XDocument is not a static class, and we do actually use instances of it.

If I want to avoid all changes to the existing code then I’ll need to turn my as-yet unwritten IO.Abstractions.XDocument into a proxy for System.Xml.Linq.XDocument. Unfortunately System.Xml.Linq.XDocument doesn’t implement an interface directly; fortunately it isn’t sealed, so I can derive from it. Even better, System.Xml.Linq.XDocument also provides a copy constructor so I don’t need to wrap an instance and present the correct methods myself. I just need to derive from System.Xml.Linq.XDocument, and construct using the copy constructor.

namespace Xml.Abstractions
{
    public class XDocument : System.Xml.Linq.XDocument
    {
        public XDocument(System.Xml.Linq.XDocument baseDocument) : base(baseDocument)
        {
        }

        public static Func<string, XDocument> LoadImpl = 
            f => new XDocument(System.Xml.Linq.XDocument.Load(f));

        public new static XDocument Load(string file)
        {
            return LoadImpl(file);
        }
    }
}

I’m a little concerned with the amount of work going on in construction here; I’ll have to take a look at how the copy constructor behaves.

Finally I can introduce the new XDocument along the seam in TradeFileLoader, so we end up with exactly what we started with, except for the changed namespace directives:

//using System.IO;
//using System.Xml;
using IO.Abstractions;
using Xml.Abstractions;

namespace AbstractionExamples
{
    public static class TradeFileLoader
    {
        public static ITrade Load(string rootFolder, string file)
        {
            ITrade trade = null;
            var filePath = Path.Combine(rootFolder, file + ".trade");
            if (File.Exists(filePath))
            {
                trade = Trade.Load(filePath);
            }
            return trade;
        }
    }

    public class Trade : ITrade
    {
        public static ITrade Load(string file)
        {
            var xml = XDocument.Load(file);
            return new Trade(xml); 
        }

        public Trade(XDocument xml) { /* ... */ } 
    }
}

So, the process I've gone through when faced with these is:

  1. Trace static calls until you run out of code you own (e.g. to Path.Combine, XDocument.Load).
  2. Create an abstracted class in a new namespace that mimics the existing static interface.
  3. Provide the new class with delegated behaviour that defaults to passing through to the original.
  4. (And naturally you'll have tests for this new code to verify that).
  5. If the class you’re overriding is not static, derive from the original class and hope that there’s a copy constructor, or be ready for potentially lots more work.
  6. Swap in new behaviour on the seam by just replacing old namespace directives (using System.IO, &c) with the new.

Finally, if I want to test the new class I just need to inject my desired test behaviour into Path, File and XDocument -- after adding a method to ITrade and Trade to return the root element of the trade XML. It turns out that the arrange, act, assert flow looks quite natural:

namespace AbstractionExamples.Tests
{
    [TestFixture]
    public class Tests
    {
        [Test]
        public void ReturnsValidTrade_WhenGivenAValidFile()
        {
            File.ExistsImpl = 
                f => { if (f.Equals(@"root\file.trade")) return true; return false; };
            XDocument.LoadImpl = 
                f =>
		{
                    var doc = new XDocument(
                        System.Xml.Linq.XDocument.Parse("<root></root>"));
                    return doc;
                };

            var trade = TradeFileLoader.Load("root", "file");

            Assert.AreEqual("root", trade.Root.LocalName);
    }
}

Friday, 28 January 2011

Mocks and dependencies

On rationalizing class dependencies -- John Sonmez is exploring a number of these ideas at the moment, and is building a useful list of patterns for dealing with common scenarios.

Thursday, 27 January 2011

First, check your dependencies; avoid breaking encapsulation

Recently I was bogged down attempting to mock out half the world when testing a method, only to find that a little thought would have made it irrelevant.

You don’t have to mock to isolate – sometimes you just need to understand your dependencies better.

This example is a case in point. I went down a couple of blind alleys because I didn’t consider the coupling between the method under test and other classes. It’s this one:

public void MethodUnderTest()
{
    UtilityClassProvider.Get<UtilityClass>().SomeUsefulMethod();
}

In this abstracted example the problem’s pretty easy to spot. I started mocking a UtilityClassProvider and a UtilityClass, which was all rather more involved than necessary because of the way UtilityClassProvider instantiates a class, and because UtilityClass was sealed, and couldn’t be unsealed. Rhino Mocks complains if you attempt to mock a sealed class, and I couldn’t extract an interface and use that as UtilityClass needs an instantiatable type to create – and so on.

But then -- UtilityClassProvider looks like it’s meant to be a builder. Surely it could be refactored into one, standardising the code a bit too. Refactoring UtilityClassProvider to a builder pattern would be worthwhile, even if it did mean the refactoring started to affect more than the class under test.

Probably best to limit the refactoring to begin with.

I looked at it and realised that this method didn’t depend on UtilityClassProvider at all. At worst, it relies on a UtilityClass, and at best only on the effects of SomeUsefulMethod. The encapsulation-breaking of UtilityClassProvider.Get<UtilityClass>.SomeUsefulMethod() is simply ugly, and let me lead myself up the garden path.

So, in this case I extracted an IUtilityClass property with public accessors, and extracted an IUtilityClass interface from UtilityClass. This shifts the emphasis for creating (and injecting) the IUtilityClass instance to the calling code.

public sealed class UtilityClass : IUtilityClass
{
    public void SomeUsefulMethod()
    {
    }
}

public interface IUtilityClass
{
    void SomeUsefulMethod();
}

public class ClassUnderTest
{
    public IUtilityClass UtilityClass { get; set; }

    public void MethodUnderTest()
    {
        UtilityClass.SomeUsefulMethod();
    }
}

I could have gone a step further and removed the dependency on UtilityClass, and just extracted a method that proxied SomeUsefulMethod instead – but it would have required a call to an IUtilityClass anyway, and going this far removes the more awkward problems associated with the dependency on UtilityClassProvider.

With either solution I’ve removed direct dependencies from the method under test, meaning that the method is a) simpler and b) easier to isolate. The extracted method(s) represent logic that might be better provided by another object or structure – but that’s another step. I might recover a UtilityClassProvider from a container like Unity, for example.

Finally, now I can test the method either with a mock IUtilityClass or an actual UtilityClass, if the class is amenable to it (in this simple example it is; in real life it wasn’t, so mocking helped). I can then inject the mocked IUtilityClass, call MethodUnderTest and voila.

[TestFixture]
public class Tests
{
    [Test]
    public void Test()
    {
        var mockUtilityClass = MockRepository
            .GenerateMock<IUtilityClass>();
        mockUtilityClass
            .Expect(o => o.SomeUsefulMethod());
        
        var cut = new ClassUnderTest();
        cut.UtilityClass = mockUtilityClass;

        cut.MethodUnderTest();

        mockUtilityClass.VerifyAllExpectations();
    }
}

So, the moral is: if you find yourself refactoring half the world just so you can then mock it – check your dependencies first, as you might not be dependent on half of what you think you are.

Mocking dependencies

Given the following UtilityClassProvider and UtilityClass classes, what are suitable ways of mocking out the call to SomeUsefulMethod() and isolating the MethodUnderTest()? Wrinkle: UtilityClass needs to stay sealed.

public class UtilityClassProvider
{
    public T Get<T>() where T : new()
    {
        return new T();
    }
}

public sealed class UtilityClass
{
    public void SomeUsefulMethod()
    {
    }
}

public class ClassUnderTest
{
    public UtilityClassProvider UtilityClassProvider { get; set; }

    public void MethodUnderTest()
    {
        UtilityClassProvider.Get<UtilityClass>().SomeUsefulMethod();
    }
}

[TestFixture]
public class Tests
{
    // Fails; Rhino can't mock a sealed class
    [Test]
    public void Test()
    {
        var mockUtilityClass = MockRepository
            .GenerateMock<UtilityClass>();
        mockUtilityClass
            .Expect(o => o.SomeUsefulMethod());
        var mockUtilityClassProvider = MockRepository
            .GenerateMock<UtilityClassProvider>();
        mockUtilityClassProvider
            .Expect(o => o.Get<UtilityClass>())
            .Return(mockUtilityClass);

        var classUnderTest = new ClassUnderTest() { 
            UtilityClassProvider = mockUtilityClassProvider };
        classUnderTest.MethodUnderTest();

        mockUtilityClassProvider.VerifyAllExpectations();
        mockUtilityClass.VerifyAllExpectations();
    }
}

Unsealing UtilityClass would make mocking easier – but what are the alternatives?

Tuesday, 23 November 2010

Conway's Game of Life

Conway’s Game of Life must be one of the most continuously re-implemented algorithms in the history of software development. In case you’ve not seen it, the Game of Life is played out with cellular automata, or cells that are switched on or off by a set of rules. The rule set is applied to the all cells in one step, creating a new state or generation repeatedly.

A cellular automata implementation shares much with other problems often solved with stepwise and parallelizable modelling – as say some approaches to quantum electrodynamics, fluid mechanics and other physical models – so I figured it was as good a motif as any to explore some new technologies, and maybe work up some kata along the way.

The first step was to build up a core library code that implements the Game of Life. For a first implementation we need a grid of cells on which to play out the game, and a rule set that transforms the grid from one state to another.

This immediately sounded like a good basis for relatively quick kata. There are a couple of useful principles involved: practically, it’s immediately amenable to unit testing as we explicitly define the state we expect from any transformation; and the outline of the algorithm – applying a rule set to a whole state to transform it into a new state – is a useful one to remember.

So, to outline the kata, we need:

  1. A 2d grid that reports whether a cell at x,y is on (alive); allows us to set on/off state at x,y; allows us to clear (all off) and reset (random on/off); and perhaps display a simple representation of the grid to stdout.
  2. A rule set that transforms a 2d grid and returns a new 2d grid with some rule set applied.
  3. Unit tests should express the rules in terms of expected start and end grid states.

The rules – well they’re written up in many places, but to summarise:

  1. Cells with fewer than 2 neighbours die.
  2. Cells with 2 neighbours that are alive, stay alive.
  3. Cells with 3 neighbours stay alive if alive, or come to life if dead.
  4. Cells with 4 or more neighbours die.

The last time I went through this kata my set of C# NUnit tests looked like this – but I’d say the idea is to start from scratch each time:

    [TestFixture]
    public class CoreTests
    {
        [Test]
        [ExpectedException("System.ArgumentOutOfRangeException")]
        public void StubGridDoesntTakeNegativeWidth()
        {
            var g = new StubLifeGrid(-1, 1);
        }

        [Test]
        [ExpectedException("System.ArgumentOutOfRangeException")]
        public void StubGridDoesntTakeNegativeHeight()
        {
            var g = new StubLifeGrid(1, -1);
        }

        [Test]
        [ExpectedException("System.ArgumentOutOfRangeException")]
        public void StubGridOutOfRangeWidthRequestFails()
        {
            var g = new StubLifeGrid(2, 2);
            g.AliveAt(3, 1);
        }

        [Test]
        [ExpectedException("System.ArgumentOutOfRangeException")]
        public void StubGridOutOfRangeHeightRequestFails()
        {
            var g = new StubLifeGrid(2, 2);
            g.AliveAt(1, 3);
        }

        [Test]
        [ExpectedException("System.ArgumentOutOfRangeException")]
        public void StubGridSetOutOfRangeWidthFails()
        {
            var g = new StubLifeGrid(2, 2);
            g.Set(3, 1, true);
        }

        [Test]
        [ExpectedException("System.ArgumentOutOfRangeException")]
        public void StubGridSetOutOfRangeHeightFails()
        {
            var g = new StubLifeGrid(2, 2);
            g.Set(1, 3, true);
        }

        [Test]
        public void DiesWithNoNeighbours()
        {
            ILifeGrid g = new StubLifeGrid(3, 3);
            // Starts alive
            g.Set(1, 1, true);
            
            var r = new BasicRuleSet();
            g = r.Transform(g);

            Assert.IsFalse(g.AliveAt(1, 1));

            // Starts dead
            g = new StubLifeGrid(3, 3);
            g.Set(1, 1, false);

            g = r.Transform(g);

            Assert.IsFalse(g.AliveAt(1, 1));
        }

        [Test]
        public void DiesWithOneNeighbour()
        {
            ILifeGrid g = new StubLifeGrid(3, 3);
            // Starts alive
            g.Set(1, 1, true);
            g.Set(0, 0, true); // create all possible and test

            var r = new BasicRuleSet(); // static, in fact?
            g = r.Transform(g);

            Assert.IsFalse(g.AliveAt(1, 1));

            // Starts dead
            g = new StubLifeGrid(3, 3);
            g.Set(1, 1, false);
            g.Set(0, 0, true);

            g = r.Transform(g);

            Assert.IsFalse(g.AliveAt(1, 1));
        }

        [Test]
        public void StaysAliveWithTwoNeighbours()
        {
            ILifeGrid g = new StubLifeGrid(3, 3);
            g.Set(1, 1, true);
            g.Set(0, 0, true);
            g.Set(0, 1, true);
            g.Display();

            var r = new BasicRuleSet(); // static, in fact?
            g = r.Transform(g);
            g.Display();

            Assert.IsTrue(g.AliveAt(1, 1));

            // starts dead, stays dead
            g = new StubLifeGrid(3, 3);
            g.Set(1, 1, false);
            g.Set(0, 0, true);
            g.Set(0, 1, true);
            g.Display();

            r = new BasicRuleSet(); // static, in fact?
            g = r.Transform(g);
            g.Display();

            Assert.IsFalse(g.AliveAt(1, 1));
        }

        [Test]
        public void BornWithThreeNeighbours()
        {
            ILifeGrid g = new StubLifeGrid(3, 3);
            g.Set(1, 1, false);
            g.Set(0, 0, true);
            g.Set(0, 1, true);
            g.Set(0, 2, true);
            g.Display();

            var r = new BasicRuleSet(); // static, in fact?
            g = r.Transform(g);
            g.Display();

            Assert.IsTrue(g.AliveAt(1, 1));
        }

        [Test]
        public void StaysAliveWithThreeNeighbours()
        {
            ILifeGrid g = new StubLifeGrid(3, 3);
            g.Set(1, 1, true);
            g.Set(0, 0, true);
            g.Set(0, 1, true);
            g.Set(0, 2, true);

            var r = new BasicRuleSet(); // static, in fact?
            g = r.Transform(g);

            Assert.IsTrue(g.AliveAt(1, 1));
        }
    }

Sunday, 3 October 2010

empty XmlNodeList and avoiding a null check

Now, why does XmlNode.SelectNodes(string xpath) return null if it doesn’t find anything?

I have some existing code that reads some parameters from XML, and passes some objects and those parameters into a method that filters those objects into two sets – one that is sensitive in some way to those parameters, and one that’s not.

FilterProducts(
    XmlNodeList processes, 
    XmlNodeList processAttributes, 
    out List<Product> sensitiveFolio,
    out List<Product> nonSensitiveFolio);

I’m drawing the list of processes and processAttributes from XML – and in some cases people don’t define any attributes, wanting to just build a folio sensitive to a list of processes. In this case the XmlNodeList returned is null – and when I pass that to FilterProducts it blows up.

After the null pattern, then, I figured what I wanted to do was actually pass an empty XmlNodeList rather than a null. FilterProducts could then just pass over the attributes, and work out what was dependent on the processes only. Then I realised that what I really needed to do was dig through FilterProducts and refactor it because it was doing too much – at least compared to the usage pattern was now different to that which originally formed it.

However; both these can happen in sequence. I could deliver the simpler – using the existing code and an empty list, which wouldn’t affect any other code, then taking a look at refactoring FilterProducts, which might affect other parts of the codebase. So, populate processAttributes with a successully parsed XmlNodeList, or a new EmptyNodeList if that's null:

XmlNodeList processAttributes = 
    myConfigNode.SelectNodes("/processAttributes") ?? new EmptyNodeList; 

and then defined EmptyNodeList by subclassing XmlNodeList:

private class EmptyNodeList : XmlNodeList 
{ 
    public override XmlNode Item(int index) 
    {
        throw new NotImplementedException(); 
    }

    public override IEnumerator GetEnumerator() 
    { 
        return new EmptyNodeListEnumerator(); 
    }

    public class EmptyNodeListEnumerator : IEnumerator 
    { 
        public bool MoveNext() 
        { 
            return false; 
        }

        public void Reset() 
        { 
            throw new NotImplementedException(); 
        }

        public object Current 
        { 
            get 
            { 
                throw new NotImplementedException(); 
            } 
        } 
    }

    public override int Count 
    { 
        get 
        { 
            return 0; 
        } 
    }     
}

That works fine, delivering some code, and leaving a look at refactoring some code used in multiple places for later.