I've been noticing more and more that people use the 'as' keyword in C# to presumably do safe type casts. But enough is enough, it's not ideally the way to actually go about type conversions, nor does it have any benefits.
To start things of, the expression 'instance1 as SomeType' keyword is just the equivalent of '(instance1 is SomeType ? (SomeType)instance1 : null)' and the following code will throw an exception anyway, iff the type is not instance of SomeType... So why do you type cast this way?
[TestMethod]
public void Index()
{
// Setup
HomeController controller = new HomeController();
// Execute
ViewResult result = controller.Index() as ViewResult;
// Verify
ViewDataDictionary viewData =
result.ViewData as ViewDataDictionary;
Assert.AreEqual( "Home Page", viewData["Title"] );
}
The above example is from the ASP.NET MVC Framework, and while it's just a sample from the testing framework this code is bad. Because when the right-hand side 'controller.Index() as ViewResult' returns null (the return type was not of type ViewResult) and this code should, because this expression implies that type does not have to be ViewResult. There is no error handling code and the result is the NullReferenceException, which at this point is a side effect of some other code that failed.
Do you know the null coalescing operator? It's about the most useless thing in the language, or is it?
(controller.Index() as ViewResult) ?? new ViewResult()
Now, if you really want the program to crash in the event of such a failure you should NOT do this, nor should you yield null in the case of a type test failure. But if you want to actually type cast and in the case of a problem fall-back to some instance, you can use the above snippet.
Think about it. The example is really stupid because it fails to capture that an error could (or could not occur) as well as that if it would, the wrong error would be reported. But this snippet, it can actually make sense of a failed type test and rely on any other code to provide an instance of some type that doesn't have to stop execution of your program.
6 comments:
Your examples miss the correct usage of as... it should always be followed by a block where the result of the as cast is checked for not equals null... Like this:
// Verify
ViewDataDictionary viewData = result.ViewData as viewDataDictionary;
if(viewData != null)
{
Assert.AreEqual( "Home Page", viewData["Title"] );
}
In that situation, no exception will happen. You can do a "runtime" cast, with a logic check for success instead of a compile time cast (ie, just putting the casting type in front in parentheses), which may or may not actually be caught by the compiler...
The reason to use as instead of is is that you end up casting twice using is, if in the end you want to cast the object. When you use is it's doing a cast, then you cast again when it was successful... That's why as rocks, becasue it combines the check and the cast into a single operation, reducing the casting overhead.
It's kind of like the TryParse or TryGetValue methods...
Well, I think you're reading too much in to it.
Look at this, it says nothing of the sort that 'as' would be faster, it's the equivalent of "expression is type ? (type)expression : (type)null". And as long as I don't have equivalent all backwards it's the exact same. Not sure where you got that idea from but if you look at the IL, it's different in that using the 'as' keyword, it will result in additional IL opcodes. That will no matter what you say, cost you more than a conventional static cast.
Now, you're doing the right thing, in that you're checking that the value is not null. But my point is here, that you need to think about the program flow before you go about using conditional branches in the event of a type test failure.
You're test will silently pass in the event of a failure which could be just fine.
I would probably go about this a different way. Say...
if ( someObject is someType )
{
// Do this...
}
else if ( someObject is someOtherType )
{
// Do something else...
}
else
{
// Assert something, or fail, or pass
}
The above reasoning will more clearly capture what's going on, or what was meant to be going on. And that's is why the 'as' keyword might be bad and difficult to convey the meaning of your code which is exactly what I'm after.
Using a simple static cast is of course faster, and if it fails it will throw an exception, which is of course a lot slower and more costly.
The primary difference between as as a static cast is that it doesn't throw an expception.. Which may or may not be what you want.
What I said was that using as is definately faster than using the is keyword and THEN doing a static cast when is is successful, because using is causes two checks... One for the is statement, and one for the static cast.
From the link you provided:
"It is equivalent to the following expression except that expression is evaluated only one time."
That's the point of as, in a nutshell.
That said, I often use a similar but slightly different methodology than what you posted.. using Type.IsAssignableFrom...
Type t = obj.GetType();
if (typeof(someType).IsAssignableFrom(t))
{
// Do this...
}
else if (typeof(someOtherType).IsAssignableFrom(t))
{
// Do something else...
}
else
{
// Assert something...
}
but that's generally in context where I already have the object Type, or I need to do some reflection on the type, so I'll need the Type refernce anyway.. If all I'm dealing with is the object instance, I'll use the "as, if not null" pattern, because using is, is fundamentally useless if you're plannign to cast it to the type anyway.
The only downside I see from using as, is that you have a variable that is scoped outside of the "if not null" block when it's really meant to be used inside the block. This could (in really dumb situations) result in someone using the object refernce, when it was null, outside of the correct scope, leading to a null reference exception.
>shrug<
Okay! I'll withdraw one thing, that the compiler result of 'as' is more that that of a static cast. It's exactly the same, except that 'as' will push a null value on top of the evaluation stack while a static cast will throw on failure.
But that's it, 'is' and 'as' are virtually the same.
I think you misunderstood the meaning of expression. While the 'as' syntax can be rewritten to it's equivalent nested-if the expression is still not evaluated more than once despite occurring at two places in the "equivalent" nested-if code.
i.e A method call "GetObject() as SomeType" would be the equivalent of "object obj = GetObject(); obj is SomeType ? (SomeType)obj : null", and not "GetObject() is SomeType ? (SomeType)GetObject() : null" which kind of implies that the method was called twice.
I've actually never had to use the IsAssignableFrom method. If I could, I would most likely rely on the 'is' keyword because of it's shorter syntax.
But the purpose of this entire post, was to write code that captures meaning. And so, e.g. If I write a test which expects something to be of a certain type. I want that test to throw an InvalidCastException if the cast is invalid.
I completely agree about the last bit.
In the context of testing, you want as detailed of exceptions as possible. And you want those to be in "developer speak" so that you can easily debug your failed unit tests when needed.
But in released production code you never want a program to throw an InvalidCastException, or a NullReferenceException, or anything of that nature. You want to throw a custom exception that explains to the user what the real problem is, in terms the user will understand.
So, instead of just letting the static cast fail, and handling the invalidcastexception, use is or as, check for false or null (respectively), then throw a customized exception that explains the real cause that is behind the effect of the cast failing.
If you don't know why... And can't explain it in terms the user would understand, then that's a completely different problem with the code.. It means it's capable of doing things you're not aware of, and that's a much bigger problem.
Of course you could use customized exceptions and still handle the static cast exception, and just throw them into the inner exception property... Which is kind of a good happy medium.
We're pretty much getting at the same thing. I've might have been a bit to quick to judge or harsh in that last reply. But I've enjoyed the discussion.
You're point about doing both 'is' and static cast is very much so slower than just 'as' and checking for null.
But I want proper exceptions in the event of a failure. That's been one of my many goals. (in reply to the release build... if you let exceptions propagate freely that's immensely bad)
As for performance, don't you think where being a bit picky working with a managed JIT-compiled language? I think you can let this one slide. Albeit be it me who brought up the performance implication.
Post a Comment