Erhöhen andere Blöcke die Codekomplexität? [geschlossen]

Hier ist ein sehr vereinfachtes Beispiel . Dies ist nicht unbedingt eine sprachspezifische Frage, und ich fordere Sie auf, die vielen anderen Möglichkeiten zu ignorieren, mit denen die Funktion geschrieben werden kann, sowie die Änderungen, die daran vorgenommen werden können. . Die Farbe ist einzigartig

string CanLeaveWithoutUmbrella()
{
    if(sky.Color.Equals(Color.Blue))
    {
        return "Yes you can";
    }
    else
    {
        return "No you can't";
    }
}

Eine Menge Leute, die ich getroffen habe, ReSharper, und dieser Typ (dessen Kommentar mich daran erinnerte, dass ich das schon seit einiger Zeit fragen wollte) würden empfehlen, den Code umzugestalten, um den folgenden elseBlock zu entfernen :

(Ich kann mich nicht erinnern, was die Mehrheit gesagt hat, ich hätte das vielleicht nicht anders gefragt.)

string CanLeaveWithoutUmbrella()
{
    if(sky.Color.Equals(Color.Blue))
    {
        return "Yes you can";
    }
    return "No you can't";
}

Frage: Wird die Komplexität erhöht, wenn der elseBlock nicht berücksichtigt wird?

Ich habe den Eindruck else, dass der Code in beiden Blöcken direkt verwandt ist.

Außerdem finde ich, dass ich subtile Fehler in der Logik verhindern kann, insbesondere nach späteren Änderungen am Code.

Nehmen Sie diese Variante meines vereinfachten Beispiels (ignorieren Sie die Tatsache, dass der orOperator ein absichtlich vereinfachtes Beispiel ist):

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

Jemand kann jetzt einen neuen ifBlock basierend auf einer Bedingung nach dem ersten Beispiel hinzufügen, ohne sofort richtig zu erkennen, dass die erste Bedingung eine Einschränkung für seine eigene Bedingung darstellt.

Wenn ein elseBlock vorhanden war, wer auch immer die neue Bedingung , den Inhalt des bewegen zu gehen gezwungen sein würde , hinzugefügt elseBlockes (und wenn sie irgendwie beschönigen es Heuristik zeigt der Code nicht erreichbar ist, die es nicht im Fall einer tut ifbeschränke andere) .

Natürlich gibt es auch andere Möglichkeiten, wie das spezifische Beispiel definiert werden sollte. All dies verhindert diese Situation, aber es ist nur ein Beispiel.

Die Länge des von mir angegebenen Beispiels kann den visuellen Aspekt verzerren. Nehmen wir also an, dass der Platz, der bis zu den Klammern beansprucht wird, für den Rest der Methode relativ unbedeutend ist.

Ich habe vergessen , einen Fall zu erwähnen , in dem ich mit dem Weglassen eines anderen Block einverstanden sind , und wird bei Verwendung eines ifBlock eine Einschränkung anzuwenden, müssen logisch für erfüllt alle folgenden Code, wie eine Null-Prüfung (oder andere Wachen) .

Aus meiner Sicht ist der explizite else-Block vorzuziehen. Wenn ich das sehe:

if (sky.Color != Blue) {
   ...
}  else {
   ...
}

Ich weiß, dass es sich um sich gegenseitig ausschließende Optionen handelt. Ich muss nicht lesen, was sich in den if-Blöcken befindet, um es zu erkennen.

Wenn ich das sehe:

if (sky.Color != Blue) {
   ...
} 
return false;

Auf den ersten Blick sieht es so aus, als ob es trotzdem false zurückgibt, und der Himmel ist optional nicht blau.

Wie ich es sehe, spiegelt die Struktur des zweiten Codes nicht wider, was der Code tatsächlich tut. Das ist schlecht. Wählen Sie stattdessen die erste Option, die die logische Struktur widerspiegelt. Ich möchte nicht in jedem Block nach Return / Break / Continue-Logik suchen müssen, um den logischen Ablauf zu kennen.

Warum jemand den anderen bevorzugen würde, ist mir ein Rätsel, hoffentlich wird jemand die gegenteilige Position einnehmen und mich mit seiner Antwort aufklären.

Ich habe vergessen, einen Fall zu erwähnen, in dem ich mit dem Weglassen eines else-Blocks einverstanden bin. Wenn ich einen if-Block verwende, um eine Einschränkung anzuwenden, die für den folgenden Code logisch erfüllt sein muss, z ).

Ich würde sagen, dass die Wachbedingungen in Ordnung sind, wenn Sie den glücklichen Pfad verlassen haben. Es ist ein Fehler aufgetreten, der verhindert, dass die normale Ausführung fortgesetzt wird. In diesem Fall sollten Sie eine Ausnahme auslösen, einen Fehlercode zurückgeben oder was auch immer für Ihre Sprache geeignet ist.

Kurz nach der ursprünglichen Version dieser Antwort wurde in meinem Projekt folgender Code entdeckt:

Foobar merge(Foobar left, Foobar right) {
   if(!left.isEmpty()) {
      if(!right.isEmpty()) {
          Foobar merged = // construct merged Foobar
          // missing return merged statement
      }
      return left;
   }
   return right;
}

Indem die Rückgabe nicht innerhalb von elses platziert wurde, wurde die Tatsache übersehen, dass eine Rückgabeerklärung fehlte. Wäre noch jemand anders beschäftigt gewesen, hätte der Code nicht einmal kompiliert. (Die weitaus größere Sorge, die ich habe, ist natürlich, dass die Tests, die in diesem Code geschrieben wurden, ziemlich schlecht waren, um dieses Problem nicht zu erkennen.)

Der Hauptgrund für das Entfernen des elseBlockes, den ich gefunden habe, ist übermäßiges Einrücken. Das Kurzschließen von elseBlöcken ermöglicht eine viel flachere Codestruktur.

Viele ifAussagen sind im Wesentlichen Wachen. Sie verhindern die Dereferenzierung von Nullzeigern und andere "Tun Sie das nicht!" fehler. Und sie führen zu einer schnellen Beendigung der aktuellen Funktion. Beispielsweise:

if (obj === NULL) {
    return NULL;
}
// further processing that now can assume obj attributes are set

Nun mag es scheinen, dass eine elseKlausel hier sehr vernünftig wäre:

if (obj === NULL) {
    return NULL;
}
else if (obj.color != Blue) {
   // handle case that object is not blue
}

Und in der Tat, wenn das alles ist, was Sie tun, ist es nicht viel mehr eingerückt als das obige Beispiel. Dies ist jedoch selten alles, was Sie mit echten mehrstufigen Datenstrukturen tun (eine Bedingung, die bei der Verarbeitung gängiger Formate wie JSON, HTML und XML ständig auftritt). Wenn Sie also den Text aller untergeordneten Elemente eines bestimmten HTML-Knotens ändern möchten, sagen Sie:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
p = elements[0]
if ((p.children === NULL) or (p.children.length == 0)) {
    return NULL;
}
for (c in p.children) {
    c.text = c.text.toUpperCase();
}
return p;

Die Wachen erhöhen die Einrückung des Codes nicht. Mit einer elseKlausel wird die gesamte Arbeit nach rechts verschoben:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
else {
    p = elements[0]
    if ((p.children === NULL) or (p.children.length == 0)) {
        return NULL;
    }
    else {
        for (c in p.children) {
            c.text = c.text.toUpperCase();
        }
        return p;
    }
}

Dies beginnt eine signifikante Rechtsbewegung zu haben, und dies ist ein ziemlich triviales Beispiel mit nur zwei Schutzbedingungen. Jeder zusätzliche Wächter fügt eine weitere Einrückungsstufe hinzu. Echter Code, den ich geschrieben habe, um XML zu verarbeiten, oder der detaillierte JSON-Feeds verwendet, kann problemlos 3, 4 oder mehr Schutzbedingungen aufbauen. Wenn Sie die immer verwenden else, werden Sie 4, 5 oder 6 Ebenen eingerückt. Möglicherweise mehr. Und das trägt definitiv zur Komplexität des Codes bei, und es wird mehr Zeit darauf verwendet, zu verstehen, was mit was übereinstimmt. Die schnelle, flache Art des vorzeitigen Ausstiegs beseitigt einen Teil dieser "überschüssigen Struktur" und scheint einfacher zu sein.

Nachtrag Die Kommentare zu dieser Antwort haben mich darauf aufmerksam gemacht, dass möglicherweise nicht klar war, dass die Behandlung mit NULL / Leer nicht der einzige Grund für Schutzbedingung ist. Nicht annähernd! Während sich dieses einfache Beispiel auf die Behandlung mit NULL / Leerzeichen konzentriert und keine anderen Gründe enthält, hat das Durchsuchen tiefer Strukturen wie XML und ASTs nach "relevanten" Inhalten häufig eine lange Reihe spezifischer Tests, um irrelevante und uninteressante Knoten auszusondern Fälle. Eine Reihe von "Wenn dieser Knoten nicht relevant ist, Rückgabetests" führt zu der gleichen Art von Rechtsabweichung wie NULL- / Leer-Guards. Und in der Praxis werden nachfolgende Relevanztests häufig mit NULL / Leer-Guards für die entsprechend tieferen Datenstrukturen gekoppelt - ein Doppelschlag für Rechtsdrift.

Wenn ich das sehe:

if(something){
    return lamp;
}
return table;

Ich sehe eine gemeinsame Idiom . Ein allgemeines Muster , das sich in meinem Kopf wie folgt übersetzt:

if(something){
    return lamp;
}
else return table;

Da dies eine sehr verbreitete Programmiersprache ist, hat der Programmierer, der es gewohnt ist, diese Art von Code zu sehen, immer eine implizite "Übersetzung" in seinem Kopf, die ihn in die richtige Bedeutung "umwandelt".

Ich brauche das Explizite nicht else, mein Kopf „legt es dort hin“, weil er das Muster als Ganzes erkannt hat . Ich verstehe die Bedeutung des gesamten gemeinsamen Musters, daher benötige ich keine kleinen Details, um es zu verdeutlichen.

Lesen Sie beispielsweise den folgenden Text:

Hast du das gelesen?

Ich liebe Paris im Frühling

Wenn ja, liegen Sie falsch. Lese den Text erneut. Was tatsächlich geschrieben ist

Ich liebe Paris im dem Frühling

Es gibt zwei „ die“ Wörter im Text. Warum haben Sie einen der beiden vermisst?

Es ist, weil Ihr Gehirn ein gemeinsames Muster sah und es sofort in seine bekannte Bedeutung übersetzte. Es musste nicht alles Detail für Detail durchgelesen werden, um die Bedeutung herauszufinden, da es das bereits erkannte Muster bereits beim Sehen . Deshalb ignorierte es das Extra "the ".

Dasselbe gilt für die obige Redewendung. Programmierer, die viel Code gelesen haben, sind es gewohnt, dieses Muster zu erkennenif(something) {return x;} return y; . Sie brauchen keine Erklärung, um elsezu verstehen, was es bedeutet, denn ihr Gehirn „legt es für sie da“. Sie erkennen es als ein Muster mit einer bekannten Bedeutung: "Was nach der schließenden Klammer ist, wird nur implizit elsevon der vorherigen ausgeführt if".

Also meiner Meinung nach ist das elseunnötig. Verwenden Sie jedoch nur das, was besser lesbar erscheint.

Sie fügen dem Code visuelle Unordnung hinzu, also können sie die Komplexität erhöhen.

Das Gegenteil ist der Fall. Übermäßiges Refactoring zur Reduzierung der Codelänge kann jedoch auch die Komplexität erhöhen (natürlich nicht in diesem einfachen Fall).

Ich stimme der Aussage zu, dass der elseBlock die Absicht angibt. Mein Fazit ist jedoch anders, weil Sie Ihrem Code Komplexität hinzufügen, um dies zu erreichen.

Ich bin mit Ihrer Aussage nicht einverstanden, dass Sie damit subtile Fehler in der Logik verhindern können.

Schauen wir uns ein Beispiel an, jetzt sollte es nur dann wahr sein, wenn der Himmel blau und die Luftfeuchtigkeit hoch ist, die Luftfeuchtigkeit nicht hoch ist oder wenn der Himmel rot ist. Aber dann verwechseln Sie eine Klammer und platzieren sie falsch else:

bool CanLeaveWithoutUmbrella() {
    if(sky.Color == Color.Blue)
    {
        if (sky.Humidity == Humidity.High) 
        {
            return true;
        } 
    else if (sky.Humidity != Humidity.High)
    {
        return true;
    } else if (sky.Color == Color.Red) 
    {
            return true;
    }
    } else 
    {
       return false;
    }
}

Wir haben all diese dummen Fehler im Produktionscode gesehen und können sehr schwer zu erkennen sein.

Ich würde folgendes vorschlagen:

Ich finde das leichter zu lesen, weil ich auf einen Blick sehe, dass der Standardwert ist false.

Die Idee, den Inhalt eines Blocks zu verschieben, um eine neue Klausel einzufügen, ist fehleranfällig. Dies hängt irgendwie mit dem Open / Closed-Prinzip zusammen (Code sollte für Erweiterungen offen, für Änderungen jedoch geschlossen sein). Sie müssen den vorhandenen Code ändern (z. B. kann der Codierer einen Fehler beim Ausgleichen von Klammern verursachen).

Schließlich führen Sie die Leute dazu, auf eine vorgegebene Weise zu antworten, die Sie für die richtige halten. Sie stellen zum Beispiel ein sehr einfaches Beispiel vor, erklären aber anschließend, dass die Leute es nicht berücksichtigen sollten. Die Einfachheit des Beispiels wirkt sich jedoch auf die gesamte Frage aus:

• Wenn der Fall so einfach ist wie der vorgestellte, dann würde meine Antwort darin bestehen, irgendwelche if elseKlauseln überhaupt wegzulassen .

  return (sky.Color == Color.Blue);

• Wenn der Fall etwas komplizierter ist, würde ich mit verschiedenen Klauseln antworten:

  bool CanLeaveWithoutUmbrella () {

  if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
      return true;
  } else if (sky.Humidity != Humidity.High) {
      return true;
  } else if (sky.Color == Color.Red) {
      return true;
  }
  
  return false;
  

  }

• Wenn der Fall kompliziert ist und nicht alle Klauseln true zurückgeben (möglicherweise wird ein double zurückgegeben) und ich einen Haltepunkt- oder einen Loggin-Befehl einführen möchte, würde ich Folgendes in Betracht ziehen:

  double CanLeaveWithoutUmbrella() {
  
      double risk = 0.0;
  
      if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
          risk = 1.0;
      } else if (sky.Humidity != Humidity.High) {
          risk = 0.5;
      } else if (sky.Color == Color.Red) {
          risk = 0.8;
      }
  
      Log("value of risk:" + risk);
      return risk;
  

  }

• Wenn es sich nicht um eine Methode handelt, die etwas zurückgibt, sondern um einen if-else-Block, der eine Variable setzt oder eine Methode aufruft, würde ich eine Schlussklausel elseeinfügen.

Daher ist auch die Einfachheit des Beispiels zu berücksichtigen.

Obwohl der beschriebene If-else-Fall die größere Komplexität aufweist, spielt er in den meisten (aber nicht allen) praktischen Situationen keine große Rolle.

Als ich vor Jahren an Software arbeitete, die für die Luftfahrtindustrie verwendet wurde (sie musste einen Zertifizierungsprozess durchlaufen), stellte sich Ihre Frage. Es stellte sich heraus, dass diese "else" -Anweisung mit finanziellen Kosten verbunden war, da sie die Komplexität des Codes erhöhte.

Hier ist der Grund.

Das Vorhandensein des "Sonst" erzeugte einen impliziten dritten Fall, der ausgewertet werden musste - weder das "Wenn" noch das "Sonst", das ergriffen wurde. Sie könnten denken (wie ich zu der Zeit war) WTF?

Die Erklärung ging so ...

Aus logischer Sicht gibt es nur die beiden Möglichkeiten - das "Wenn" und das "Sonst". Was wir jedoch zertifizierten, war die Objektdatei, die der Compiler generierte. Wenn es also ein "sonst" gab, musste eine Analyse durchgeführt werden, um zu bestätigen, dass der generierte Verzweigungscode korrekt war und kein mysteriöser dritter Pfad auftauchte.

Stellen Sie dies dem Fall gegenüber, in dem es nur ein "Wenn" und kein "Sonst" gibt. In diesem Fall gibt es nur zwei Optionen: den Wenn-Pfad und den Nicht-Wenn-Pfad. Zwei Fälle sind weniger komplex als drei.

Hoffe das hilft.

Das wichtigste Ziel von Code ist es, verständlich als festgeschrieben zu sein (im Gegensatz zu leicht umgestaltbar, was nützlich, aber weniger wichtig ist). Ein etwas komplexeres Python-Beispiel kann verwendet werden, um dies zu veranschaulichen:

def value(key):
    if key == FROB:
        return FOO
    elif key == NIK:
        return BAR
    [...]
    else:
        return BAZ

Dies ist ziemlich klar - es ist das Äquivalent einer caseAnweisung oder einer Dictionary-Suche, der übergeordneten Version von return foo == bar:

KEY_VALUES = {FROB: FOO, NIK: BAR, [...]}
DEFAULT_VALUE = BAZ

def value(key):
    return KEY_VALUES.get(key, default=DEFAULT_VALUE)

Dies ist klarer, denn obwohl die Anzahl der Token höher ist (32 gegenüber 24 für den ursprünglichen Code mit zwei Schlüssel / Wert-Paaren), ist die Semantik der Funktion jetzt explizit: Es handelt sich nur um eine Suche.

(Dies hat Konsequenzen für das Refactoring. Wenn Sie einen Nebeneffekt key == NIKhaben möchten, haben Sie drei Möglichkeiten:

1. Revert zum if/ elif/ elseStil und den Einsatz einer einzigen Zeile im key == NIKFall.
2. Speichern Sie das Nachschlageergebnis in einem Wert und fügen Sie vor der Rückgabe eine ifAnweisung valuefür den Sonderfall hinzu.
3. Geben Sie eine ifErklärung in den Anrufer ein.

Jetzt sehen wir, warum die Nachschlagefunktion eine mächtige Vereinfachung ist: Sie macht die dritte Option offensichtlich zur einfachsten Lösung, da sie nicht nur zuvalue einem geringeren Unterschied als die erste Option führt, sondern auch als einzige dafür sorgt, dass nur eine Sache getan wird. )

Ich gehe auf den Code von OP zurück und denke, dies kann als Leitfaden für die Komplexität von elseAnweisungen dienen: Code mit einer expliziten elseAnweisung ist objektiv komplexer, aber wichtiger ist, ob dadurch die Semantik des Codes klar und einfach wird. Und das muss von Fall zu Fall beantwortet werden, da jeder Code eine andere Bedeutung hat.

Ich denke, es hängt davon ab, welche Art von ifAussage Sie verwenden. Einige ifAnweisungen können als Ausdrücke betrachtet werden, andere nur als Kontrollflussanweisungen.

Ihr Beispiel sieht für mich wie ein Ausdruck aus. In C-ähnlichen Sprachen ?:ähnelt der ternäre Operator einer ifAnweisung, ist jedoch ein Ausdruck, und der else-Teil kann nicht weggelassen werden. Es ist sinnvoll, einen else-Zweig in einem Ausdruck nicht auszulassen, da der Ausdruck immer einen Wert haben muss.

Mit dem ternären Operator würde Ihr Beispiel so aussehen:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue)
               ? true
               : false;
}

Da es sich jedoch um einen booleschen Ausdruck handelt, sollte er wirklich vereinfacht werden

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}

Das Einfügen einer expliziten else-Klausel macht Ihre Absicht klarer. Wächter können in manchen Situationen sinnvoll sein, aber wenn sie zwischen zwei möglichen Werten wählen, von denen keiner außergewöhnlich oder ungewöhnlich ist, helfen sie nicht.

Eine andere Sache, über die man in diesem Argument nachdenken sollte, sind die Gewohnheiten, die jeder Ansatz fördert.

• if / else formuliert ein Codierungsmuster in Form von Verzweigungen um. Wie Sie sagen, manchmal ist diese Aussage gut. Es gibt wirklich zwei mögliche Ausführungspfade und wir möchten das hervorheben.

• In anderen Fällen gibt es nur einen Ausführungspfad und dann all die außergewöhnlichen Dinge, über die sich Programmierer Gedanken machen müssen. Wie Sie bereits erwähnt haben, sind Schutzklauseln hierfür hervorragend geeignet.

• Es gibt natürlich die drei oder mehr Fälle (n), in denen wir normalerweise dazu ermutigen, die if / else-Kette zu verlassen und eine case / switch-Anweisung oder einen Polymorphismus zu verwenden.

Das Leitprinzip, an das ich hier denke, ist, dass eine Methode oder Funktion nur einen Zweck haben sollte. Jeder Code mit einer if / else- oder switch-Anweisung dient nur zur Verzweigung. Es sollte also absurd kurz sein (4 Zeilen) und nur an die richtige Methode delegieren oder das offensichtliche Ergebnis liefern (wie in Ihrem Beispiel). Wenn Ihr Code so kurz ist, ist es schwer, diese mehrfachen Rückgaben zu übersehen :) Ähnlich wie bei "als schädlich eingestuft", kann jede Verzweigungsanweisung missbraucht werden, daher lohnt es sich normalerweise, einige kritische Überlegungen in else-Anweisungen aufzunehmen. Wie Sie bereits erwähnt haben, bietet ein else-Block einen Platz, an dem sich Code ansammelt. Sie und Ihr Team müssen sich entscheiden, wie Sie das finden.

Worüber sich das Tool wirklich beschwert, ist die Tatsache, dass Sie ein return / break / throw innerhalb des if-Blocks haben. Was das betrifft, haben Sie eine Schutzklausel geschrieben, sie aber vermasselt. Sie können wählen, ob Sie das Tool glücklich machen oder seinen Vorschlag "unterhalten" möchten, ohne ihn zu akzeptieren.

Ich denke, die Antwort hängt davon ab. Ihr Codebeispiel gibt (wie Sie indirekt betonen) einfach die Bewertung einer Bedingung zurück und wird, wie Sie offensichtlich wissen, am besten als einzelner Ausdruck dargestellt.

Um festzustellen, ob das Hinzufügen einer else-Bedingung den Code verdeutlicht oder verdeckt, müssen Sie bestimmen, was das IF / ELSE darstellt. Ist es (wie in Ihrem Beispiel) ein Ausdruck? In diesem Fall sollte dieser Ausdruck extrahiert und verwendet werden. Ist es eine Schutzbedingung? Wenn ja, dann ist das ELSE unnötig und irreführend, da es die beiden Zweige gleich erscheinen lässt. Ist es ein Beispiel für prozeduralen Polymorphismus? Extrahiere es. Setzt es Voraussetzungen? Dann kann es wichtig sein.

Bevor Sie sich entscheiden, das ELSE einzubeziehen oder zu eliminieren, entscheiden Sie, was es darstellt, und geben Sie an, ob es vorhanden sein soll oder nicht.

Die Antwort ist etwas subjektiv. Ein elseBlock kann die Lesbarkeit unterstützen, indem festgestellt wird, dass ein Codeblock ausschließlich für einen anderen Codeblock ausgeführt wird, ohne dass beide Blöcke gelesen werden müssen. Dies kann hilfreich sein, wenn beispielsweise beide Blöcke relativ lang sind. Es gibt jedoch Fälle, in denen ifs ohne elses besser lesbar sein kann. Vergleichen Sie den folgenden Code mit einer Anzahl von if/elseBlöcken:

if(a == b) {
    if(c <= d) {
        if(e != f) {
            a.doSomeStuff();
            c.makeSomeThingsWith(b);
            f.undo(d, e);
            return 9;
        } else {
            e++;
            return 3;
        }
    } else {
        return 1;
    }
} else {
    return 0;
}

mit:

if(a != b) {
    return 0;
}

if(c > d) {
    return 1;
}

if(e != f) {
    e++;
    return 3;
}

a.doSomeStuff();
c.makeSomeThingsWith(b);
f.undo(d, e);
return 9;

Der zweite Codeblock wandelt die verschachtelten ifAnweisungen in Schutzklauseln um. Dies reduziert die Einrückung und macht einige der Rückgabebedingungen klarer ("wenn a != b, dann kehren wir zurück 0!")

In den Kommentaren wurde darauf hingewiesen, dass es in meinem Beispiel einige Löcher geben kann, deshalb werde ich es ein wenig weiter ausarbeiten.

Wir hätten den ersten Codeblock hier so ausschreiben können, um ihn lesbarer zu machen:

if(a != b) {
    return 0;
} else if(c > d) {
    return 1;
} else if(e != f) {
    e++;
    return 3;
} else {
    a.doSomeStuff();
    c.makeSomeThingsWith(b);
    f.undo(d, e);
    return 9;
}

Dieser Code entspricht den beiden obigen Blöcken, birgt jedoch einige Herausforderungen. Die elseKlausel hier verbindet diese if-Anweisungen miteinander. In der obigen Version der Schutzklausel sind die Schutzklauseln voneinander unabhängig. Wenn Sie eine neue hinzufügen, schreiben Sie einfach eine neue ifzwischen der letzten ifund der restlichen Logik. Dies ist auch hier mit nur geringfügig mehr kognitiver Belastung möglich. Der Unterschied besteht jedoch darin, dass vor dieser neuen Schutzklausel eine zusätzliche Logik erforderlich ist. Wenn die Aussagen unabhängig waren, konnten wir Folgendes tun:

...
if(e != f) {
    e++;
    return 3;
}

g.doSomeLogicWith(a);

if(!g.isGood()) {
    return 4;
}

a.doSomeStuff();
...

Mit der verbundenen if/elseKette ist das nicht so einfach. In der Tat wäre der einfachste Weg, die Kette zu durchbrechen, um mehr wie die Guard-Klausel-Version auszusehen.

Aber das macht wirklich Sinn. Die if/elseschwache Verwendung impliziert eine Beziehung zwischen Teilen. Wir könnten vermuten, dass dies in der verketteten Version a != birgendwie verwandt ist . Diese Annahme wäre irreführend, wie wir aus der Fassung der Schutzklausel ersehen können, dass sie tatsächlich nicht miteinander zusammenhängen. Das bedeutet, dass wir mit unabhängigen Klauseln mehr tun können, als sie neu anzuordnen (vielleicht, um die Abfrageleistung zu optimieren oder den logischen Fluss zu verbessern). Wir können sie auch kognitiv ignorieren, wenn wir an ihnen vorbei sind. In einer Kette bin ich mir weniger sicher, dass die Äquivalenzbeziehung zwischen und später nicht wieder auftaucht.c > dif/elseif/elseab

Die von implizierte Beziehung elsewird auch im tief verschachtelten Beispielcode deutlich, jedoch auf andere Weise. Die elseAnweisungen sind von der Bedingung, an die sie gebunden sind, getrennt und in umgekehrter Reihenfolge mit den Bedingungen verknüpft (die erste elseist an die letzte gebunden if, die letzte elseist an die erste gebunden if). Dies erzeugt eine kognitive Dissonanz, bei der wir den Code zurückverfolgen und versuchen müssen, die Einrückung in unserem Gehirn in eine Linie zu bringen. Es ist ein bisschen so, als würde man versuchen, ein paar Klammern zusammenzusetzen. Es wird noch schlimmer, wenn die Einrückung falsch ist. Optionale Zahnspangen helfen auch nicht.

Ich habe vergessen, einen Fall zu erwähnen, in dem ich mit dem Weglassen eines else-Blocks einverstanden bin. Wenn ich einen if-Block verwende, um eine Einschränkung anzuwenden, die für den folgenden Code logisch erfüllt sein muss, z ).

Dies ist eine nette Konzession, die aber keine Antwort ungültig macht. Ich könnte das in Ihrem Codebeispiel sagen:

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

Eine gültige Interpretation für das Schreiben von Code wie diesem könnte sein, dass "wir nur mit einem Regenschirm gehen müssen, wenn der Himmel nicht blau ist." Hier gibt es eine Einschränkung, dass der Himmel blau sein muss, damit der folgende Code ausgeführt werden kann. Ich habe die Definition Ihrer Konzession getroffen.

Was ist, wenn ich sage, wenn die Himmelsfarbe schwarz ist, ist es eine klare Nacht, daher ist kein Regenschirm erforderlich. (Es gibt einfachere Möglichkeiten, dies zu schreiben, aber es gibt einfachere Möglichkeiten, das gesamte Beispiel zu schreiben.)

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color == Color.Blue)
    {
        return true;
    }

    if(sky.Color == Color.Black)
    {
        return true;
    }

    return false;
}

Wie im obigen größeren Beispiel kann ich die neue Klausel einfach hinzufügen, ohne viel darüber nachzudenken. In einem if/elsekann ich nicht so sicher sein, dass ich etwas nicht vermassle, besonders wenn die Logik komplizierter ist.

Ich werde auch darauf hinweisen, dass Ihr einfaches Beispiel auch nicht so einfach ist. Ein Test für einen nicht-binären Wert ist nicht dasselbe wie das Inverse dieses Tests mit invertierten Ergebnissen.

Sie haben Ihr Beispiel zu stark vereinfacht. Entweder Alternative kann besser lesbar sein, abhängig von der größeren Situation: Insbesondere ist es hängt davon ab , ob einer der beiden Zweige des Zustandes abnormal . Lassen Sie sich zeigen: In einem Fall, in dem beide Branchen gleich wahrscheinlich sind, ...

void DoOneOfTwoThings(bool which)
{
    if (which)
        DoThingOne();
    else
        DoThingTwo();
}

... ist lesbarer als ...

void DoOneOfTwoThings(bool which)
{
    if (which) {
        DoThingOne();
        return;
    }
    DoThingTwo();
}

... weil der erste parallel aufgebaut ist und der zweite nicht. Aber wenn der Großteil der Funktion einer Aufgabe gewidmet ist und Sie nur einige Voraussetzungen prüfen müssen, ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input)) {
        ProcessInputOfTypeTwo(input);
        return;
    }
    ProcessInputDefinitelyOfTypeOne(input);

}

... ist lesbarer als ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input))
        ProcessInputOfTypeTwo(input);
    else
        ProcessInputDefinitelyOfTypeOne(input);
}

... weil absichtlich nicht parallele Struktur der Einrichtung einen Hinweis auf einen zukünftigen Leser sieht vor, dass Eingang bekommen , die „wirklich gibt zwei“ hier ist abnormal . (Wäre im wirklichen Leben ProcessInputDefinitelyOfTypeOnekeine separate Funktion, so wäre der Unterschied noch größer.)

Ja, die Komplexität nimmt zu, weil die else-Klausel überflüssig ist . Es ist daher besser, wegzulassen, um den Code schlank zu halten. Dies entspricht dem Weglassen redundanter thisQualifikationsmerkmale (Java, C ++, C #) und dem Weglassen von Klammern in returnAnweisungen und so weiter.

Ein guter Programmierer denkt "Was kann ich hinzufügen?" während ein großartiger Programmierer denkt "Was kann ich entfernen?".

Ich bemerkte, dass ich meine Meinung über diesen Fall änderte.

Wenn ich diese Frage vor ungefähr einem Jahr gestellt hätte, hätte ich etwas beantwortet wie:

» Zu einer Methode sollte es nur eins entryund eins geben exit« In diesem Sinne hätte ich vorgeschlagen, den Code zu überarbeiten, um:

bool CanLeaveWithoutUmbrella()
{
    bool result

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    else
    {
        result = false;
    }

    return result;
}

Aus Sicht der Kommunikation ist klar , was zu tun ist. Ifetwas ist der Fall, manipulieren Sie das Ergebnis auf eine Weise , else manipulieren Sie es auf eine andere Weise .

Das ist aber unnötig else . Der elsedrückt den Standardfall aus . In einem zweiten Schritt könnte ich es also umgestalten

bool CanLeaveWithoutUmbrella()
{
    bool result=false

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    return result;
}

Dann stellt sich die Frage: Warum überhaupt eine temporäre Variable verwenden? Der nächste Schritt der Refaktorisierung führt zu:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue)
    {
        return true;
    }
    return false;
}

Das ist also klar, was es tut. Ich würde sogar noch einen Schritt weiter gehen:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) return true;
    return false;
}

Oder für schwache Nerven :

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) { return true; }
    return false;
}

Man könnte es so lesen: »CanLeaveWithoutUmbrella gibt einen bool . Wenn nichts Besonderes passiert, wird false zurückgegeben «Dies ist der erste Lesevorgang von oben nach unten.

Und dann "parsen" Sie die Bedingung. »Wenn die Bedingung erfüllt ist, gibt sie true zurück. « Sie könnten die Bedingung vollständig überspringen und hätten verstanden, was diese Funktion im Standardfall tut. Ihr Auge und Ihr Verstand müssen nur einige Buchstaben auf der linken Seite durchblättern, ohne den Teil auf der rechten Seite zu lesen.

Ich habe den Eindruck, der sonst direkter die Absicht angibt, indem er die Tatsache angibt, dass der Code in beiden Blöcken direkt verwandt ist.

Ja, das ist richtig. Diese Informationen sind jedoch redundant . Sie haben einen Standardfall und eine "Ausnahme", die von derif -Anweisung .

Beim Umgang mit komplexeren Strukturen würde ich eine andere Strategie wählen und den ifoder den hässlichen Bruder switchüberhaupt weglassen .

Um es kurz zu machen, in diesem Fall elseist es eine gute Sache, das Schlüsselwort loszuwerden . Es ist hier völlig überflüssig, und je nachdem, wie Sie Ihren Code formatieren, werden Ihrem Code ein bis drei völlig nutzlose Zeilen hinzugefügt . Überlegen Sie, was im ifBlock enthalten ist:

return true;

Daher wird, wenn der ifBlock eingegeben werden soll, der gesamte Rest der Funktion per Definition nicht ausgeführt. Wenn es jedoch nicht eingegeben wird, ifwird der gesamte Rest der Funktion ausgeführt , da keine weiteren Anweisungen vorhanden sind . Es wäre völlig anders, wenn eine returnAussage nicht im ifBlock wäre, in welchem ​​Fall das Vorhandensein oder Fehlen einerelse Blocks Auswirkungen, aber hier hätte es keine Auswirkungen.

In Ihrer Frage haben Sie nach dem Umkehren Ihrer ifBedingung und dem Weglassen elseFolgendes angegeben:

Jemand kann jetzt einen neuen if-Block hinzufügen, der auf einer Bedingung nach dem ersten Beispiel basiert, ohne die erste Bedingung sofort richtig zu erkennen, und die eigene Bedingung einschränken.

Dann müssen sie den Code etwas genauer lesen. Wir verwenden Hochsprachen und eine klare Organisation des Codes, um diese Probleme zu mindern. Durch Hinzufügen eines vollständig redundanten elseBlocks wird der Code jedoch mit "Rauschen" und "Unordnung" versehen, und wir sollten auch unsere ifBedingungen nicht invertieren oder erneut invertieren müssen. Wenn sie den Unterschied nicht erkennen können, wissen sie nicht, was sie tun. Wenn sie den Unterschied erkennen, können sie die ursprünglichen ifBedingungen immer selbst umkehren .

Ich habe vergessen, einen Fall zu erwähnen, in dem ich mit dem Weglassen eines else-Blocks einverstanden bin. Wenn ich einen if-Block verwende, um eine Einschränkung anzuwenden, die für den folgenden Code logisch erfüllt sein muss, z ).

Das ist im Wesentlichen, was hier passiert. Sie kehren aus dem ifBlock zurück, sodass die ifBedingung, die als ausgewertet false wird, eine Einschränkung ist, die für den gesamten folgenden Code logisch erfüllt sein muss.

Wird die Komplexität erhöht, wenn der else-Block nicht eingeschlossen wird?

Nein, es wird eine Verringerung der Komplexität in Ihrem Code bedeuten, und es kann sogar eine Verringerung des kompilierten Codes bedeuten, je nachdem, ob bestimmte supertriviale Optimierungen vorgenommen werden oder nicht.

In dem konkreten Beispiel, das Sie angegeben haben, ist dies der Fall.

• Ein Prüfer wird gezwungen, den Code erneut zu lesen, um sicherzustellen, dass es sich nicht um einen Fehler handelt.
• Dies erhöht die (unnötige) zyklomatische Komplexität, da dem Flussdiagramm ein Knoten hinzugefügt wird.

Einfacher geht es wohl nicht:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}

Ich versuche immer, meinen Code so zu schreiben, dass die Absicht so klar wie möglich ist. Ihrem Beispiel fehlt ein gewisser Kontext, deshalb werde ich ihn ein wenig modifizieren:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        if(this.color == Color.Blue)
        {
            return true;
        }
        else
        {
            return false;
        }
    }
}

Unsere Absicht scheint zu sein dass wir ohne Regenschirm gehen können, wenn der Himmel blau ist. Diese einfache Absicht geht jedoch in einem Meer von Codes verloren. Es gibt verschiedene Möglichkeiten, die das Verständnis erleichtern.

Erstens gibt es Ihre Frage zu der elseBranche. Mir macht es sowieso nichts aus, aber ich neige dazu, das wegzulassen else. Ich verstehe das Argument, explizit zu sein, aber ich bin der Meinung, dass ifAnweisungen 'optionale' Zweige wie den return trueeinen einschließen sollten . Ich würde die return falseVerzweigung nicht als optional bezeichnen, da sie standardmäßig verwendet wird. In beiden Fällen sehe ich dies als sehr geringes Problem und weit weniger wichtig als die folgenden:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        if(this.color == Color.Blue)
        {
            return true;
        }
        return false;
    }
}

Ein viel wichtigeres Problem ist, dass Ihre Filialen zu viel tun! Zweige sollten, wie alles, "so einfach wie möglich sein, aber nicht mehr". In diesem Fall haben Sie eine ifAnweisung verwendet, die zwischen Codeblöcken (Auflistungen von Anweisungen) verzweigt, wenn Sie nur zwischen Ausdrücken (Werten) verzweigen müssen . Dies ist klar, da a) Ihre Codeblöcke nur einzelne Anweisungen enthalten und b) dieselbe Anweisung sind ( return). Wir können den ternären Operator verwenden ?:, um den Zweig auf den Teil einzugrenzen, der sich unterscheidet:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        return (this.color == Color.Blue)
            ? true
            : false;
    }
}

Jetzt erreichen wir die offensichtliche Redundanz, mit der unser Ergebnis identisch ist this.color == Color.Blue. Ich weiß, dass Ihre Frage uns bittet, dies zu ignorieren, aber ich denke, es ist wirklich wichtig, darauf hinzuweisen, dass dies eine prozedurale Denkweise erster Ordnung ist: Sie zerlegen die Eingabewerte und konstruieren einige Ausgabewerte. Das ist in Ordnung, aber wenn möglich ist es viel mächtiger, in Beziehungen oder Transformationen zwischen Input und Output zu denken . In diesem Fall ist die Beziehung offensichtlich die gleiche, aber ich sehe oft einen verwickelten prozeduralen Code wie diesen, der eine einfache Beziehung verdeckt. Lassen Sie uns fortfahren und diese Vereinfachung vornehmen:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        return (this.color == Color.Blue);
    }
}

Als nächstes möchte ich darauf hinweisen, dass Ihre API ein doppeltes Negativ enthält: Es ist möglich, dass dies CanLeaveWithoutUmbrellader Fall ist false. Das vereinfacht natürlich das NeedUmbrellaSein true, also lasst uns das machen:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool NeedUmbrella()
    {
        return (this.color != Color.Blue);
    }
}

Nun können wir über Ihren Codierungsstil nachdenken. Es sieht so aus, als ob Sie eine objektorientierte Sprache verwenden. Wenn Sie jedoch ifAnweisungen zum Verzweigen zwischen Codeblöcken verwenden, haben Sie am Ende prozeduralen Code geschrieben.

Im objektorientierten Code sind alle Codeblöcke Methoden und alle Verzweigungen erfolgen über dynamisches Versenden , z. mit Klassen oder Prototypen. Es ist fraglich, ob dies die Dinge einfacher macht, aber es sollte Ihren Code konsistenter mit dem Rest der Sprache machen:

class Sky {
    bool NeedUmbrella()
    {
        return true;
    }
}

class BlueSky extends Sky {
    bool NeedUmbrella()
    {
        return false;
    }
}

Beachten Sie, dass die Verwendung von ternären Operatoren zum Verzweigen zwischen Ausdrücken in der funktionalen Programmierung üblich ist.