Anyone else hate this?
Doofer doofer = GetWidget(widgetId) as Doofer;
Then, somewhere miles away from this innocuous little line, I find that my doofer is unexpectedly null. This is a great way to introduce hard-to-trace bugs, unless of course it’s complimented with guard clauses everywhere to test for nulls, which most of the time means unneccesary clutter.
What’s wrong with good old…
Doofer doofer = (Doofer)GetWidget(widgetId);
?
In this second case, if the result of GetWidget() can’t be turned into a Doofer, I’ll get an exception thrown right there and then. Assuming that my code here works specifically on doofers, I may as well spit out an exception right up here – how the heck am I supposed to work with this other type of widget you’ve sent me?
I think the only time to use a soft ‘as’ cast is in the tiny minority of cases where you really don’t care if the result of GetWidget can be turned into a Doofer. It seems to me that 90% of the time, you really do care, and you’d like to know as soon as it’s not. Even in that 10%, you’d probably be better off using a null object.
It’s a shame, because the ‘as’ operator is a bit more readable, and we all hate having to slam all those brackets in everywhere. Fortunately, C# provides a little-known solution to this, known as implicit casting.
Interesting. I’ve always followed the advice given by Don Box in Essential .NET, which is that the as operator (which translates to a CIL isinst instruction) should be preferred to a c# cast (which translates to a CIL castclass instruction) unless you know the cast will succeed because of the cost of exceptions. If your code works specifically on doofers, I’d recommend that you ask for them specifically – i.e. in a function that works specifically on doofers, the argument should be of type doofer. That way it’s the responsibility of the calling code to check it’s going to pass you something you know you can use. In the client code, if there’s a chance that the cast will fail, you only need one test for null after using the as operator and off you go. There is only a problem when you’re downcasting (e.g. going from a base type to a specific subclass) or sidecasting (going from a type to an orthogonal type.) For sidecasting, I prefer converters (e.g. ToString or Convert.ToInt32.) In the case of downcasting, I think you have to consider why it is necessary at all – the usual case being retrieving from a list or some other container that is deliberately unfussy about type – and see if you can refactor (e.g. generics provide greater protection against this type of bug.)
Great points and info Mark, and who am I to argue with the mighty Box, eh?
Don’t get me wrong. I’m not for a moment advocating throwing an exception as part of any expected production execution path. I can’t imagine wanting to… so why worry about the ‘cost’?
The value of a well-placed exception though, for me, is that it identifies clearly to any consumer or modifier of a method that it has violated some contract. In other words, the programmer has made a mistake.
In this case, the code we’re working on takes a ‘widgetId’ (I should have called the parameter dooferId) which *must* be the Id of a doofer for the subsequent lines of code in the method to be meaningful and work.
We could re-write the first case like this:
Doofer doofer = GetWidget(dooferId) as Doofer; if (doofer == null) throw new ArgumentException("dooferId","Must be the ID of a valid Doofer in the system");That works, and maybe it’s more readable than my alternative. I’d have been happy if I’d seen that.
I have to agree with Mark on all-points!
I just happened to find this explanation on the same issue:
http://blog.nerdbank.net/2008/06/when-not-to-use-c-keyword.html
The author shows that using “as” only returns a “NullReferenceException”, where a “()” cast returns an “InvalidCastException” and is a more useful error during debugging. But, he also shows where the “as” could be useful, since it silently returns a null value on failure.
Personally, I’ll take a slightly less “readable” line over potential irritation during debugging!
-Derek