Prefix cast or as-cast?

I read today 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.

  1. I don’t agree. The ‘as’ operator can and should be used in the pattern you described above as ‘bad practice’, only without the silly return statement. While I understand the need of useful exceptions, it seems to me that you have concocted weird examples to substantiate your claim.

    Let me give you a few different examples.

    What if you want to create a method that gets an object parameter and you want it to do different things depending on the type (think extension methods)? Unless you use .NET 4.0 and a cast to dynamic, you do not have any way of unboxing an object other than try to cast it or doing weird GetType().Name switches. Or maybe they are not weird, but I find them so. And don’t start me on the Double Dispatch pattern, which also requires you have control over all the classes involved, which pretty much makes it refactorable away.

    How do you check if an object implements a certain interface? You use ‘as’, especially since the implementation might be explicit and even if your type is unboxed, you still can’t use the interface members directly.

    And talking of meaningful exceptions, I still feel that it is better to cast an object to a type using ‘as’ and then throwing a context aware meaningful exception if it is null and it shouldn’t have been. What you were talking about seems to me like a different code smell: the need to assert a condition. Basically “I want to have a method that gets a base type argument and throws an exception if the type is not a specific implementation”. I know that we hit this kind of scenarios more often in this age of web services and serialization, but still… why would that method ever get a base type parameter in the first place?

    I think a more poignant debate is on what are the scenarios where we need to cast at all and which ones are really inevitable.

    • “How do you check if an object implements a certain interface? You use ‘as’, especially since the implementation might be explicit and even if your type is unboxed, you still can’t use the interface members directly.”
      Try the “is” operator :lol:

Leave a Comment


NOTE - You can use these HTML tags and attributes:
<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>

WP Like Button Plugin by Free WordPress Templates