views:

3051

answers:

13

In development blogs, online code examples and (recently) even a book, I keep stumbling about code like this:

var y = x as T;
y.SomeMethod();

or, even worse:

(x as T).SomeMethod();

That doesn't make sense to me. If you are sure that x is of type T, you should use a direct cast: (T)x. If you are not sure, you can use as but need to check for null before performing some operation. All that the above code does is to turn a (useful) InvalidCastException into a (useless) NullReferenceException.

Am I the only one who thinks that this a blatant abuse of the as keyword? Or did I miss something obvious and the above pattern actually makes sense?

+104  A: 

Your understanding is true. That sounds like trying to micro-optimize to me. You should use a normal cast when you are sure of the type. Besides generating a more sensible exception, it also fails fast. If you're wrong about your assumption about the type, your program will fail immediately and you'll be able to see the cause of failure immediately rather than waiting for a NullReferenceException or ArgumentNullException or even a logical error sometime in the future. In general, an as expression that's not followed by a null check somewhere is a code smell.

On the other hand, if you are not sure about the cast and expect it to fail, you should use as instead of a normal cast wrapped with a try-catch block. Moreover, use of as is recommended over a type check followed by a cast. Instead of:

if (x is SomeType)
   ((SomeType)x).SomeMethod();

which generates an isinst instruction for the is keyword, and a castclass instruction for the cast (effectively performing the cast twice), you should use:

var v = x as SomeType;
if (v != null)
    v.SomeMethod();

This only generates an isinst instruction. The former method has a potential flaw in multithreaded applications as a race condition might cause the variable to change its type after the is check succeeded and fail at the cast line. The latter method is not prone to this error.


The following solution is not recommended for use in production code. If you really hate such a fundamental construct in C#, you might consider switching to VB or some other language.

In case one desperately hates the cast syntax, he/she can write an extension method to mimic the cast:

public static T To<T>(this object o) { // Name it as you like: As, Cast, To, ...
    return (T)o;
}

and use a neat[?] syntax:

obj.To<SomeType>().SomeMethod()
Mehrdad Afshari
I think the race condition is irrelevant. If you are experiencing this problem, then your code is not thread-safe, and there are more reliable ways to solve it than by using keyword "as". +1 for the rest of the answer.
RMorrisey
+1 for @RMorrisey. If the above code had a race condition you almost certainly have *serious* problems. Otherwise, one vs two bytecodes in a "compiled language" probably isn't going to be an issue!
Tom Hawtin - tackline
@RMorrisey: I have at least one example in mind: Assume you have a `cache` object that another thread tries to invalidate it by setting it to `null`. In lock-free scenarios, these kind of stuff can arise.
Mehrdad Afshari
Excellent answer!
Andrew Garrison
is+cast is enough to trigger a "Do not cast unnecessarily" warning from FxCop: http://msdn.microsoft.com/en-us/library/ms182271.aspx That should be enough reason to avoid the construct.
David Schmitt
@David: Oh, yeah. Thanks for reminding me. I had forgotten that FxCop rule.
Mehrdad Afshari
Point taken. I can see how it's an improvement in that scenario. In my coding I haven't had occasion/need to work without using locks.
RMorrisey
+21  A: 

IMHO, as just make sense when combined with a null check:

var y = x as T;
if (y != null)
    y.SomeMethod();
Rubens Farias
+5  A: 

The direct cast needs a pair of parentheses more than the as keyword. So even in the case where you're 100 % sure what the type is, it reduces visual clutter.

Agreed on the exception thing, though. But at least for me, most uses of as boil down to check for null afterwards, which I find nicer than catching an exception.

Joey
+7  A: 

99% of the time when I use "as" is when I'm not sure what's the actual object type

var x = obj as T;
if(x != null){
 //x was type T!
}

and I don't want to catch explicit cast exceptions nor make cast twice, using "is":

//I don't like this
if(obj is T){
  var x = (T)obj; 
}
Yacoder
You've just described the proper use case for `as`. What's the other 1%?
P Daddy
In a typo? =) I meant 99% of the time I use this _exact_ code snippet, while sometimes I may use "as" in a method call or some place else.
Yacoder
D'oh, and how is that less useful than the second popular answer???
Yacoder
+1 I agree calling this out is as valuable as Rubens Farias's answer - people will hopefully come here and this will be a useful example
Ruben Bartelink
+1  A: 

I believe that the as keyword could be thought of as a more elegant looking version of the dynamic_cast from C++.

Andrew Garrison
I'd say a straight cast in C# is more like `dynamic_cast` in C++.
P Daddy
i think the straight cast in C# is more equivalent to the static_cast in C++.
Andrew Garrison
@Ruben Bartelink: It only returns null with pointers. With references, which you should be using when possible, it throws `std::bad_cast`.
P Daddy
@Andrew Garrison: `static_cast` performs no runtime type check. There is no similar cast to this in C#.
P Daddy
Sadly, I didn't know you could even use the casts on references, as I have only ever used them on pointers, but P Daddy is absolutely correct!
Andrew Garrison
Ruben Bartelink
+4  A: 

It's just because people like the way it looks, it's very readable.

Lets face it: the casting/conversion operator in C-like languages is pretty terrible, readability-wise. I would like it better if C# adopted either the Javascript syntax of:

object o = 1;
int i = int(o);

Or define a to operator, the casting equivalent of as:

object o = 1;
int i = o to int;
JulianR
Just so you know, the JavaScript syntax you mention is also allowed in C++.
P Daddy
@PDaddy: It's not a direct 100% compatible alternate syntax though and is not intended as that (operator X vs conversion constructor)
Ruben Bartelink
I'd prefer it to use the C++ syntax of `dynamic_cast<>()` (and similar). You're doing something ugly, it should look ugly.
Tom Hawtin - tackline
+1  A: 

It's probably more popular for no technical reason but just because it's easier to read and more intuitive. (Not saying it makes it better just trying to answer the question)

DrDro
+2  A: 

People likeas so much because it makes them feel safe from exceptions... Like guarantee on a box. A guy puts a fancy guarantee on the box 'cause he wants you to feel all warm and toasty inside. You figure you put that little box under your pillow at night, the Guarantee Fairy might come down and leave a quarter, am I right Ted?

Back on topic... when using a direct cast, there is the possibility for an invalid cast exception. So people apply as as a blanket solution to all of their casting needs because as (by itself) will never throw an exception. But the funny thing about that, is in the example you gave (x as T).SomeMethod(); you are trading an invalid cast exception for a null reference exception. Which obfuscates the real problem when you see the exception.

I generally don't use as too much. I prefer the is test because to me, it appears more readable and makes more sense then trying a cast and checking for null.

Bob
"I prefer the is test " - "is" followed by a cast is of course slower than "as" followed by a test for null (just like "IDictionary.ContainsKey" followed by dereferencing using the indexer is slower than "IDictionary.TryGetValue"). But if you find it more readable, no doubt the difference is rarely significant.
Joe
The important statement in the middle part is how people apply `as` as a blanket solution because it makes them feel safe.
Bob
@Bob: Finally got it, +1
Ruben Bartelink
@Ruben, Awesome, thanks
Bob
+8  A: 

I've often seen references to this misleading article as evidence that "as" is faster than casting.

One of the more obvious misleading aspects of this article is the graphic, which does not indicate what is being measured: I suspect it's measuring failed casts (where "as" is obviously much faster as no exception is thrown).

If you take the time to do the measurements, then you'll see that casting is, as you'd expect, faster than "as" when the cast succeeds.

I suspect this may be one reason for "cargo cult" use of the as keyword instead of a cast.

Joe
Thanks for the link, that's very interesting. From how I understood the article, he *does* compare the non-exception case. Nevertheless, the article was written for .net 1.1, and the comments point out that this changed in .net 2.0: Performance is now almost equal, with prefix cast even being slightly faster.
Heinzi
The article does imply he is comparing the non-exception case, but I did some tests a long time ago and wasn't able to reproduce his claimed results, even with .NET 1.x. And since the article doesn't provide the code used to run the benchmark, it's impossible to say what's being compared.
Joe
@Joe: Good point, thanks the for the clarification.
Heinzi
"Cargo cult" - perfect. Check out "Cargo Cult Science Richard Feynman" for the full info.
Bob Denny
A: 

One reason for using "as":

T t = obj as T;
 //some other thread changes obj to another type...
if (t != null) action(t); //still works

Instead of (bad code):

if (obj is T)
{
     //bang, some other thread changes obj to another type...
     action((T)obj); //InvalidCastException
}
Rauhotz
If you've got race conditons this uglym you've got bigger issues (but agree its a nice sample to go with the others so +1
Ruben Bartelink
-1 as this perpetuates a fallacy. If other threads can be changing the type of obj, then you still have problems. The claim "//still works" is very unlikley to hold true, as t will be used as a pointer to T, but it points to memory which is no longer a T. Neither solution will work when the other thread changes the type of obj while action(t) is in progress.
Stephen C. Steel
@Stephen C. Steel: You seem to be quite confused. Changing the type of `obj` would mean changing the `obj` variable itself to hold a reference to another object. It wouldn't alter the contents of memory at which resides the object originally referenced by `obj`. This original object would remain unchanged, and the `t` variable would still hold a reference to it.
P Daddy
http://www.infoq.com/news/2010/01/CDS-Dictionary
Ruben Bartelink
@P Daddy - I think you are correct, and I was wrong: if obj was rebound from a T object to a T2 object, then t would still be pointing at the old T object. Since t still references the old object, it can't be garbage collected, so the old T object will remain valid. My race condition detector circuits were trained on C++, where similar code using dynamic_cast would be a potential problem.
Stephen C. Steel
+3  A: 

This has to be one of my top peeves.

Stroustrup's D&E and/or some blog post I cant find right now discusses the notion of a to operator which would address the point made by http://stackoverflow.com/users/73070/johannes-rossel (i.e., same syntax as as but with DirectCast semantics).

The reason this didnt get implemented is because a cast should cause pain and be ugly so you get pushed away from using it.

Pity that 'clever' programmers (often book authors (Juval Lowy IIRC)) step around this by abusing as in this fashion (C++ doesnt offer an as, probably for this reason).

Even VB has more consistency in having a uniform syntax that forces you to choose a TryCast or DirectCast and make up your mind!

Ruben Bartelink
+1. You probably meant `DirectCast` *behavior*, not *syntax*.
Heinzi
@Heinzi: Ta for +1. Good point. Decided to be a smartarse and use `semantics` instead :P
Ruben Bartelink
+16  A: 

Using 'as' does not apply user defined conversions while the cast will use them where appropriate. That can be an important difference in some cases.

Larry Fix
This is important to remember. Eric Lippert goes over that here: http://blogs.msdn.com/ericlippert/archive/2009/10/08/what-s-the-difference-between-as-and-cast-operators.aspx
P Daddy
Nice comment, P! If your code depends on this distinction, though, I'd say there's a late-night debugging session in your future.
TrueWill
+11  A: 

I wrote a bit about this here:

http://blogs.msdn.com/ericlippert/archive/2009/10/08/what-s-the-difference-between-as-and-cast-operators.aspx

I understand your point. And I agree with the thrust of it: that a cast operator communicates "I am sure that this object can be converted to that type, and I am willing to risk an exception if I'm wrong", whereas an "as" operator communicates "I am not sure that this object can be converted to that type; give me a null if I'm wrong".

However, there is a subtle difference. (x as T).Whatever() communicates "I know not just that x can be converted to a T, but moreover, that doing so involves only reference or unboxing conversions, and furthermore, that x is not null". That does communicate different information than ((T)x).Whatever(), and perhaps that is what the author of the code intends.

Eric Lippert
I disagree with your speculative defense of the code's author in your last sentence. `((T)x).Whatever()` *also* communicates that `x` is not [intended to be] null, and I highly doubt that an author would typically care whether the conversion to `T` occurs with only reference or unboxing conversions, or if it requires a user-defined or representation-changing conversion. After all, if I define `public static explicit operator Foo(Bar b){}`, then it is clearly my intent that `Bar` be considered compatible with `Foo`. It's rare that I would want to avoid this conversion.
P Daddy
Well, perhaps *most* authors of code would not be making that subtle distinction. I personally might be, but if I was, I'd add a comment to that effect.
Eric Lippert