Kurzschlussbewertung, ist es schlechte Praxis?

Etwas, das ich schon länger kenne, aber nie in Betracht gezogen habe, ist, dass es in den meisten Sprachen möglich ist, Operatoren in einer if-Anweisung basierend auf ihrer Reihenfolge Vorrang einzuräumen. Ich benutze dies oft, um Nullreferenzausnahmen zu verhindern, zB:

if (smartphone != null && smartphone.GetSignal() > 50)
{
   // Do stuff
}

In diesem Fall überprüft der Code zuerst, ob das Objekt nicht null ist, und verwendet dann dieses Objekt, um zu wissen, dass es vorhanden ist. Die Sprache ist klug, weil sie weiß, dass es keinen Sinn macht, die zweite Anweisung zu bewerten, wenn die erste Anweisung falsch ist, und daher wird niemals eine Nullreferenzausnahme ausgelöst. Dies funktioniert genauso für andund orOperatoren.

Dies kann auch in anderen Situationen nützlich sein, z. B. wenn überprüft wird, ob ein Index innerhalb eines Arrays liegt, und eine solche Technik meines Wissens in verschiedenen Sprachen ausgeführt werden kann: Java, C #, C ++, Python und Matlab.

Meine Frage ist: Ist diese Art von Code eine Repräsentation schlechter Praktiken? Entsteht diese schlechte Praxis aus einem versteckten technischen Problem (dh, dies könnte schließlich zu einem Fehler führen) oder führt dies zu einem Lesbarkeitsproblem für andere Programmierer? Könnte es verwirrend sein?

Nein, das ist keine schlechte Praxis. Sich auf das Kurzschließen von Bedingungen zu verlassen, ist eine allgemein akzeptierte, nützliche Technik - solange Sie eine Sprache verwenden, die dieses Verhalten garantiert (einschließlich der überwiegenden Mehrheit der modernen Sprachen).

Ihr Codebeispiel ist ziemlich klar und in der Tat ist dies oft die beste Art, es zu schreiben. Alternativen (wie verschachtelte ifAnweisungen) wären unübersichtlicher, komplizierter und daher schwerer zu verstehen.

Ein paar Vorsichtsmaßnahmen:

Verwenden Sie den gesunden Menschenverstand in Bezug auf Komplexität und Lesbarkeit

Folgendes ist keine so gute Idee:

if ((text_string != null && replace(text_string,"$")) || (text_string = get_new_string()) != null)

Wie viele "clevere" Möglichkeiten, die Zeilen in Ihrem Code zu reduzieren, kann eine übermäßige Abhängigkeit von dieser Technik zu schwer verständlichem, fehleranfälligem Code führen.

Wenn Sie mit Fehlern umgehen müssen, gibt es oft einen besseren Weg

Ihr Beispiel ist in Ordnung, solange es sich bei dem normalen Programmstatus des Smartphones um Null handelt. Wenn dies jedoch ein Fehler ist, sollten Sie eine intelligentere Fehlerbehandlung einbeziehen.

Vermeiden Sie wiederholte Überprüfungen desselben Zustands

Wie Neil betont, weisen viele wiederholte Überprüfungen derselben Variablen unter verschiedenen Bedingungen höchstwahrscheinlich auf einen Konstruktionsfehler hin. Wenn Sie zehn verschiedene Aussagen , die durch den Code gestreut haben , die nicht ausgeführt werden sollen , wenn smartphoneist null, zu prüfen , ob es ein besserer Weg, dies zu handhaben als die Variable jedes Mal überprüft.

Hier geht es nicht wirklich um Kurzschlüsse. Es ist ein allgemeineres Problem von sich wiederholendem Code. Erwähnenswert ist jedoch, dass häufig Code mit vielen wiederholten Anweisungen wie Ihrer Beispielanweisung angezeigt wird.

Angenommen, Sie haben eine C-Sprache ohne verwendet &&und mussten den entsprechenden Code wie in Ihrer Frage ausführen.

Ihr Code wäre:

if(smartphone != null)
{
  if(smartphone.GetSignal() > 50)
  {
    // Do stuff
  }
}

Dieses Muster würde viel auftauchen.

Stellen Sie sich nun die Version 2.0 unserer hypothetischen Sprache vor &&. Denken Sie, wie cool Sie denken würden, dass es war!

&&ist das anerkannte, idiomatische Mittel, um das zu tun, was mein Beispiel oben tut. Es ist keine schlechte Praxis, es zu verwenden, in der Tat ist es eine schlechte Praxis, es in Fällen wie den oben genannten nicht zu verwenden: Ein erfahrener Programmierer in einer solchen Sprache würde sich fragen, wo der elseGrund dafür war, Dinge nicht auf die normale Art und Weise zu tun.

Die Sprache ist klug, weil sie weiß, dass es keinen Sinn macht, die zweite Anweisung zu bewerten, wenn die erste Anweisung falsch ist, und daher wird niemals eine Nullreferenzausnahme ausgelöst.

Nein, Sie sind schlau, denn Sie wussten, dass es keinen Sinn macht, die zweite Aussage zu bewerten, wenn die erste Aussage falsch ist. Die Sprache ist dumm wie eine Schachtel Steine ​​und hat das getan, was Sie ihm gesagt haben. (John McCarthy war noch schlauer und erkannte, dass eine Kurzschlussbewertung eine nützliche Sache für Sprachen sein würde).

Der Unterschied zwischen Ihnen und der Sprache ist wichtig, da gute und schlechte Praxis oft darauf zurückzuführen ist, dass Sie so schlau wie nötig sind, aber nicht schlauer.

Erwägen:

if(smartphone != null && smartphone.GetSignal() > ((range = (range != null ? range : GetRange()) != null && range.Valid ? range.MinSignal : 50)

Dies erweitert Ihren Code, um a zu überprüfen range. Wenn das rangenull ist, versucht es, es durch einen Aufruf auf zu setzen, GetRange()obwohl dies möglicherweise fehlschlägt und daher rangemöglicherweise immer noch null ist. Wenn range danach nicht null ist und es Validdann seine MinSignalEigenschaft ist, wird andernfalls ein Standardwert von 50verwendet.

Dies hängt &&auch davon ab, aber es ist wahrscheinlich zu klug, es in eine Zeile zu setzen (ich bin nicht zu 100% sicher, dass ich es richtig gemacht habe, und ich werde es nicht noch einmal überprüfen, weil diese Tatsache meinen Standpunkt zeigt).

Das &&ist hier jedoch nicht das Problem, aber die Fähigkeit, damit viel in einen Ausdruck zu schreiben (eine gute Sache), erhöht unsere Fähigkeit, schwer verständliche Ausdrücke unnötig zu schreiben (eine schlechte Sache).

Ebenfalls:

if(smartphone != null && (smartphone.GetSignal() == 2 || smartphone.GetSignal() == 5 || smartphone.GetSignal() == 8 || smartPhone.GetSignal() == 34))
{
  // do something
}

Hier verbinde ich die Verwendung von &&mit einer Prüfung auf bestimmte Werte. Dies ist bei Telefonsignalen nicht realistisch, kommt aber in anderen Fällen vor. Hier ist es ein Beispiel dafür, dass ich nicht schlau genug bin. Wenn ich folgendes getan hätte:

if(smartphone != null)
{
  switch (smartphone.GetSignal())
  {
    case 2: case 5: case 8: case 34:
      // Do something
      break;
  }
}

Ich hätte sowohl an Lesbarkeit als auch an Leistung gewonnen (die Mehrfachaufrufe GetSignal()wären wahrscheinlich nicht wegoptimiert worden).

Auch hier ist das Problem nicht so sehr, &&als diesen bestimmten Hammer zu nehmen und alles andere als Nagel zu sehen. Wenn ich es nicht benutze, kann ich etwas Besseres tun als wenn ich es benutze.

Ein letzter Fall, der von der bewährten Praxis abweicht, ist:

if(a && b)
{
  //do something
}

Verglichen mit:

if(a & b)
{
  //do something
}

Das klassische Argument, warum wir das letztere bevorzugen könnten, ist, dass es einige Nebeneffekte bei der Beurteilung gibt b, ob wir wollen, ob aes wahr ist oder nicht. Da bin ich anderer Meinung: Wenn dieser Nebeneffekt so wichtig ist, lassen Sie ihn in einer separaten Codezeile passieren.

In Bezug auf die Effizienz dürfte jedoch eine der beiden Möglichkeiten besser sein. Der erste führt offensichtlich weniger Code aus (überhaupt keine Bewertung bin einem Codepfad), was uns die Zeit ersparen kann, die für die Bewertung erforderlich ist b.

Der erste hat aber noch einen Zweig. Überlegen Sie, ob wir es in unserer hypothetischen C-Sprache mit no neu schreiben &&:

if(a)
{
  if(b)
  {
    // Do something
  }
}

Dieses Extra ifist in unserer Verwendung von versteckt &&, aber es ist immer noch da. Als solcher ist es ein Fall, in dem eine Verzweigungsvorhersage stattfindet und daher möglicherweise eine Verzweigungsfehlvorhersage erfolgt.

Aus diesem Grund kann der if(a & b)Code in einigen Fällen effizienter sein.

Hier würde ich sagen, dass dies zunächst if(a && b)immer noch die beste Vorgehensweise ist: Häufiger, in einigen Fällen die einzige, die durchführbar ist (wo bFehler auftreten, wenn sie afalsch ist) und häufiger als nicht. Es ist jedoch erwähnenswert, dass if(a & b)dies in bestimmten Fällen häufig eine nützliche Optimierung darstellt.

Sie können dies weiterführen, und in einigen Sprachen ist dies die idiomatische Vorgehensweise: Sie können die Kurzschlussberechnung auch außerhalb von bedingten Anweisungen verwenden, und sie werden selbst zu einer Form von bedingten Anweisungen. Zum Beispiel ist es in Perl für Funktionen idiomatisch, im Fehlerfall etwas Falsches (und im Erfolgsfall etwas Wahres) und so etwas zurückzugeben

open($handle, "<", "filename.txt") or die "Couldn't open file!";

ist idiomatisch. openGibt im Erfolgsfall etwas ungleich Null zurück (so dass der dieTeil danach ornie passiert), im Fehlerfall jedoch undefiniert, und dann geschieht der Aufruf von die(das ist Perls Weg, eine Ausnahme auszulösen).

Das ist gut in Sprachen, in denen jeder es tut.

Obwohl ich der Antwort von dan1111 im Allgemeinen zustimme , gibt es einen besonders wichtigen Fall, den es nicht abdeckt: den Fall, in dem smartphonegleichzeitig verwendet wird. In diesem Szenario ist diese Art von Muster eine bekannte Quelle für schwer zu findende Fehler.

Das Problem ist, dass die Kurzschlussbewertung nicht atomar ist. Ihr Thread kann , dass der Check smartphoneheißt null, kann ein anderer Thread kommen und zunichte machen es, und dann Thread versucht umgehend GetSignal()und sprengt.

Aber das geht natürlich nur, wenn die Sterne ausgerichtet sind und das Timing Ihrer Fäden genau richtig ist.

Obwohl diese Art von Code sehr verbreitet und nützlich ist, ist es wichtig, über diese Einschränkung Bescheid zu wissen, damit Sie diesen bösen Fehler genau verhindern können.

Es gibt viele Situationen, in denen ich eine Bedingung zuerst überprüfen und eine zweite Bedingung nur dann überprüfen möchte, wenn die erste Bedingung erfolgreich war. Manchmal nur aus Effizienzgründen (weil es keinen Sinn macht, die zweite Bedingung zu überprüfen, wenn die erste bereits fehlgeschlagen ist), manchmal, weil mein Programm sonst abstürzen würde (Ihre Prüfung auf NULL zuerst), manchmal, weil die Ergebnisse sonst schrecklich wären. (WENN (Ich möchte ein Dokument drucken) UND (Drucken eines Dokuments fehlgeschlagen)) DANN wird eine Fehlermeldung angezeigt - Sie möchten das Dokument nicht drucken und prüfen, ob das Drucken fehlgeschlagen ist, wenn Sie kein Dokument im Dialogfeld drucken möchten erster Platz!

Es bestand also ein RIESIGER Bedarf an einer garantierten Kurzschlussbewertung logischer Ausdrücke. Es war nicht in Pascal vorhanden (das keine garantierte Kurzschlussbewertung hatte), was ein großer Schmerz war; Ich habe es zuerst in Ada und C gesehen.

Wenn Sie wirklich der Meinung sind, dass dies eine "schlechte Praxis" ist, nehmen Sie bitte ein mäßig komplexes Stück Code und schreiben Sie es neu, damit es ohne Kurzschlussbewertung funktioniert. Das ist, als würde man sagen, dass beim Atmen Kohlendioxid entsteht und man fragt, ob das Atmen eine schlechte Praxis ist. Probieren Sie es ein paar Minuten lang aus.

Eine Überlegung, die in anderen Antworten nicht erwähnt wird: Manchmal deuten diese Überprüfungen auf eine mögliche Umgestaltung des Null-Objekt- Entwurfsmusters hin. Zum Beispiel:

if (currentUser && currentUser.isAdministrator()) 
  doSomething();

Könnte vereinfacht werden:

if (currentUser.isAdministrator())
  doSomething ();

Wenn currentUserstandardmäßig ein "anonymer Benutzer" oder ein "Nullbenutzer" mit einer Fallback-Implementierung verwendet wird, ist der Benutzer nicht angemeldet.

Nicht immer ein Codegeruch, aber etwas zu beachten.

Ich werde unbeliebt sein und sagen, ja, es ist eine schlechte Übung. WENN die Funktionalität Ihres Codes darauf beruht, bedingte Logik zu implementieren.

dh

 if(quickLogicA && slowLogicB) {doSomething();}

ist gut, aber

if(logicA && doSomething()) {andAlsoDoSomethingElse();}

ist schlecht, und sicherlich würde niemand zustimmen ?!

if(doSomething() || somethingElseIfItFailed() && anotherThingIfEitherWorked() & andAlwaysThisThing())
{/* do nothing */}

Begründung:

Ihre Code-Fehler, wenn beide Seiten ausgewertet werden, anstatt einfach die Logik nicht auszuführen. Es wurde argumentiert, dass dies kein Problem ist. Der Operator 'und' ist in c # definiert, sodass die Bedeutung klar ist. Ich behaupte jedoch, dass dies nicht wahr ist.

Grund 1. Historisch

Im Wesentlichen verlassen Sie sich auf eine Compileroptimierung (was ursprünglich vorgesehen war), um Funktionen in Ihrem Code bereitzustellen.

Obwohl in c # die Sprachspezifikation garantiert, dass der Boolesche Operator 'und' einen Kurzschluss ausführt. Dies gilt nicht für alle Sprachen und Compiler.

wikipeda listet verschiedene Sprachen auf und nimmt Kurzschlüsse auf http://en.wikipedia.org/wiki/Short-circuit_evaluation, da es viele Ansätze gibt. Und ein paar zusätzliche Probleme, auf die ich hier nicht eingehen möchte.

Insbesondere haben FORTRAN, Pascal und Delphi Compiler-Flags, die dieses Verhalten umschalten.

Daher: Möglicherweise finden Sie, dass Ihr Code funktioniert, wenn er für die Veröffentlichung kompiliert wird, jedoch nicht für das Debuggen oder mit einem alternativen Compiler usw.

Grund 2. Sprachunterschiede

Wie Sie in Wikipedia sehen können, gehen nicht alle Sprachen beim Kurzschließen gleich vor, auch wenn sie festlegen, wie die Booleschen Operatoren damit umgehen sollen.

Insbesondere die Operatoren 'und' von VB.Net schließen nicht kurz, und TSQL weist einige Besonderheiten auf.

Angesichts der Tatsache, dass eine moderne "Lösung" wahrscheinlich eine Reihe von Sprachen wie Javascript, Regex, Xpath usw. enthält, sollten Sie dies berücksichtigen, wenn Sie Ihre Codefunktionalität klarstellen.

Grund 3. Unterschiede innerhalb einer Sprache

Zum Beispiel verhält sich das Inline-If von VB anders als das If. Auch in modernen Anwendungen kann Ihr Code zwischen beispielsweise Code Behind und einem Inline-Webformular wechseln. Sie müssen sich der Bedeutungsunterschiede bewusst sein.

Grund 4. Ihr spezielles Beispiel für die Nullprüfung des Parameters einer Funktion

Dies ist ein sehr gutes Beispiel. Insofern ist es etwas, was jeder tut, aber wenn man darüber nachdenkt, ist es eine schlechte Praxis.

Diese Art der Prüfung ist sehr häufig anzutreffen, da sie Ihre Codezeilen erheblich reduziert. Ich bezweifle, dass irgendjemand dies in einer Codeüberprüfung erwähnen würde. (und offensichtlich aus den Kommentaren und der Bewertung können Sie sehen, dass es beliebt ist!)

Es ist jedoch aus mehr als einem Grund nicht erfolgreich!

1. Seine sehr klar aus zahlreichen Quellen, dass Best Practice ist Ihre Parameter auf Gültigkeit zu überprüfen , bevor Sie sie verwenden und eine Ausnahme zu werfen , wenn sie ungültig sind. Ihr Code-Snippet zeigt den umgebenden Code nicht an, aber Sie sollten wahrscheinlich Folgendes tun:

   if (smartphone == null)
   {
       //throw an exception
   }
   // perform functionality

2. Sie rufen eine Funktion innerhalb der bedingten Anweisung auf. Obwohl der Name Ihrer Funktion impliziert, dass lediglich ein Wert berechnet wird, können Nebenwirkungen ausgeblendet werden. z.B

   Smartphone.GetSignal() { this.signal++; return this.signal; }
   
   else if (smartphone.GetSignal() < 1)
   {
       // Do stuff 
   }
   else if (smartphone.GetSignal() < 2)
   {
       // Do stuff 
   }

   Best Practice würde vorschlagen:

   var s = smartphone.GetSignal();
   if(s < 50)
   {
       //do something
   }

Wenn Sie diese bewährten Methoden auf Ihren Code anwenden, sehen Sie, dass Ihre Möglichkeit, durch die Verwendung der versteckten Bedingung kürzeren Code zu erzielen, verschwindet. Die beste Vorgehensweise ist, etwas zu tun , um den Fall zu behandeln, in dem das Smartphone null ist.

Ich denke, Sie werden feststellen, dass dies auch für andere gebräuchliche Verwendungen der Fall ist. Sie müssen sich daran erinnern, dass wir auch haben:

Inline-Bedingungen

var s = smartphone!=null ? smartphone.GetSignal() : 0;

und Null-Koaleszenz

var s = smartphoneConnectionStatus ?? "No Connection";

die eine ähnliche Funktionalität für kurze Codelängen mit mehr Klarheit bieten

zahlreiche links zur unterstützung aller meiner punkte:

https://stackoverflow.com/questions/1158614/what-is-the-best-practice-in-case-one-argument-is-null

Validierung von Konstruktorparametern in C # - Best Practices

Sollte man auf null prüfen, wenn er nicht null erwartet?

https://msdn.microsoft.com/en-us/library/seyhszts%28v=vs.110%29.aspx

https://stackoverflow.com/questions/1445867/why-would-a-language-not-use-short-circuit-evaluation

http://orcharddojo.net/orchard-resources/Library/DevelopmentGuidelines/BestPractices/CSharp

Beispiele für Compiler-Flags, die sich auf das Kurzschließen auswirken:

Fortran: "sce | nosce Standardmäßig führt der Compiler in ausgewählten logischen Ausdrücken eine Kurzschlussbewertung nach XL Fortran-Regeln durch. Durch Angabe von sce kann der Compiler Nicht-XL Fortran-Regeln verwenden. Der Compiler führt eine Kurzschlussbewertung durch, wenn die aktuellen Regeln dies zulassen Die Standardeinstellung ist "nosce".

Pascal: "B - Eine Flag-Direktive, die die Kurzschlussauswertung steuert. Weitere Informationen zur Kurzschlussauswertung finden Sie unter Reihenfolge der Auswertung der Operanden von dyadischen Operatoren. Weitere Informationen finden Sie unter der Compiler-Option -sc für den Befehlszeilen-Compiler ( dokumentiert im Benutzerhandbuch). "

Delphi: "Delphi unterstützt standardmäßig die Kurzschlussauswertung. Sie kann mit der Compiler-Direktive {$ BOOLEVAL OFF} ausgeschaltet werden."