'is' versus try cast with null check

I noticed that Resharper suggests that I turn this:

if (myObj.myProp is MyType)
{
   ...
}

into this:

var myObjRef = myObj.myProp as MyType;
if (myObjRef != null)
{
   ...
}

Why would it suggest this change? I'm used to Resharper suggesting optimization changes and code reduction changes, but this feels like it wants to take my single statement and turn it into a two-liner.

According to MSDN:

> An is expression evaluates to true if both of the following conditions > are met: > > expression is not null. expression can be cast to type. That is, a > cast expression of the form (type)(expression) will complete without > throwing an exception.

Am I misreading that, or doesn't is do the exact same checks, just in a single line without the need to explicitly create another local variable for the null check?

Because there's only one cast. Compare this:

if (myObj.myProp is MyType) // cast #1
{
    var myObjRef = (MyType)myObj.myProp; // needs to be cast a second time
                                         // before using it as a MyType
    ...
}

to this:

var myObjRef = myObj.myProp as MyType; // only one cast
if (myObjRef != null)
{
    // myObjRef is already MyType and doesn't need to be cast again
    ...
}

---

C# 7.0 supports a more compact syntax using pattern matching:

if (myObj.myProp is MyType myObjRef)
{
    ...
}

The best option is use pattern matching like that:

if (value is MyType casted){
    //Code with casted as MyType
    //value is still the same
}
//Note: casted can be used outside (after) the 'if' scope, too

There's no information yet about what actually happens below the belt. Take a look at this example:

object o = "test";
if (o is string)
{
	var x = (string) o;
}

This translates to the following IL:

IL_0000:  nop         
IL_0001:  ldstr       "test"
IL_0006:  stloc.0     // o
IL_0007:  ldloc.0     // o
IL_0008:  isinst      System.String
IL_000D:  ldnull      
IL_000E:  cgt.un      
IL_0010:  stloc.1     
IL_0011:  ldloc.1     
IL_0012:  brfalse.s   IL_001D
IL_0014:  nop         
IL_0015:  ldloc.0     // o
IL_0016:  castclass   System.String
IL_001B:  stloc.2     // x
IL_001C:  nop         
IL_001D:  ret   

What matters here are the isinst and castclass calls -- both relatively expensive. If you compare that to the alternative you can see it only does an isinst check:

object o = "test";
var oAsString = o as string;
if (oAsString != null)
{
	
}

IL_0000:  nop         
IL_0001:  ldstr       "test"
IL_0006:  stloc.0     // o
IL_0007:  ldloc.0     // o
IL_0008:  isinst      System.String
IL_000D:  stloc.1     // oAsString
IL_000E:  ldloc.1     // oAsString
IL_000F:  ldnull      
IL_0010:  cgt.un      
IL_0012:  stloc.2     
IL_0013:  ldloc.2     
IL_0014:  brfalse.s   IL_0018
IL_0016:  nop         
IL_0017:  nop         
IL_0018:  ret  

Also worth mentioning is that a value type will use unbox.any rather than castclass:

object o = 5;
if (o is int)
{
	var x = (int)o;
}

IL_0000:  nop         
IL_0001:  ldc.i4.5    
IL_0002:  box         System.Int32
IL_0007:  stloc.0     // o
IL_0008:  ldloc.0     // o
IL_0009:  isinst      System.Int32
IL_000E:  ldnull      
IL_000F:  cgt.un      
IL_0011:  stloc.1     
IL_0012:  ldloc.1     
IL_0013:  brfalse.s   IL_001E
IL_0015:  nop         
IL_0016:  ldloc.0     // o
IL_0017:  unbox.any   System.Int32
IL_001C:  stloc.2     // x
IL_001D:  nop         
IL_001E:  ret   

Note however that this not necessarily translates to a faster result as we can see here. There seem to have been improvements since that question was asked though: casts seem to be performed as fast as they used to be but as and linq are now approximately 3 times faster.

Resharper warning:

"Type check and direct cast can be replaced with try cast and check for null"

Both will work, it depends how your code suits you more. In my case I just ignore that warning:

//1st way is n+1 times of casting
if (x is A) ((A)x).Run();
else if (x is B) ((B)x).Run();
else if (x is C) ((C)x).Run();
else if (x is D) ((D)x).Run();
//...
else if (x is N) ((N)x).Run();    
//...
else if (x is Z) ((Z)x).Run();

//2nd way is z times of casting
var a = x as Type A;
var b = x as Type B;
var c = x as Type C;
//..
var n = x as Type N;
//..
var z = x as Type Z;
if (a != null) a.Run();
elseif (b != null) b.Run();
elseif (c != null) c.Run();
...
elseif (n != null) n.Run();
...
elseif (x != null) x.Run();

In my code 2nd way is longer and worse performance.

To me this seems dependent on what the odds are that it's going to be of that type or not. It would certainly be more efficient to do the cast up front if the object is of that type most of the time. If it's only occasionally of that type then it may be more optimal to check first with is.

The cost of creating a local variable is very negligible compared to the cost of the type check.

Readability and scope are the more important factors for me typically. I would disagree with ReSharper, and use the "is" operator for that reason alone; optimize later if this is a true bottleneck.

(I'm assuming that you are only using myObj.myProp is MyType once in this function)

I would say this is to make a strongly-typed version of myObj.myProp, which is myObjRef. This should then be used when you are referencing this value in the block, vs. having to do a cast.

For example, this:

myObjRef.SomeProperty

is better than this:

((MyType)myObj.myProp).SomeProperty

It should be suggesting a second change as well:

(MyType)myObj.myProp

into

myObjRef

This saves a property access and a cast, compared to the original code. But it's only possible after changing is to as.