BestPractice bei null Handling

Servus,

mir ist gerade eine Frage gekommen:

Handling A:

private void Test(object x)
{
    if (x != null)
    {
        //do something
    }
}

Handling B:

private void Test(object x)
{
    if (x == null)
    {
        return;
    }
    //Do something
}

Gibt es hier ein BestPractice? Hat eine der Versionen Performance-Vorteile?

Danke schonmal!

Guckst Du hier:

https://sharplab.io/#v2:EYLgtghglgdgPgAQEwEYCwAoTCDMACZPAYUwG9M9K8AHAJygDcIAXAUwIBY8AVVgZ2YoAFAHtgAK1YBjZngAeASgpVyGKurxQAZniFy8AQgC8eGAFcANhaVqNlVXbsIUATiEAiABKsrIvAHURWgsAE3cFAG5lOwBfaLw420p4ukYWdgQuXgEkUQlpWUV4h0dtXX0jE3MrG0c8ErrKBAB2KKSNRLr49Wc3Lx8LP0DgsMj4xJigA==

Das Ding zeigt dir, was der Compiler beim Lowering-Schritt macht und siehe da: Es ist technisch identisch 😉
Mach es von der Übersichtlichkeit abhängig, wähle je Situation das, was am übersichtlichsten ist.

Einen (mMn.) Best Practices gibt's aber: Nullable Reference Types
Im private oder internal Code schreibe ich persönlich gar keine Null-Checks mehr, außer ich markiere den Parameter als nullable, dann zwingt (wenn ich es als Fehler behandeln lasse) der Compiler mich dazu.

Im Bestfall sollte man Null ja vermeiden. :p

Ich bevorzuge meistens Handling B, da ich dann auch Nesting vermeiden kann.

Nicht selten finde ich auch noch alten Code von mir, bei dem teilweise 4-5 Ebenen in if Blöcken verschachtelt sind, was den Code sehr unleserlich macht.

T-Virus

Hallo,

der Sharplab Link suggeriert dass beide Varianten zu gleichem Code kompiliert werden. Tatsächlich wird der C#-Code vom Compiler zu IL kompiliert und dort sind die Unterschiede (Variante A / B) zu sehen.
Für den C#-Reiter in Sharplab wird zwar gleicher Code angezeigt, da Sharplab intern das IL zu C# zurückführt (und hier wohl eine semantisch korrekte Repräsentation zeigt, jedoch keine 1:1 Repräsentation).

BTW: Lowering hat mit dieser "Umwandlung" nichts zu tun -- lowering wäre z.B. foreach über Array → for-Schleife usw.

Gibt es hier ein BestPractice?

Sehe ich wie T-Virus. Argument-Validation zu Beginn der Methode und dann entweder entsprechende Fehler schmeißen od. aus Methode aussteigen.

Hat eine der Versionen Performance-Vorteile?

Ist das relevant?
Und falls ja, so spätestens mit .NET 8 nicht mehr, denn dort ist die sogenannte dynamische profilgestützte Optimierung (D-PGO) des JIT aktiviert und dadurch erzeugt der JIT Maschinencode in Abhängigkeit vom tätsächlichen Programm-Ablauf.
Hier im Beispiel (egal ob Variante A od. B) falls x nie od. zumindest selten (< 50 %) null ist, so wird der Block für "null-Fall" in den "kalten" Bereich der Method geschoben. Kurz: die Performance ist in beiden Fällen gleich.

Wie die Frage nach der "Relevanz" schon angedeutet hat, würde ich mir darüber keine Gedanken machen und v.a. den Code leserlich schreiben.
Sollte Profiling, etc. zeigen dass diese Methode kritisch ist, dann (und "nur dann") ist es sinnvoll diese zu optimieren. Ein paar Tricks gibt es dazu.

Wobei ab .NET 7 Leserlichkeit und Perf Hand in Hand gehen bei ein paar Exceptions.
Vor .NET 7:

public int Foo(object x)
{
    if (x is null)
    {
        throw new ArgumentNullException(nameof(x));
    }
    
    return x.GetHashCode();
}

Ab .NET 7:

public int Foo(object x)
{
    ArgumentNullException.ThrowIfNull(x);
    
    return x.GetHashCode();
}

Der Code wird IMO leserlicher und es zugleich weniger Maschinencode vom JIT erzeugt.

Anmerkung: im Beispiel ist das Argumnet object x und nicht object? x (also mit nullable annotations). Da Foo eine public Methode ist und x eben nicht als null erwartet wird (daher keine Annotation) muss auch das Argument validiert werden -- sprich ein Fehler erzeugt werden, falls es doch null ist.
Anders ist es bei folgendem Beispiel:

public int Bar(object? x)
{
    return x is not null
         ? x.GetHashCode
         : 0;    // od. welcher Wert auch immer für `x is null` verwendet werden soll
}

Hier wird null erlaubt, daher muss null auch toliert werden und ein Ergebnis ohne Fehler (Exception) zurückgegeben werden.

Insofern kann ich

keine Null-Checks mehr, außer ich markiere den Parameter als nullable, dann zwingt (wenn ich es als Fehler behandeln lasse) der Compiler mich dazu.

nicht richtig einordnen. Vllt. ist es richtig gemeint od. auch nicht. Jedenfalls so wie ich es beschrieben haben ist es im Sinne von Nullable-Annotations.

Im private oder internal Code schreibe ich persönlich gar keine Null-Checks mehr

Das hat eigentlich noch nie wirklich Sinn gemacht (also Null-Checks in nicht öffentlichen Methoden), da diese ja nur von public Methoden aufgerufen werden können (direkt od. indirekt). Um dennoch potentielle NullReferenceException vorzubeugen kann man

/* internal od. */ private int Foo(object x)
{
    Debug.Assert(x is not null);
    
    return x.GetHashCode();
}

schreiben. Dann wird zumindest beim Debuggen od. falls CI/CD mit einem Debug-Build die Tests auch durchführt dieses Assert ausgeführt. Im Release-Build wird dieses Assert vom C#-Compiler weggelassen, daher auch keine negativen Effekte für die Perf.

mfG Gü

Danke an alle für die guten Erklärungen!

Erste Hälfte deines Beitrags:

Danke für die Richtungstellung.
Dass Sharplab so arbeitet, wusste ich nicht und dass es da tatsächlich einen Unterschied gibt, auch nicht.
Wieder was gelernt

Zweite Hälfte:

nicht richtig einordnen. Vllt. ist es richtig gemeint od. auch nicht. Jedenfalls so wie ich es beschrieben haben ist es im Sinne von Nullable-Annotations.

Ich glaube, ich meine das, was du beschrieben hast.
Worauf ich anspiele (in den Klammern), ist die Option, die warnungen als Fehler zu behandeln.
In dem Fall zwingt mich der Compiler (oder Visual Studio, egal) dazu, das null richtig zu behandeln.

Das hat eigentlich noch nie wirklich Sinn gemacht (also Null-Checks in nicht öffentlichen Methoden), da diese ja nur von public Methoden aufgerufen werden können (direkt od. indirekt).

... und die wiederum prüfen allles sorgfältig, bevor es weiter geht.

Ich meine mit "internal" aber eher, ob das ganze Projekt oder der Teil davon dafür gedacht ist, extern weiterverwendet zu werden, also z.B. bei einem Framework. Wenn nur ich (oder mein Team) daran arbeitet und die Nullable Reference Types richtig verwendet, gibt's auch kaum NullReferenceExceptions. Es kann natürlich sein, dass man irgendwas nutzt, was das nicht unterstützt (z.b. WPF), aber dann muss er erste Aufrufer dafür sorge tragen, dass das nicht null ist, was da weiter gegeben wird.