What we sometimes see in C# code

MyClass myClass = SomeFunc() as MyClass;

// just to show we are accessing a string property
Console.WriteLine(myClass.SomeProperty);

What this code does

SomeFunc() is probably declared as  object SomeFunc();

We know that it actually returns an instance of MyClass, so we have to cast to MyClass.

 

Why is this code “not optimal”

Consider this:

• If SomeFunc() returns an object which cannot be cast to MyClass, myClass will be silently set to null.
  In the next line, we get a NullReferenceException.
• If SomeFunc() returns null, myClass will be null too.
  In the next line, we get a NullReferenceException.

So, we cannot distinguish between both cases.

And in either case we get an exception. The safety which as may seem to provide is “wasted” here.

 

 

Other code with C-like cast

MyClass myClass = (MyClass)SomeFunc();

// just to show we are accessing a string property */
Console.WriteLine(myClass.SomeProperty);

If SomeFunc() returns an object which cannot be cast to MyClass, we immediately get an InvalidCastException in the first line.
This is good, since we fail early in this case.

Other example with null test

MyClass myClass = SomeFunc() as MyClass;

if (myClass == null)
{
  ErrorLog("SomeFunc did not properly return a valid object. Exiting now...");
  return;
}

// just to show we are accessing a string property */
Console.WriteLine(myClass.SomeProperty);

The author of this code probably did not fully understand the purpose of exception handling, in a way of “we check every return value and exit if an error occured”.
But at least, the casting with as makes sense here.

Recommendation

I (personally) handle this as follows:
If I am really sure that SomeFunc() returns something castable to MyClass, I use the old C like cast (MyClass).

If some day the cast fails, I have a chance to catch this in the (hopefully) used exception handling code.
And I can distinguish this from a null return.

But if SomeFunc() may return something not castable, I check this with

if (myClass == null)
{

Here the primary question should be “when will this happen, and what will I do in this case”.

Maybe this code part was meant only for tracing purposes, and it is sufficient to just log this and continue.

Links, follow-ups

A good article from Eric Lippert can be found here in Eric’s (old, archived) blog pages

For anyone interested: here is Eric’s new blog.