Cast abuse

Every so often when reading through some C# code I’ll run across the as keyword. Now this is a very nifty feature of C#. Just like the familiar parenthetical cast, it returns the object cast to the new type if it succeeds. However, if it fails then it simply returns null instead of throwing an exception.

This is cool when you aren’t sure if you can cast an object to the type you need, but don’t want to force the runtime to test if this can be done twice by using is followed by a parenthetical cast.

It’s markedly not cool when you are not testing if the object you got is null. I see a lot of people doing stuff like (o as Widget).DoStuff(). This is the worst thing you could do here. Why? Because if o cannot be cast to Widget then the result will be null, and a NullReferenceException will be thrown. This will misdirect you to start checking why o is null, when in fact it is not. Using the “old-fashioned” parenthetical cast is appropriate here: ((Widget) o).DoStuff(). If the cast fails you will get the expected InvalidCastException and know immediately what the problem is.

Can someone please explain to me why as is so frequently abused like this?

CategoriesC#

13 Replies to “Cast abuse”

  1. I agree with you, but if i’m really sure that a cast not fails (like a 1.1 ArrayList filled with only strings…) i don’t like to write the check for null items, because the code is not so readable…

  2. This irritates me too πŸ™‚

    You’re right, this happens a lot, and it is clearly a programmer error. I think it happens because many C# programmers aren’t aware of the difference, and the “as” syntax is nicer to read.

  3. It might have migrated across from the Delphi language which was the starting point for Anders when he designed C#. In Delphi, the ‘as’ keyword behaves as the straight cast does in C#, whereas the straight cast in Delphi does not perform any check at all. Had me a bit mixed up when first migrating as well.

  4. Let me clarify… the use has migrated from Delphi, Anders clearly implemented it differently in C#. It must be late… sorry for the confusion.

  5. Yea, I too think that a lot of programmers don’t realize the difference. Plus, checking for “null” is something most programmers never do (no matter what). It’s great that you’re pointing it out, though. πŸ™‚

  6. πŸ™‚ You are right about that. Well that is because almost nobody seam to understand casting. Maybe these where VB programmers before they came to .net.

    The same as retrieving objects from an untyped dataset πŸ™‚

    DataRow dr = ds.Tables[0].Rows[0];
    Convert.ToInt32(dr[0]);

    This is fairly common while it should of course be:

    DataRow dr = ds.Tables[0].Rows[0];
    (int)dr[0];

    And yet another example of the ‘as’ keyword

    You’ll see this quite often too:

    if(o is MyOjbect)
    {
    MyObject mo = (MyObject)o;
    }

    While your method should be used by comparing the result against null. Well, first of all check that the existing reference isn’t null πŸ™‚

  7. Ramon:

    Actually that test will catch it, because the “is” operator always returns false when the left operand is null. “o is MyObject” is asking the question “is o not null and can it be converted to MyObject?”

    Note that null is special in this case because while “is” will always return false, a cast to anything (except for a value type) will succeed.

  8. @Chris: Yes you are right but what I meant is that getting a null reference as input parameter is almost always incorrect and that you should first validatie that. While it is true that it will not result in an error with the ‘as’ or ‘is’ keywords it is probably not intended (in most scenarios) to be used that way.

    Besides that.. my if scenario results in worse performance because the runtime now checks ‘o’ twice while with the ‘as’ keyword this is only done once.

  9. Ramon:

    Yes, I’m not saying that you should not use “as.” I’m saying you shouldn’t do it when doing a one-shot cast like “(o as Foo).DoSomething()” because if o can’t be converted to Foo the exception you get (NullReferenceException) will be very misleading.

  10. Yeah, this is an interesting one. I think it’s really just because the syntax looks nice. What happens is that you have cases where you’re sure the cast will succeed, so you use “as” instead of a parenthetical cast just for the prettier syntax. I don’t think there’s really anything wrong with that usage, except that it establishes a pattern of using “as” that is different from the correct usage pattern when the cast may fail.

  11. I often do this if I need to do a check+cast for only a single operation:

    if(o is Something) {
    ((Something)o).Blah();
    }

    If I intend to do multiple operations after a cast, it’s nice to use ‘as’:

    Something something = o as Something;
    if(o != null) {
    o.Blah();
    o.Hooray();
    }

    It’s really useful when you are working with an object that implements many interfaces and you want to know if it’s ok to call a method from some interface. The ‘is’ will let you know if your cast is safe, or the ‘as’ will do the checking and the cast if it’s safe.

    Unless you’re sure ‘as’ will not return null, or if it does and you do a check for null, be extremely careful with NullReferenceException (NRE). You basically _never_ want to have an NRE raised on you – ever.

    Under the hood, an NRE is just a sign that the JIT tried to dereference a native null pointer, which in turn causes a SIGSEGV (segfault). The runtime however traps the SIGSEGV signal so it can translate it into the NRE and send it upstream to your managed application as an exception.

    All this meaning that NREs are expensive and also can be prone to break your application if Mono mingles with some buggy library. If some library in your application traps SIGSEGV and doesn’t restore the previous signal handler, Mono will never be able to translate that SIGSEGV into an NRE and your application will crash for no apparent reason, and with no easy way to trace and find out why.

    This was a big bug in GStreamer a few versions back where it trapped SIGSEGV during its plugin loading process but restored it to the system default handler and not the previous handler (Mono’s JIT), thus Mono became unable to recover from the SIGSEGV.

    /me ends rant πŸ™‚

  12. As a side note, the IL generated by “((Foo) o).Bar()” and “(o as Foo).Bar()” are identical in size and only differ in that the former uses “castclass Foo” and the latter “isinst Foo”. A quick google suggests that performance is identical when the cast succeeds.

    As a matter of coding style though, I still say using “as” here is putting a square peg in a round hole.

  13. Perhaps DoStuff is implemented as an extension method on Widget that safely does nothing when Widget is null… πŸ™‚

Comments are closed.