Anti-Patterns and Worst Practices – Heisenbugs

As I mentioned before, a Heisenbug occurs when trying to check the state of an object. These types of defects are common with concurrency issues are present. Microsoft has put out a library to help diagnose these problems: CHESS (http://msdn.microsoft.com/en-us/devlabs/cc950526.aspx). I’m not going to be addressing the concurrency issue, but the more common simpler types of Heisenbugs; the occurrences of these types of flaws in a “normal” application. When digging deeper into the issue, you may be causing more of a problem. This type of bug can manifest itself when not adhering to command-query separation guidelines (CQS). I’ve seen a prime example of this in code that performs calculations and validation inside a C# property setter.

Breaking CQS

When you first glance at this code, you may immediately identify that there’s a problem here. If you do, good for you; this is the kind of code that breaks CQS guidelines and can creep into your codebase if you’re not careful. (Another reason pair-programming and code reviews are extremely helpful)

public IShippingMethod ShippingMethod
{
    get
    {
        return shippingMethod;
    }
    set
    {
        if(value.IsValidFor(this))
            shippingMethod = value;
    }
}

Why is this bad? If you haven’t spotted it already, it’s the conditional inside the the setter. If you’re a consumer of this API, you can set the ShippingMethod and immediately after, the value that you set is not the value currently assigned. This type of behavior inside a setter hasn’t been as common to me as it could be (thankfully), but I’ve seen other behaviors that can cause issues. Take this similar example:

public IShippingMethod ShippingMethod
{
    get
    {
        return shippingMethod;
    }
    set
    {
        if(value.IsValidFor(this))
        {
            shippingMethod = value;
            shippingCode = value.GenerateShippingCode(this);
        }
        else
        {
            shippingCode = ShippingCode.GROUND;
        }
    }
}

The setting of one property is deciding a change in another field. You could essentially set the ShippingCode, then set the ShippingMethod and have a different result in the ShippingCode you just assigned. If you want to make things worse, put that shippingCode assignment operator inside the get of the ShippingMethod property. Why would anybody do this? Because if it can be done, it will be done at some point. Talk about driving a developer crazy!?

Here is where CQS comes into play. In our query of what the Order’s ShippingMethod was, we were also commanding it to recalculate another value. (This example is based off of one that I’ve seen. You can replace the set property with a function that sets it, this provides a little bit more visibility to the developer that something is going to happen. In .Net the getters/setters are functions behind the scenes, but there is a bit of an understanding that these aren’t supposed to perform actions. See AutoProperties.) In the previous example I could show commands being performed inside the get, actually doing both a command and query, blatantly violating CQS, but I’ll spare you some pungent code smell.

Stack Memory Allocation

These types of defects can be avoided by being careful and understanding your code base. Not all the time do we have the luxury of working with code we’ve written. This similar type of behavior can exist in places where you’re not used it. Take the following test, it seems to make sense.

[Test]
    public void NotificationService_should_have_first_message_equal_to_one()
    {
        // Arrange
        var service = new NotificationService();

        // Act
        var first = service.GetNextNotification();

        // Assert
        Assert.That(first.Message, Is.EqualTo("one"));
    }
}

This test passes with no problems if you run it straight through without examining it. However, when you step through the code with the debugger turned on:

With the highlighted text above, I added a QuickWatch to determine what the value will be when calling the GetNextNotification function on NotificationService. My test was passing, it should return the value I’m asserting below; it should return the string “one”:

In this screen shot of the QuickWatch dialog, you can see that the value of the Notification’s Message property is “two”. Why on earth would it be different this time around? As small as this example is, it proves that Heisenbugs can most certainly exist in very simple environments. Why does this exist? Well, in this example, it’s due to poor implementation. You may have spotted it already with the name of my GetNextNotification function. The Next is the giveaway, I could have omitted the Next in the function name and made it even harder to figure out, it’s returning the top of a Stack collection. The debugger is helping break this (by the debugger examining it, I’m getting commands & queries, thus unexpected behavior).

public Notification GetNextNotification()
{
    return stack.Pop();
}

Dealing with Deferred Execution

The last type of Heisenbug I’d like to briefly touch on is a side effect of some newer functionality of the .Net runtime, deferred execution.

Deferred execution means that the evaluation of an expression is delayed until its realized value is actually required. Deferred execution can greatly improve performance when you have to manipulate large data collections, especially in programs that contain a series of chained queries or manipulations. In the best case, deferred execution enables only a single iteration through the source collection. (From MSDN)

I’m not suggesting that deferred execution is a Heisenbug, just that you can get some unexpected behavior when dealing with it. If you check out Chris Williams’ example you’ll see a nice quick example of how this can be “weird”. Here’s the C# version of his example:

var filter = "System";
var query = from a in AppDomain.CurrentDomain.GetAssemblies()
            where a.GetName().Name.Contains(filter)
            select a.GetName().Name;
//filter = "Xml";

When you’re dealing with LINQ or yield return, just keep in mind what’s going on and you should be fine. The unexpected behavior in my experience is most noticeable when testing with the debugger, all the more reason to avoid it. 😉

[See more Anti-Patterns and Worst Practices]