„is” versus try cast with null check

107

Zauważyłem, że Resharper sugeruje, żebym włączył to:

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

zaangażowany w to:

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

Dlaczego miałby sugerować tę zmianę? Jestem przyzwyczajony do sugerowania przez Resharpera zmian optymalizacyjnych i zmian w zakresie redukcji kodu, ale wydaje mi się, że chce wziąć moje pojedyncze oświadczenie i przekształcić je w dwuwierszową.

Według MSDN :

To wyrażenie ma wartość true, jeśli spełnione są oba następujące warunki:

wyrażenie nie jest puste. wyrażenie można rzutować na typ . Oznacza to, że rzutowane wyrażenie formularza (type)(expression)zakończy się bez zgłaszania wyjątku.

Czy źle to czytam, czy też nie isrobię dokładnie tych samych kontroli, tylko w jednym wierszu, bez potrzeby jawnego tworzenia kolejnej zmiennej lokalnej do sprawdzenia wartości null?

HotN
źródło
1
czy używasz myObjRef w dalszej części kodu? jeśli tak, nie będziesz potrzebował MyPropgettera po tej zmianie.
Domyślnie

Odpowiedzi:

147

Ponieważ jest tylko jedna obsada. Porównaj to:

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

do tego:

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 obsługuje bardziej zwartą składnię przy użyciu dopasowania wzorców :

if (myObj.myProp is MyType myObjRef)
{
    ...
}
Jeff E.
źródło
3
dokładnie. użycie 'is' to po prostu zrobienie czegoś takiego jak return ((myProp as MyType) == null)
Bambu
2
Jeśli chodzi o zmiany, jest to całkiem minuta. Sprawdzanie zerowe będzie porównywalne do sprawdzania drugiego typu. asmoże być kilka nanosekund szybciej, ale uważam to za przedwczesną mikrooptymalizację.
Servy
4
Należy również pamiętać, że oryginalna wersja nie jest bezpieczna dla wątków. Wartość myObjlub myPropmoże zostać zmieniona (przez inny wątek) między rzutem isa rzutowaniem, powodując niepożądane zachowanie.
Jeff E
1
Mógłbym również dodać, że użycie as+ != nullspowoduje również wykonanie nadpisanego !=operatora MyTypeif zdefiniowany (nawet jeśli myObjRefjest null). Chociaż w większości przypadków nie stanowi to problemu (zwłaszcza jeśli poprawnie go zaimplementujesz), w niektórych skrajnych przypadkach (zły kod, wydajność) może nie być pożądane. (musiałoby to być jednak dość ekstremalne )
Chris Sinclair
1
@Chris: Zgadza się, poprawne tłumaczenie kodu będzie używane object.ReferenceEquals(null, myObjRef).
Ben Voigt,
10

Najlepszą opcją jest użycie dopasowania wzorców w ten sposób:

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
Francesco Cattoni
źródło
Jak dokładnie ten jest lepszy od drugiego fragmentu pytania?
Victor Yarema
Drugi fragment pytania odnosi się do podstawowego użycia is (bez deklaracji zmiennej) iw takim przypadku dwukrotnie sprawdzisz typ (jeden w instrukcji is, a drugi przed rzutem)
Francesco Cattoni
6

Nie ma jeszcze informacji o tym, co tak naprawdę dzieje się pod pasem. Spójrz na ten przykład:

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

Przekłada się to na następujący 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   

Liczy się tutaj isinsti castclasspołączenia - oba stosunkowo drogie. Jeśli porównasz to z alternatywą, zobaczysz, że tylko isinstsprawdza:

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  

Warto również wspomnieć, że typ wartości będzie używał unbox.anyzamiast 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   

Należy jednak pamiętać, że niekoniecznie przekłada się to na szybszy wynik, jak widać tutaj . Nie wydaje się być poprawa ponieważ pytano jednak: odlewy wydają się być przeprowadzone tak szybko jak kiedyś, ale asi linqobecnie około 3 razy szybciej.

Jeroen Vannevel
źródło
4

Ostrzeżenie dla Resharper:

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

Oba będą działać, zależy to od tego, jak Twój kod bardziej Ci odpowiada. W moim przypadku po prostu ignoruję to ostrzeżenie:

//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();

W moim kodzie druga droga to dłuższa i gorsza wydajność.

Tomek
źródło
1
W twoim prawdziwym przykładzie występuje po prostu problem projektowy. Jeśli kontrolujesz typy, po prostu użyj interfejsu, takiego jak IRunable. Jeśli nie masz kontroli, może przydałbyś się dynamic?
M. Mimpen,
3

Wydaje mi się, że to zależy od tego, jakie są szanse, że będzie tego typu, czy nie. Z pewnością skuteczniejsze byłoby wykonanie rzutu z przodu, gdyby obiekt był tego typu przez większość czasu. Jeśli tylko czasami jest tego typu, bardziej optymalne może być sprawdzenie najpierw za pomocą is.

Koszt utworzenia zmiennej lokalnej jest bardzo znikomy w porównaniu z kosztem sprawdzenia typu.

Czytelność i zakres są dla mnie zazwyczaj ważniejszymi czynnikami. Nie zgodziłbym się z ReSharper i użyłbym operatora „jest” tylko z tego powodu; zoptymalizować później, jeśli jest to prawdziwe wąskie gardło.

(Zakładam, że używasz tylko myObj.myProp is MyTyperaz w tej funkcji)

Bom
źródło
0

Powinien również sugerować drugą zmianę:

(MyType)myObj.myProp

w

myObjRef

Spowoduje to zapisanie dostępu do właściwości i rzutowania w porównaniu z oryginalnym kodem. Ale jest to możliwe tylko po zmianie isna as.

Ben Voigt
źródło
@Default: Nie, nie jest. To nie znaczy, że nie ma go w kodzie.
Ben Voigt
1
przepraszam .. niezrozumiany. jednak (MyType)wyrzuci wyjątek, jeśli rzutowanie się nie powiedzie. astylko zwraca null.
Domyślnie
@Default: rzutowanie nie powiedzie się, ponieważ typ został już sprawdzony is(ten kod jest w pytaniu).
Ben Voigt
1
jednak re # chce zastąpić ten kod - co oznacza, że ​​nie będzie go po sugerowanej zmianie.
Domyślnie
Myślę , że podążam tutaj za twoją myślą (zajęło mi to trochę czasu). Masz na myśli, że pierwsza linia jest gdzieś w kodzie i ta linia zostanie uproszczona po sugestii Re # do drugiej linii?
Domyślnie
0

Powiedziałbym, że ma to na celu utworzenie silnie wpisanej wersji myObj.myProp, która jest myObjRef. Powinno to być następnie używane, gdy odwołujesz się do tej wartości w bloku, zamiast wykonywania rzutowania.

Na przykład to:

myObjRef.SomeProperty

jest lepsze niż to:

((MyType)myObj.myProp).SomeProperty
Jerad Rose
źródło