Who's Afraid of the big-bad cast operator?
My previous blog entry (which was completely misunderstood by some people who commented - I blame the fact that English is not my native tongue) reminded me of another poor coding behavior. Just so we don't have a repeat of that last post, let me clarify that I have nothing against the as operator - I use it all the time - I just have a problem with certain usage patterns that employ it.
For whatever reason, people tend to use the "as" operator instead of the cast operator.
A lot of times you will see code like this:
MyClass c = o as MyClass;
c.DoSomething();
Where the following code makes MUCH more sense:
MyClass c = (MyClass)o;
c.DoSomething();
I have two theories as to why people do it this way:
1. People are afraid of Exceptions.
2. People are used to dynamic_cast<>() from C++ which is the easiest way to check if a class is of a certain runtime type.
The reason the example above with the"as" operator is a Bad Thing is because it defers the exception to a later point in your code. If the class is not what you expect it to be, your cast will succeed (resulting in NULL) only to fail a few lines later on a NullReferenceException - which in turn makes debugging and logging harder to understand because you are not really seeing the correct exception.
Now, there are many valid usages of the as operator - but they almost always rely on the fact that you actually do something with the fact that the class is different than what you expect. So for example, if you want your Equals method to be able to handle classes different than your own, that's a GREAT place to use as:
public override bool Equals(object o)
{
MyClass other = o as MyClass;
if (other == null) return false;
//....
}
However, if you do not intend for your .Equals method to support comparison with other types (and have no other usage for the knowledge that the reference you got is not what you expected), you should definitely use the regular cast operator.
Comments
Anonymous
March 24, 2008
Just a couple of days ago I saw "as" being used 4 times in 2 lines of code! Sheesh, especially that those 4 casts could have been reduced to just 2 but that's another story. As for why people use "as" instead of "is" you may be right that they are sort of afraid of cast exceptions but when I see "as" being used without a subsequent null check I tend to believed that they simply started writing code, realised that they need to cast something to get what they need and just slapped an "as" in.Anonymous
March 25, 2008
Thanks for blogging this. It's been a battle on our project to highlight when and when not to use "as" and regular casts. You've put it succinctly, although i think "is" requires mention. In my opinion, an "as" should be used if the code uses "is" followed by a cast if the "is" returns true, as the "as" does both these actions. If the value must ALWAYS be the type, a regular cast should be used; if the value doesn't matter and only the type does, the "is" should be used. Like I said, thanks. :)Anonymous
March 25, 2008
The comment has been removedAnonymous
March 25, 2008
If a button is not guaranteed to exist or if .FindControl("button") may return a different class than Button, then this is, in mind, a great way of using "as". If, on the other hand, your application is guaranteed to always have a button under that name (which it doesn't), a cast is probably better.Anonymous
March 25, 2008
If you embark on a voyage that will have you do both "is" and "as", you are most likely wasting time as "is" is basically "as" with a check for null. You are now doing that twice.