Wartbarkeit der Booleschen Logik - Wird verschachtelt, wenn Anweisungen benötigt werden?

11

Welche davon ist besser für die Wartbarkeit?

if (byteArrayVariable != null)
    if (byteArrayVariable .Length != 0)
         //Do something with byteArrayVariable 

ODER

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Ich bevorzuge das Lesen und Schreiben des zweiten, aber ich erinnere mich, dass das vollständige Lesen von Code schlecht für die Wartbarkeit ist.

Dies liegt daran, dass Sie sich auf die Sprache verlassen, um den zweiten Teil von nicht zu bewerten, ifwenn der erste Teil falsch ist und nicht alle Sprachen dies tun. (Der zweite Teil löst eine Ausnahme aus, wenn er mit einer Null bewertet wird byteArrayVariable.)

Ich weiß nicht, ob das wirklich ein Grund zur Sorge ist oder nicht, und ich hätte gerne ein allgemeines Feedback zu dieser Frage.

Vielen Dank.

Vaccano
quelle
1
Die erste Form hat das Risiko zu baumeln - sonst
JBRWilkinson
nur neugierig, mit welcher Sprache arbeitest du?
STW
@STW - Wir verwenden C # und haben alten Code in Delphi.
Vaccano
2
gut zu wissen. Ich bin nicht vertraut mit Delphi aber in C # können Sie in den darauf verlassen, &&und ||Betreibern Durchführung Kurzauswertung. Das Wechseln von einer Sprache in eine andere hat normalerweise einige Probleme, und es ist gut, wenn Sie Codeüberprüfungen durchführen, um diese kleinen Probleme zu lösen.
STW
Während ich hier neben vielen Antworten die zweite besser lesbar finde, ist die erste viel einfacher zu debuggen. In diesem Fall würde ich immer noch die zweite verwenden, da Sie überprüfen, ob die Fehlerbehebung einfach ist. Aber sei im Allgemeinen vorsichtig.
Magus

Antworten:

30

Ich denke, die zweite Form ist in Ordnung und repräsentiert auch deutlicher, was Sie versuchen zu tun. Du sagst das...

Ich erinnere mich, dass ich Code vollständig gelesen habe, dass solche Dinge schlecht für die Wartbarkeit sind. Dies liegt daran, dass Sie sich auf die Sprache verlassen, um den zweiten Teil des if nicht zu bewerten, wenn der erste Teil falsch ist und nicht alle Sprachen dies tun.

Es ist egal, ob alle Sprachen das tun. Sie schreiben in einer bestimmten Sprache, daher ist es nur wichtig, ob diese Sprache dies tut. Andernfalls sagen Sie im Wesentlichen, dass Sie die Funktionen einer bestimmten Sprache nicht verwenden sollten, da andere Sprachen diese Funktionen möglicherweise nicht unterstützen, und das ist einfach albern.

Mipadi
quelle
12
Einverstanden. Warum um alles in der Welt würde es mich interessieren, wenn mein Code beim Kopieren in eine andere Sprache gleich ausgeführt wird?
Tim Goodman
16

Ich bin überrascht, dass niemand es erwähnt hat (also hoffentlich verstehe ich die Frage nicht falsch) - aber beide saugen irgendwie!

Die bessere Alternative wäre die Verwendung von Early-Exits (Platzieren einer return-Anweisung früh im Code, wenn kein anderer Code ausgeführt werden soll). Dies setzt voraus, dass Ihr Code relativ gut überarbeitet wurde und dass jede Methode einen präzisen Zweck hat.

An diesem Punkt würden Sie etwas tun wie:

if ((byteArrayVariable == null) || (byteArrayVariable.Length != 0)) {
  return; // nothing to be done, leaving
}

// Conditions met, do something

Es könnte sehr nach Validierung aussehen - und genau das ist es. Es wird überprüft, ob die Bedingungen für die Ausführung der Methode erfüllt sind. Beide von Ihnen angebotenen Optionen haben die tatsächliche Logik innerhalb der Vorbedingungsprüfungen verborgen.

Wenn Ihr Code viele Spaghetti enthält (lange Methoden, viele verschachtelte Wenns mit zwischen ihnen verstreuter Logik usw.), sollten Sie sich auf das größere Problem konzentrieren, Ihren Code vor den kleineren Stilproblemen in Form zu bringen. Abhängig von Ihrer Umgebung und der Schwere des Chaos gibt es einige großartige Tools (z. B. Visual Studio verfügt über eine Refactoring-Funktion "Methode extrahieren").


Wie Jim erwähnt, gibt es noch Raum für Debatten darüber, wie der vorzeitige Ausstieg strukturiert werden kann. Die Alternative zu dem, was oben steht, wäre:

if (byteArrayVariable == null) 
  return; // nothing to be done, leaving

if (byteArrayVariable.Length != 0)
  return;

// Conditions met, do something
STW
quelle
1
Ich bin mit der Rückgabe einverstanden, anstatt die gute Logik zu verpacken, aber dieselben Leute werden dasselbe Argument über die Verwendung von || verwenden wie für &&.
MIA
14

Verlieren Sie keine Zeit zu stoppen versuchen , für jeden nicht diese Bedingung. Wenn Sie die Daten oder Bedingungen disqualifizieren können, tun Sie dies so bald wie möglich.

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
// do something

Auf diese Weise können Sie Ihren Code viel einfacher an neue Bedingungen anpassen

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
if (NewCondition == false) Exit;
if (NewerCondition == true) Exit;
// do something
Michael Riley - AKA Gunny
quelle
2
+1 Ja, ja, ja. Eine so einfache Logik sollte nicht zu schwierigem Code führen. Dein (Fragesteller-) Bauch sollte dir das sagen.
Job
11

Ihr Beispiel ist das perfekte Beispiel dafür, warum die verschachtelten ifsSprachen für die meisten Sprachen, die den booleschen Vergleich ordnungsgemäß unterstützen, ein schlechter Ansatz sind. Durch das Verschachteln ifsentsteht eine Situation, in der Ihre Absicht überhaupt nicht klar ist. Ist es erforderlich, dass die zweite ifunmittelbar auf die erste folgt? Oder liegt es nur daran, dass Sie dort noch keine weitere Operation durchgeführt haben?

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

In diesem Fall diese ziemlich müssen zusammen sein. Sie dienen als Schutz, um sicherzustellen, dass Sie keine Operation ausführen, die zu einer Ausnahme führen würde. Wenn Sie verschachtelt verwenden ifs, überlassen Sie es der Interpretation der Person, die Ihnen folgt, ob die beiden einen zweiteiligen Schutz oder eine andere Verzweigungslogik darstellen. Indem ifich sie zu einer einzigen kombiniere, sage ich das. Es ist viel schwieriger, eine Operation dort hinein zu schleichen, wo sie nicht sein sollte.

Ich werde immer eine klare Kommunikation meiner Absicht gegenüber der dummen Behauptung von jemandem bevorzugen, dass sie "nicht in allen Sprachen funktioniert". Ratet mal, was ... throwfunktioniert auch nicht in allen Sprachen. Heißt das, ich sollte es nicht beim Codieren in C # oder Java verwenden und mich stattdessen auf Rückgabewerte verlassen, um zu signalisieren, wenn ein Problem aufgetreten ist?

Alles in allem ist es viel besser lesbar, es zu invertieren und einfach aus der Methode zurückzukehren, wenn beide nicht zufrieden sind, wie STW in ihrer Antwort sagt.

MIA
quelle
Mehr als zwei Bedingungen können ihre eigene Funktion rechtfertigen, die eine Null zurückgibt.
Job
3

In diesem Fall stehen Ihre beiden Klauseln in direktem Zusammenhang, daher ist die 2. Form vorzuziehen.

Wenn sie nicht verwandt wären (z. B. eine Reihe von Schutzklauseln unter verschiedenen Bedingungen), würde ich die 1. Form bevorzugen (oder eine Methode mit einem Namen umgestalten, der darstellt, was die zusammengesetzte Bedingung tatsächlich darstellt).

FinnNk
quelle
1

Wenn Sie in Delphi arbeiten, ist der erste Weg im Allgemeinen sicherer. (Für das gegebene Beispiel gibt es nicht viel zu gewinnen, wenn verschachtelte ifs verwendet werden.)

Mit Delphi können Sie über Compiler-Schalter festlegen, ob boolesche Ausdrücke ausgewertet oder "kurzgeschlossen" werden sollen. Wenn Sie die erste Methode (verschachtelte ifs) ausführen, können Sie sicherstellen, dass die zweite Klausel genau dann ausgeführt wird, wenn (und genau danach ) die erste Klausel als wahr ausgewertet wird.

Frank Shearar
quelle
1
In 23 Jahren Delphi-Entwicklung habe ich die Option "Complete boolean eval" nie geändert. Wenn ich anderswo einen Versandcode hätte, würde ich {$ BOOLEVAL OFF} oder {$ B-} in das Gerät einfügen.
Gerry
Ich auch nicht. Ich benutze auch immer die Bereichsprüfung ... aber andere Leute nicht. Und es sollte keinen Unterschied machen, außer vielleicht aus Sicht der Leistung, weil Sie keine Nebenwirkungen in Funktionen verwenden sollten, die in booleschen Ausdrücken verwendet werden. Ich bin mir aber sicher, dass es da draußen jemand tut!
Frank Shearar
@Gerry: Einverstanden. Der einzige Grund, warum Sie möglicherweise eine vollständige Bewertung wünschen könnten, sind Nebenwirkungen - und Nebenwirkungen unter bestimmten Bedingungen sind eine schlechte Idee!
Loren Pechtel
Lies einfach meinen Kommentar noch einmal 23 Jahre sollten 13 sein! Ich habe keine Tardis
Gerry
1

Der zweite Stil kann in VBA nach hinten losgehen, da beim zweiten Test, wenn das Objekt generiert wird, der zweite Test und der zweite Fehler weiterhin ausgeführt werden. Ich habe es mir gerade in VBA zur Gewohnheit gemacht, bei der Objektüberprüfung immer die erste Methode zu verwenden.


quelle
2
Das liegt daran, dass VBA eine schreckliche Sprache ist
Falmarri
@ Falmarri, es hat einige schreckliche Dinge (und einige gute Dinge). Ich würde viele Dinge reparieren, wenn ich meine Druthers hätte.
2
Das Fehlen einer booleschen Kurzschlussbewertung ist mehr als alles andere ein Vermächtnis. Keine Ahnung, warum sie nicht damit angefangen haben. Das VB.NET "Fix" von OrElse und AndAlso ist ein bisschen nervig, aber wahrscheinlich die einzige Option, um damit umzugehen, ohne die Abwärtskompatibilität zu beeinträchtigen.
JohnFx
1

Das zweite Formular ist besser lesbar, insbesondere wenn Sie {} Blöcke um jede Klausel verwenden, da das erste Formular zu verschachtelten {} Blöcken und mehreren Einrückungsstufen führt, was beim Lesen schwierig sein kann (Sie müssen Einrückungsräume zählen um herauszufinden, wo jeder Block endet).

Ö. nate
quelle
0

Ich finde beide lesbar und akzeptiere das Argument nicht, dass Sie sich um die Wartbarkeit des Codes sorgen sollten, wenn Sie die Sprache wechseln.

In einigen Sprachen ist die zweite Option besser, daher wäre dies die klare Wahl.

Rechnung
quelle
0

Ich bin bei dir; Ich mache es auf die zweite Weise. Für mich ist es logisch klarer. Die Sorge, dass einige Sprachen den zweiten Teil ausführen, selbst wenn der erste als falsch bewertet wird, erscheint mir ein bisschen albern, da ich noch nie in meinem Leben ein Programm geschrieben habe, das in "einigen Sprachen" lief; Mein Code scheint immer in einer bestimmten Sprache zu existieren, die entweder mit dieser Konstruktion umgehen kann oder nicht. (Und wenn ich nicht sicher bin, ob die jeweilige Sprache damit umgehen kann, muss ich das wirklich lernen, nicht wahr?)

In meinen Augen besteht die Gefahr bei der ersten Möglichkeit darin, dass ich anfällig dafür ifbin , versehentlich Code außerhalb dieses verschachtelten Blocks einzugeben , wenn ich das nicht wollte. Das ist zwar kaum ein großes Problem, aber es scheint vernünftiger, als sich darüber zu ärgern, ob mein Code sprachunabhängig ist.

BlairHippo
quelle
0

Ich bevorzuge die zweite Option, weil sie besser lesbar ist.

Wenn die von Ihnen implementierte Logik komplexer ist (die Überprüfung, ob eine Zeichenfolge nicht null und nicht leer ist, ist nur ein Beispiel), können Sie ein nützliches Refactoring durchführen: Extrahieren Sie eine boolesche Funktion. Ich bin mit @mipadi in der Bemerkung "Code Complete" ("Es spielt keine Rolle, ob alle Sprachen das tun ..."). Wenn der Leser Ihres Codes nicht daran interessiert ist, in die extrahierte Funktion zu schauen, dann die Reihenfolge in denen boolesche Operanden ausgewertet werden, wird für sie zu einer strittigen Frage.

Azheglov
quelle
0
if ((byteArrayVariable != null)
 && (byteArrayVariable.Length != 0)) {
    //Do something with byteArrayVariable 

Aber das ist im Grunde die zweite Option.

Boykott SE für Monica Cellio
quelle