Prefix cast or as-cast?

Today, I read a nice article from Kathleen Dollard called To “as” or not to “as”. This is a pain-point for me on which I stumble often, so I decided to write this little rant.

I particularly liked a paragraph from the above-cited article:

One of the things that makes hard bugs hard is when there is a disconnect in time or space between the cause and the symptom. Time is time, space is lines of code, assembly placement, etc. Code can be written to minimize these disconnects. One of the ways to do that is to fail quickly. When application state becomes incorrect, yell about it immediately and rarely continue with the application in an invalid state. A null-reference exception at an unexpected time just makes code more difficult to debug.

I couldn’t express this as good as Kathleen did. Make no mistake, I am quite biased in this comparison (direct-cast vs. as-cast). I kind of hate the abuse of the as operator.

Very often, people turn to as instead of the direct (prefix) cast because:

• They fear the InvalidCastException (strange, they don’t seem to fear the NullReferenceException)
• They feel the syntax more fluent, closer to the human language.

I would consider the only valid case to use the as-cast is, just like Kathleen states, when a null value result is valid for the rest of the execution of the code. For the rest of the cases, it’s just wrong.

This also promotes (doesn’t necessarily causes but promotes) bad practices like this:

public static void OnButtonClick(object sender, EventArgs e)
{
    var button = sender as Button;
    if (button == null)
    {
        return;
    }
    if (button.Tag == "somevalue")
    {
        // do something
    }
    // ...
}

In this example, the event handler (which could be attached to more than one distinct button) simply forces under the rug a situation which would be abnormal (the sender not being a button) instead of releasing it so the developers could find it easier and debug it. A saner approach is:

public static void OnButtonClick(object sender, EventArgs e)
{
    var button = (Button)sender;
    if (button.Tag == "somevalue")
    {
        // do something
    }
    // ...
}

This brings me to another advantage of the prefix-cast : it produces shorter, clearer code.

In other cases, the as abuse does more harm, hiding the source of a bug:

public void ProcessData(Entity entity)
{
    var person = entity as Person;
    UpdatePersonStatistics(person);
    // .. more code
}

public void UpdatePersonStatistics(Person person)
{
    NormalizeData(person);
    // .. more code
}

public void NormalizeData(Person person)
{
    person.Name = person.Name.Substring(0, 50);
    person.Address = person.Address.Substring(0, 100);
    // .. more code
}

Of course, this is a contrived example full of bad practices but for now let’s focus on the as usage. Suppose the ProcessData method receives an instance of Category by mistake. Since Category inherits Entity, the compiler will not complain.

The result is that there will be a NullReferenceException two methods further, in the NormalizeData method. If the cast was done with a prefix cast, the error was a little bit easier to spot. This is confusing two-fold:

1. The name of the exception suggests that a null reference was somehow obtained but in fact a real instance of Category was passed, not a null.
2. The error does not originate from the NormalizeData code, but from the caller of the ProcessData.

Summary

Use as only if a null result of the conversion makes sense for the flow of the execution. Otherwise use prefix cast.