'is' compared to a null check cast attempt

Question

I noticed that Resharper offers me to do this:

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

in it:

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

Why did he suggest this change? I'm used to Resharper, suggesting changes in optimization and code changes, but it looks like he wants to take my only statement and turn it into a two-line one.

According to MSDN :

expression evaluates to true if both of the following conditions are true:

expression is not equal to zero. expression can be applied to type. This cast form expression (type)(expression) will be completed without throwing an exception.

Am I misunderstanding this or does not perform the same checks on only one line without the need to explicitly create another local variable for a null check?

Answer 1

Because there is 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 ... } 

:

 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) { ... } 

Answer 2

There is no information on what is actually going on below the belt. Take a look at this example:

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

This means 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 

The calls to isinst and castclass are important castclass - and relatively expensive. If you compare this to an alternative, you will see that the isinst check isinst runs:

 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 

It's also worth mentioning that the 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 

Please note that this does not necessarily lead to a faster result, as we can see here . It seems that there have been improvements since this question was asked, though: it seems that the castes execute as fast as before, but as and linq now about 3 times faster.

Answer 3

A better option is to match usage patterns, for example:

 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 

Answer 4

It seems to me that this depends on what are the chances that he will be of this type or not. Of course, it would be more efficient to do a front throw if the object has this type most of the time. If this is only occasionally of this type, then it may be more optimal to check first.

The cost of creating a local variable is very small compared to the cost of type checking.

Readability and reach are the most important factors for me. I would not agree with ReSharper and use the "is" operator only for this reason; optimize later if this is a true bottleneck.

(I assume that you only use myObj.myProp is MyType once in this function)

Answer 5

Reconfiguration Warning:

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

Both will work, it depends on how your code suits you better. In my case, I just ignore this 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, the second way is longer and worse performance.

Answer 6

A second change should also be suggested:

 (MyType)myObj.myProp 

at

 myObjRef 

This saves access to properties and casting compared to the source code. But this is only possible after changing is to as .

Answer 7

I would say that this is to make a strongly typed version of myObj.myProp, which is myObjRef. This should then be used when you refer to this value in a block, not to execution.

For example, this:

 myObjRef.SomeProperty 

better than this:

 ((MyType)myObj.myProp).SomeProperty