Tuesday 17 November 2009

.NET Gotcha – Static Initialiser Ordering

Last week I had to troubleshoot a strange problem. A developer had broken up the source code for a large class of constants (defining various colours) into separate files using the partial keyword, and all of a sudden our Windows Forms controls were painting themselves black instead of the correct colour. Put them back into one file again and the problem went away.

Eventually we tracked the problem down due to the order in which the fields were being initialised by the static constructor. Consider the following code and unit test (which passes):

static class MyConstants
{
    public static readonly int MyNumber = 5;
    public static readonly int MyOtherNumber = MyNumber;
}

[TestFixture]
public class MyConstantsTests
{
    [Test]
    public void ConstantsAreInitialisedCorrectly()
    {
        Assert.AreEqual(5, MyConstants.MyNumber);
        Assert.AreEqual(5, MyConstants.MyOtherNumber);
    }
}

Now if we simply change the ordering of the statements in MyConstants…

static class MyConstants
{
    public static readonly int MyOtherNumber = MyNumber;
    public static readonly int MyNumber = 5;
}

… the test will fail as MyOtherNumber will be 0. Obviously if the two definitions exist in different source files, this type of problem is much harder to spot. The test does pass if we use the const keyword instead of static readonly, but since we were initialising using the Color.FromArgb function, this was not an option for us.

The moral of the story is to avoid setting static read-only fields to values dependent on other fields. Or at least be aware of the problems that can arise from doing so.

2 comments:

Unknown said...

Why are you even using the partial keyword? Only use it for (partial) generated code. If you need the partial keyword you probably need to refactor to several classes.

Unknown said...

I wasn't using it. My recommendation was to create separate classes as you suggest. The developer thought it would be "safer" to do partial classes, because this would require no code changes elsewhere.