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

18

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

Selali Adobor
quelle
In der Realität kann eine "if" -Anweisung mit einem "return" in> 50% aller Fälle als "guard block" angesehen werden. Ich denke also, Ihr Missverständnis ist hier, dass ein "Schutzblock" der Ausnahmefall und Ihr Beispiel der reguläre Fall ist.
Doc Brown
2
@AssortedTrailmix: Ich bin damit einverstanden, dass ein Fall wie der obige beim Schreiben der elseKlausel besser lesbar ist (nach meinem Geschmack ist er sogar noch besser lesbar, wenn die nicht benötigten Klammern weggelassen werden. Aber ich denke, das Beispiel ist nicht gut gewählt, weil in der In dem obigen Fall würde ich schreiben return sky.Color == Color.Blue(ohne if / else). Ein Beispiel mit einem anderen Rückgabetyp als bool würde dies wahrscheinlich klarer machen.
Doc Brown
1
Wie bereits in den obigen Kommentaren erwähnt, ist das vorgeschlagene Beispiel zu vereinfacht und lädt zu spekulativen Antworten ein. Was den Teil der Frage anbelangt, der mit "Jemand kann jetzt einen neuen if-Block hinzufügen ..." beginnt , wurde er bereits mehrmals gefragt und beantwortet, siehe z . und mehrere damit verbundene Fragen.
gnat
1
Ich verstehe nicht, was Blöcke sonst noch mit dem DRY-Prinzip zu tun haben. DRY hat mehr mit Abstraktion und Verweisen auf häufig verwendeten Code in Form von Funktionen, Methoden und Objekten zu tun, als Sie verlangen. Ihre Frage bezieht sich auf die Lesbarkeit des Codes und möglicherweise auf Konventionen oder Redewendungen.
Shashank Gupta
2
Abstimmung zur Wiedereröffnung. Die guten Antworten auf diese Frage sind nicht besonders meinungsbasiert. Wenn das angegebene Beispiel in einer Frage unzureichend ist, ist es sinnvoll, es durch Bearbeiten zu verbessern, und nicht für den Abschluss zu stimmen, wenn dies nicht der Fall ist.
Michael Shaw

Antworten:

27

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

Winston Ewert
quelle
Das fasst meine Gedanken darüber zusammen, ich bin froh, dass ich nicht der einzige bin, der so denkt. Ich arbeite mit Programmierern, Tools und IDEs, die das Entfernen des elseBlocks bevorzugen (es kann lästig sein, wenn ich dazu gezwungen bin, nur um die Konventionen für eine Codebasis einzuhalten). Ich bin auch daran interessiert, den Kontrapunkt hier zu sehen.
Selali Adobor
1
Ich schwanke darüber. Manchmal bringt Ihnen das explizite Anders nicht viel, da es unwahrscheinlich ist, dass die vom OP erwähnten subtilen Logikfehler ausgelöst werden - es ist nur Rauschen. Aber meistens stimme ich zu, und gelegentlich mache ich so etwas wie } // elsedas, was sonst als Kompromiss zwischen den beiden ginge.
Telastyn
3
@Telastyn, aber was bringt es dir, das andere zu überspringen? Ich stimme zu, dass der Nutzen in vielen Situationen sehr gering ist, aber selbst für den geringen Nutzen denke ich, dass es sich lohnt, dies zu tun. Sicherlich erscheint es mir bizarr, etwas in einen Kommentar zu schreiben, wenn es Code sein könnte.
Winston Ewert
2
Ich denke, Sie haben das gleiche Missverständnis wie das OP - "Wenn" mit "Zurück" kann meist als Schutzblock angesehen werden, so dass Sie hier den Ausnahmefall diskutieren, während der häufigere Fall von Schutzblöcken meiner Meinung nach ohne die besser lesbar ist "sonst".
Doc Brown
... also, was Sie geschrieben haben, ist nicht falsch, aber meiner Meinung nach sind die vielen positiven Stimmen, die Sie erhalten haben, nicht wert.
Doc Brown
23

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.

Jonathan Eunice
quelle
2
Dies ist eine ausführliche und gültige Antwort, aber ich werde dies der Frage hinzufügen (es ist mir zunächst entgangen). Es gibt eine "Ausnahme", bei der ich einen elseBlock immer für überflüssig halte, wenn ich einen ifBlock verwende, um eine Einschränkung anzuwenden, die für den gesamten folgenden Code logisch erfüllt sein muss , z. B. eine Nullprüfung (oder andere Schutzmaßnahmen).
Selali Adobor
@AssortedTrailmix Ich glaube, wenn Sie Fälle ausschließen, in denen Sie routinemäßig vorzeitig aus der thenKlausel aussteigen können (z. B. aufeinanderfolgende Guard-Anweisungen), gibt es keinen besonderen Wert für die Vermeidung der jeweiligen elseKlausel. In der Tat würde dies die Klarheit verringern und möglicherweise gefährlich sein (indem nicht einfach die alternative Struktur angegeben wird, die vorhanden ist / sein sollte).
Jonathan Eunice
Ich halte es für vernünftig, Schutzbedingungen zu verwenden, um auszuwerfen, wenn Sie den glücklichen Pfad verlassen haben. Für den Fall der Verarbeitung von XML / JSON stelle ich jedoch auch fest, dass Sie bessere Fehlermeldungen und saubereren Code erhalten, wenn Sie die Eingabe zuerst anhand eines Schemas überprüfen.
Winston Ewert
Dies ist eine perfekte Erklärung für die Fälle, in denen andere vermieden werden.
Chris Schiffhauer
2
Ich hätte die API so verpackt, dass stattdessen keine albernen NULL-Werte erzeugt werden.
Winston Ewert
4

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:

Bildbeschreibung hier eingeben

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.

Aviv Cohn
quelle
2

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.

FranMowinckel
quelle
Ich habe das Gefühl, Sie machen den gleichen Fehler, den andere gemacht haben, und verändern das Beispiel ein wenig zu sehr, um ein neues Problem zu untersuchen. Ich muss mir jedoch eine Sekunde Zeit nehmen und den Prozess untersuchen, um zu Ihrem Beispiel zu gelangen gültig für mich. Und eine Randnotiz, dies hat nichts mit dem Open / Closed-Prinzip zu tun . Der Anwendungsbereich ist viel zu eng, um diese Redewendung anzuwenden. Es ist jedoch interessant zu bemerken, dass seine Anwendung auf einer höheren Ebene darin besteht, diesen gesamten Aspekt des Problems zu beseitigen (indem die Notwendigkeit einer Änderung der Funktion beseitigt wird ).
Selali Adobor
1
Aber wenn wir diese stark vereinfachten Beispiel-Hinzufügungsklauseln nicht modifizieren, ihre Schreibweise nicht ändern oder Änderungen daran vornehmen können, sollten Sie berücksichtigen, dass sich diese Frage nur auf genau diese Codezeilen bezieht und keine allgemeine Frage mehr ist. In diesem Fall haben Sie vollkommen recht, da die else-Klausel keine Komplexität hinzufügt (und auch nicht entfernt), da es anfangs keine Komplexität gab. Sie sind jedoch nicht fair, weil Sie die Idee vorschlagen, dass neue Klauseln hinzugefügt werden könnten.
FranMowinckel
Und ich sage nicht, dass dies mit dem bekannten offenen / geschlossenen Prinzip zusammenhängt. Ich denke, dass die Idee, Leute zu veranlassen, Ihren Code zu ändern, wie Sie vorschlagen, fehleranfällig ist. Ich nehme nur diese Idee von diesem Prinzip.
FranMowinckel
Behalten Sie
1

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.

Sparky
quelle
Wäre in einer Objektdatei keiner der beiden Fälle als identisches JMP gerendert worden?
Winston Ewert
@ WinstonEwert - Man würde erwarten, dass sie gleich sind. Was jedoch gerendert wird und was gerendert werden soll, sind zwei verschiedene Dinge, unabhängig davon, wie stark sie sich überlappen.
Sparky
(Ich denke, SE hat meinen Kommentar aufgenommen ...) Dies ist eine sehr interessante Antwort. Ich würde zwar sagen, dass der Fall, den es aufdeckt, eine Nische darstellt, aber es zeigt, dass einige Tools einen Unterschied zwischen den beiden finden würden (Selbst die häufigste
Codemetrik,
1

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.

l0b0
quelle
Ich mag Ihre Nachschlagetabelle, die den einfachen Schalterfall umschreibt. Und ich weiß, dass dies eine beliebte Python-Sprache ist. In der Praxis stelle ich jedoch häufig fest, dass Mehrwegebedingungen ( caseoder switchAussagen) weniger homogen sind. Einige Optionen sind nur reine Wertrückgaben, aber ein oder zwei Fälle erfordern eine zusätzliche Verarbeitung. Und wenn Sie feststellen, dass die Suchstrategie nicht passt, müssen Sie eine völlig andere if elif... elseSyntax verwenden.
Jonathan Eunice
Das ist der Grund, warum ich denke, dass die Suche normalerweise ein Einzeiler oder eine Funktion sein sollte, damit die Logik um das Nachschlageergebnis von der Logik der ersten Suche getrennt ist.
06.09.14
Diese Frage bringt eine übergeordnete Sichtweise auf die Frage, die unbedingt erwähnt und im Auge behalten werden sollte, aber ich bin der Meinung, dass der Fall hinreichend eingeschränkt wurde, sodass Sie sich frei fühlen können, Ihre eigene Haltung zu erweitern
Selali Adobor,
1

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.

Michael Shaw
quelle
+1, das OP ist IMHO, das einen pathologischen Fall bespricht, der besser geschrieben werden sollte, wie Sie oben gezeigt haben. Der viel häufigere Fall für "wenn" mit "Rückkehr" ist nach meiner Erfahrung der "Schutz" -Fall.
Doc Brown
Ich weiß nicht, warum das so ist, aber die Leute ignorieren immer wieder den ersten Absatz der Frage. Tatsächlich verwenden Sie alle genau die Codezeile, von der ich ausdrücklich sage, dass sie nicht verwendet werden soll. I ask that you ignore the many other ways the function can be written, and changes that can be made to it. (I know most people are itching to write return sky.Color == Color.Blue.)
Selali Adobor
Wenn Sie Ihre letzte Frage noch einmal lesen, scheinen Sie die Absicht der Frage falsch zu verstehen.
Selali Adobor
Sie haben ein Beispiel verwendet, das praktisch vereinfacht werden sollte. Ich habe Ihren ersten Absatz gelesen und darauf geachtet, weshalb mein erster Codeblock überhaupt vorhanden ist, anstatt direkt zum besten Beispiel zu springen. Wenn Sie der Meinung sind, dass ich (oder jemand anderes) die Absicht Ihrer Frage missverstanden hat, sollten Sie sie möglicherweise bearbeiten, um Ihre Absicht deutlich zu machen.
Michael Shaw
Ich würde es bearbeiten, aber ich würde nicht wissen, wie ich es deutlicher machen könnte, da ich genau die Codezeile verwende, die Sie tun und "Tu das nicht" sage. Es gibt unendlich viele Möglichkeiten, wie Sie den Code umschreiben und die Frage entfernen können. Ich kann nicht alle behandeln, aber so viele Leute haben meine Aussage verstanden und respektiert ...
Selali Adobor
1

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.

Steve Jackson
quelle
Ich hätte nie gedacht , über die Tatsache , könnte das Werkzeug der in Erwägung ziehen , wenn eine Schutzklausel so schnell zu sein , wie es für einen in das Muster paßt, die wahrscheinlich der Fall ist, und es erklärt die Gründe für eine „Gruppe der Befürworter“ für seine Abwesenheit
Selali Adobor
@AssortedTrailmix Richtig, Sie denken über elsesemantische Aspekte nach. Code-Analyse-Tools suchen normalerweise nach fremden Anweisungen, da sie häufig auf ein Missverständnis des Kontrollflusses hinweisen. Das elseafter an if, das zurückgibt, ist verschwendete Bits. if(1 == 1)löst oft die gleiche Warnung aus.
Steve Jackson
1

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.

jmoreno
quelle
0

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.

cbojar
quelle
1
Meine Bearbeitung der Frage befasst sich mit diesem Fall. Wenn eine Einschränkung, die für alle folgenden Codes logisch erfüllt sein muss, wie z. B. eine Nullprüfung (oder andere Schutzmaßnahmen), ich bin damit einverstanden, dass ein elseBlock weggelassen wird, ist dies für die Lesbarkeit von wesentlicher Bedeutung.
Selali Adobor
1
Ihre erste Version ist schwer zu verstehen, aber ich denke nicht, dass die Verwendung von if / else dies erforderlich macht. Sehen Sie, wie ich es schreiben würde: gist.github.com/winstonewert/c9e0955bc22ce44930fb .
Winston Ewert
@ WinstonEwert Siehe Änderungen für die Antwort auf Ihre Kommentare.
Cbojar
Ihre Bearbeitung enthält einige gültige Punkte. Und ich denke wirklich, dass Schutzklauseln einen Platz haben. Aber im Allgemeinen bin ich für else-Klauseln.
Winston Ewert
0

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

zwol
quelle
Es ist vielleicht besser, ein konkreteres Beispiel für den zweiten Fall zu wählen. Meine unmittelbare Reaktion darauf lautet: "Es kann nicht so ungewöhnlich sein, wenn Sie es trotzdem verarbeitet haben."
Winston Ewert
@ WinstonEwert abnormal ist vielleicht der falsche Begriff. Aber ich stimme Zack zu, dass, wenn Sie eine Vorgabe (case1) und eine Ausnahme haben, diese als » if-> Mach das Außergewöhnliche und gehe « als Strategie für eine frühzeitige Rückkehr geschrieben werden sollte . Denken Sie an Code, in dem die Standardeinstellung mehr Überprüfungen enthält. Es wäre schneller, den Ausnahmefall zuerst zu behandeln.
Thomas Junk
Dies scheint in den Fall zu fallen, mit dem ich gesprochen habe when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards). Logischerweise hängt jede Arbeit ProcessInputOfTypeOnedavon ab, dass !InputIsReallyTypeTwo(input)wir sie außerhalb unserer normalen Verarbeitung abfangen müssen. Normalerweise ProcessInputOfTypeTwo(input);wäre das wirklich throw ICan'tProccessThatExceptionwas die Ähnlichkeit deutlicher machen würde.
Selali Adobor
@ WinstonEwert Ich hätte das vielleicht besser ausdrücken können, ja. Ich dachte an eine Situation, die beim manuellen Codieren von Tokenizern häufig auftritt: In CSS wird beispielsweise durch die Zeichenfolge " u+" normalerweise ein "Unicode-Range" -Token eingeführt. Wenn das nächste Zeichen jedoch keine Hex-Ziffer ist, dann muss man stattdessen das 'u' als Bezeichner sichern und verarbeiten. Die Haupt-Scannerschleife sendet den Befehl "Scannen eines Unicode-Bereichs-Tokens" etwas ungenau und beginnt mit "... wenn wir uns in diesem ungewöhnlichen Sonderfall befinden, sichern Sie sich, und rufen Sie stattdessen die Unterroutine" scan-an-identifier "auf.
zwol
@AssortedTrailmix Auch wenn das Beispiel, an das ich gedacht habe, nicht aus einer älteren Codebasis stammt, in der Ausnahmen möglicherweise nicht verwendet werden, handelt es sich nicht um eine Fehlerbedingung. Der aufgerufene Code ProcessInputOfTypeOneverwendet manchmal eine ungenaue, aber schnelle Überprüfung fängt stattdessen Typ zwei.
zwol
0

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?".

vidstige
quelle
1
Wenn ich sehe else, dass der Block weggelassen wurde, und wenn er in einem Einzeiler erklärt wird (was nicht oft der Fall ist), dann geschieht dies oft mit dieser Art von Argumentation finde es stark genug zu argumentieren. A good programmer things "what can I add?" while a great programmer things "what can I remove?".Es scheint ein allgemeines Sprichwort zu sein, dass viele Menschen ohne sorgfältige Überlegung überfordern, und ich persönlich finde, dass dies ein solcher Fall ist.
Selali Adobor
Ja, als ich Ihre Frage las, hatte ich sofort das Gefühl, Sie hätten sich bereits entschieden, wollten aber eine Bestätigung. Diese Antwort ist offensichtlich nicht das, was Sie wollten, aber manchmal ist es wichtig, Meinungen zu berücksichtigen, mit denen Sie nicht einverstanden sind. Vielleicht gibt es auch etwas?
Vidstige
-1

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 .

Thomas Junk
quelle
+1 Redundanz zeigt definitiv eine Zunahme nicht nur der Komplexität, sondern auch der Verwirrung. Ich weiß, dass ich aufgrund der Redundanz immer genau so geschriebenen Code überprüfen musste.
Dietbuddha
1
Könnten Sie die Fälle nach dem Beginn des Falls entfernen So this is clear in what it does...und erweitern? (Die vorgenannten Fälle sind ebenfalls von fragwürdiger Relevanz.) Ich sage das, weil es das ist, was die Frage ist, die wir untersuchen. Sie haben Ihre Haltung zum Ausdruck gebracht, den else-Block weggelassen, und es ist tatsächlich die Haltung, die näher erläutert werden muss (und die, die mich dazu gebracht hat, diese Frage zu stellen). Aus der Frage:I ask that you ignore the many other ways the function can be written, and changes that can be made to it. (I know most people are itching to write return sky.Color == Color.Blue.)
Selali Adobor
@ AssortedTrailmix Sicher! Worauf soll ich genau eingehen? Da Ihre ursprüngliche Frage lautete »Erhöhen andere Blöcke die Codekomplexität?« Und meine Antwort lautet: Ja. In diesem Fall könnte es weggelassen werden.
Thomas Junk
Und nein! Ich verstehe die Ablehnung nicht.
Thomas Junk
-1

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.

Panzerkrise
quelle
2
"Dann müssen sie den Code etwas genauer lesen." Es gibt einen Codierungsstil, mit dem Programmierer weniger auf winzige Details und mehr auf das achten können, was sie tatsächlich tun. Wenn Sie bei Ihrem Stil unnötigerweise auf diese Details achten, ist dies ein schlechter Stil. "... völlig nutzlose Zeilen ..." Sie sind nicht nutzlos - sie lassen Sie auf einen Blick erkennen, was die Struktur ist, ohne Zeit damit zu verschwenden, darüber nachzudenken, was passieren würde, wenn dieser oder jener Teil ausgeführt würde.
Michael Shaw
@MichaelShaw Ich denke, Aviv Cohns Antwort bezieht sich auf das, wovon ich spreche.
Panzercrisis
-2

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);
}
Tulains Córdova
quelle
6
Ich spreche speziell die Tatsache an, dass dieses sehr vereinfachte Beispiel umgeschrieben werden kann, um die ifAussage zu entfernen . Tatsächlich rate ich ausdrücklich von einer weiteren Vereinfachung ab, indem ich den genauen Code verwende, den Sie verwendet haben. Zusätzlich wird die zyklomatische Komplexität durch den elseBlock nicht erhöht .
Selali Adobor
-4

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.

Warbo
quelle
Der erste Absatz der Antwort scheint auf dem richtigen Weg zu sein, um die Frage zu beantworten, aber er weicht sofort von dem genauen Thema ab, das ich ausdrücklich für dieses sehr einfache Beispiel ignoriere . Von der Frage: I ask that you ignore the many other ways the function can be written, and changes that can be made to it. (I know most people are itching to write return sky.Color == Color.Blue.). In der Tat verwenden Sie die genaue Codezeile, die ich erwähne. Während dieser Teil der Frage vom Thema abweicht, würde ich außerdem sagen, dass Sie in Ihrer Schlussfolgerung zu weit gehen. Sie haben den Code grundlegend geändert.
Selali Adobor
@AssortedTrailmix Das ist nur eine Frage des Stils. Es ist sinnvoll, sich um den Stil zu kümmern, und es ist sinnvoll, ihn zu ignorieren (sich nur auf die Ergebnisse zu konzentrieren). Ich glaube nicht , es sinnvoll, Pflege macht etwa return false;vs else { return false; }während nicht Sorge um Zweig Polarität, dynamische Dispatch Ternäre, etc. Wenn Sie prozeduralen Code in einer OO - Sprache schreiben, radikale Veränderung notwendig ist ;)
Warbo
Sie können im Allgemeinen so etwas sagen, aber es ist nicht anwendbar, wenn Sie eine Frage mit genau definierten Parametern beantworten (insbesondere, wenn Sie die gesamte Frage "abstrahieren" können, ohne diese Parameter zu beachten). Ich würde auch sagen, dass der letzte Satz einfach falsch ist. Bei OOP geht es um "prozedurale Abstraktion", nicht um "prozedurale Auslöschung". Ich würde sagen, die Tatsache, dass Sie von einem Einzeiler zu einer unendlichen Anzahl von Klassen gewechselt sind (eine für jede Farbe), zeigt, warum. Außerdem frage ich mich immer noch, was dynamischer Versand mit einer if/elseAnweisung zu tun haben soll und welche Verzweigungspolarität vorliegt.
Selali Adobor
Ich habe nie gesagt, dass die OO-Lösung besser ist (in der Tat bevorzuge ich die funktionale Programmierung), ich habe nur gesagt, dass sie mit der Sprache konsistenter ist. Mit "Polarität" meinte ich, welches Ding "wahr" und welches "falsch" ist (ich habe es mit "NeedUmbrella" getauscht). Bei der OO-Konstruktion wird der gesamte Steuerungsfluss durch den dynamischen Versand bestimmt. Zum Beispiel ist Smalltalk nichts anderes erlauben en.wikipedia.org/wiki/Smalltalk#Control_structures (siehe auch c2.com/cgi/wiki?HeInventedTheTerm )
Warbo
2
Ich hätte das hochgestuft, bis ich zum letzten Absatz (über OO) gekommen wäre, der ihm stattdessen eine Ablehnung einbrachte! Das ist genau die Art von unüberlegter Überbeanspruchung von OO-Konstrukten, die Ravioli-Code erzeugt , der genauso unlesbar ist wie Spaghetti.
zwol