Wenn-sonst-Leiter, die alle Bedingungen erfüllen soll - sollte eine redundante Schlussklausel hinzugefügt werden?

10

Das ist eine Sache, die ich in letzter Zeit viel mache.

Beispiel:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else if (i > current) {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

Oft ist die if / else-Leiter wesentlich komplizierter ...

Siehe die letzte Klausel? Es ist überflüssig. Die Leiter soll letztendlich alle möglichen Bedingungen erfassen. So könnte es so umgeschrieben werden:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

So habe ich früher Code geschrieben, aber ich mag diesen Stil nicht. Meine Beschwerde ist, dass die Bedingung, unter der der letzte Teil des Codes ausgeführt wird, aus dem Code nicht ersichtlich ist. Ich habe daher begonnen, diese Bedingung explizit zu schreiben, um sie deutlicher zu machen.

Jedoch:

  • Das explizite Schreiben der endgültigen erschöpfenden Bedingung ist meine eigene Idee, und ich habe schlechte Erfahrungen mit meinen eigenen Ideen - normalerweise schreien mich die Leute an, wie schrecklich das ist, was ich tue - und (manchmal viel) später finde ich heraus, dass es tatsächlich so war suboptimal;
  • Ein Hinweis, warum dies eine schlechte Idee sein kann: Nicht anwendbar auf Javascript, aber in anderen Sprachen geben Compiler Warnungen oder sogar Fehler aus, wenn die Steuerung das Ende der Funktion erreicht. Der Hinweis, so etwas zu tun, ist vielleicht nicht allzu beliebt oder ich mache es falsch.
    • Compiler-Beschwerden haben mich manchmal dazu gebracht, die endgültige Bedingung in einen Kommentar zu schreiben, aber ich denke, dies ist schrecklich, da Kommentare im Gegensatz zu Code keinen Einfluss auf die tatsächliche Programm-Semantik haben:
    } else { // i > current
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }

Vermisse ich etwas Oder ist es in Ordnung, das zu tun, was ich beschrieben habe, oder ist es eine schlechte Idee?

Gaazkam
quelle
+1 für @ Christophes ausführliche Antwort, dass Redundanz im Code die Wahrscheinlichkeit von Fehlern erhöht. Es gibt auch ein viel geringeres Effizienzproblem. Um dies zu erweitern, könnten wir in Bezug auf das fragliche Codebeispiel die Reihe von Wenn-Sonst-Wenns als nur separate Wenns ohne andere schreiben, aber wir würden dies im Allgemeinen nicht tun, da die Wenn-Sonstigen dem Leser den Begriff exklusiver Alternativen geben anstatt einer Reihe unabhängiger Bedingungen (was auch weniger effizient wäre, da dies besagt, dass alle Bedingungen auch nach einem Spiel bewertet werden sollten).
Erik Eidt
@ErikEidt> da dies besagt, dass alle Bedingungen auch nach einer Übereinstimmung mit +1 ausgewertet werden sollten, es sei denn, Sie kehren von einer Funktion zurück oder "brechen" aus einer Schleife (was im obigen Beispiel nicht der Fall ist).
Darek Nędza
1
+1, nette Frage. Abgesehen davon könnte es Sie interessieren, dass das Beispiel in Gegenwart von NaNs nicht erschöpfend ist.
Monocell

Antworten:

6

Beide Ansätze sind gültig. Aber schauen wir uns die Vor- und Nachteile genauer an.

Für eine ifKette mit trivialen Bedingungen wie hier spielt es keine Rolle:

  • Mit einem Finale elseist es für den Leser offensichtlich, herauszufinden, in welchem ​​Zustand das Sonst ausgelöst wird.
  • Mit einem Finale else ifist es für den Leser ebenso offensichtlich, dass keine zusätzlichen elseerforderlich sind, da Sie alles behandelt haben.

Es gibt jedoch viele ifKetten, die auf komplexeren Bedingungen beruhen und Zustände mehrerer Variablen kombinieren, möglicherweise mit einem komplexen logischen Ausdruck. In diesem Fall ist es weniger offensichtlich. Und hier die Konsequenz jedes Stils:

  • final else: Sie sind sicher, dass einer der Zweige besetzt ist. Wenn Sie einen Fall vergessen haben, wird dieser letzte Zweig durchlaufen. Wenn Sie also beim Debuggen den letzten Zweig ausgewählt haben und etwas anderes erwartet haben, werden Sie schnell herausfinden.
  • final else if: Sie müssen die redundante Bedingung für den Code ableiten, und dies schafft eine potenzielle Fehlerquelle mit dem Risiko, nicht alle Fälle abzudecken. Wenn Sie einen Fall verpasst haben, wird nichts ausgeführt, und es kann schwieriger sein, herauszufinden, dass etwas fehlt (z. B. wenn einige Variablen, von denen Sie erwartet haben, dass sie festgelegt werden, Werte aus früheren Iterationen beibehalten).

Die endgültige redundante Bedingung ist also eine Risikoquelle. Deshalb würde ich lieber vorschlagen, ins Finale zu gehen else.

Bearbeiten: Codierung mit hoher Zuverlässigkeit

Wenn Sie mit Blick auf hohe Zuverlässigkeit entwickeln, könnte Sie eine andere Variante interessieren: Vervollständigen Sie Ihr redundantes explizites Finale else ifmit einem Finale else, um unerwartete Situationen zu erkennen.

Dies ist eine defensive Codierung. Es wird von einigen Sicherheitsspezifikationen wie SEI CERT oder MISRA empfohlen . Einige statische Analysetools implementieren dies sogar in der Regel , die systematisch überprüft wird (dies könnte Ihre Compiler-Warnungen erklären).

Christophe
quelle
7
Was ist, wenn ich nach der endgültigen Redundand-Bedingung ein anderes hinzufüge, das immer eine Ausnahme auslöst, die besagt: "Du solltest mich nicht erreichen!" - Wird es einige der Probleme meines Ansatzes lindern?
Gaazkam
3
@gaazkam ja, das ist ein sehr defensiver Codierungsstil. Sie müssen also noch die endgültige Bedingung berechnen, aber bei komplexen fehleranfälligen Ketten würden Sie dies zumindest schnell herausfinden. Das einzige Problem, das ich bei dieser Variante sehe, ist, dass es für offensichtliche Fälle ein wenig übertrieben ist.
Christophe
@gaazkam Ich habe meine Antwort bearbeitet, um auch Ihre zusätzliche Idee anzusprechen, die in Ihrem Kommentar erwähnt wird
Christophe
1
@gaazkam Eine Ausnahme zu werfen ist gut. Vergessen Sie in diesem Fall nicht, den unerwarteten Wert in die Nachricht aufzunehmen. Es kann schwierig sein, die Daten zu reproduzieren, und der unerwartete Wert kann einen Hinweis auf die Ursache des Problems geben.
Martin Maat
1
Eine andere Möglichkeit ist, einen assertins Finale zu setzen else if. Ihr Styleguide kann jedoch variieren, ob dies eine gute Idee ist. (Der Zustand eines assertsollte niemals falsch sein, es sei denn, der Programmierer hat es vermasselt. Daher wird die Funktion zumindest für den beabsichtigten Zweck verwendet. Aber so viele Leute missbrauchen sie, dass viele Geschäfte sie vollständig verboten haben.)
Kevin
5

Was bisher in den Antworten fehlt, ist eine Frage, welche Art von Fehler weniger schädlich ist.

Wenn Ihre Logik gut ist, spielt es keine Rolle, was Sie tun. Der wichtige Fall ist, was passiert, wenn Sie einen Fehler haben.

Sie lassen die letzte Bedingung weg: Die letzte Option wird ausgeführt, auch wenn dies nicht das Richtige ist.

Sie fügen einfach die letzte Bedingung hinzu: Abhängig von der Situation kann dies einfach bedeuten, dass etwas nicht angezeigt wird (geringer Schaden), oder es kann zu einem späteren Zeitpunkt eine Nullreferenzausnahme bedeuten (was ein Debugging sein kann) Schmerzen.)

Sie fügen die letzte Bedingung und eine Ausnahme hinzu: Es wird ausgelöst.

Sie müssen entscheiden, welche dieser Optionen die beste ist. Im Entwicklungscode halte ich dies für ein Kinderspiel - nehmen Sie den dritten Fall. Wahrscheinlich würde ich jedoch circle.src vor dem Auslösen auf ein Fehlerbild und circle.alt auf eine Fehlermeldung setzen - falls jemand beschließt, die Behauptungen später auszuschalten, schlägt dies harmlos fehl.

Eine andere Sache zu berücksichtigen - was sind Ihre Wiederherstellungsoptionen? Manchmal haben Sie keinen Wiederherstellungspfad. Was ich für das ultimative Beispiel halte, ist der erste Start der Ariane V-Rakete. Es gab einen nicht erfassten / 0-Fehler (tatsächlich ein Teilungsüberlauf), der zur Zerstörung des Boosters führte. In Wirklichkeit hatte der abgestürzte Code zu diesem Zeitpunkt keinerlei Zweck, er war in dem Moment, in dem die Strap-On-Booster aufleuchteten, umstritten. Sobald sie die Umlaufbahn oder den Boom entzünden, tun Sie das Beste, was Sie können. Fehler können nicht zugelassen werden. (Wenn die Rakete dadurch in die Irre geht, dreht der Sicherheitsbeauftragte seinen Schlüssel.)

Loren Pechtel
quelle
4

Was ich empfehle, ist die Verwendung einer assertAnweisung in Ihrem endgültigen else in einem dieser beiden Stile:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else {
        assert i > current
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

Oder eine tote Code-Behauptung:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else if (i > current) {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    } else {
        assert False, "Unreachable code"
    }
}

Das Code-Coverage-Tool kann häufig so konfiguriert werden, dass Code wie "Assert False" aus dem Coverage-Bericht ignoriert wird.


Indem Sie die Bedingung in eine Zusicherung einfügen, dokumentieren Sie die Bedingung eines Zweigs effektiv explizit. Im Gegensatz zu einem Kommentar kann die Zusicherungsbedingung jedoch tatsächlich überprüft werden und schlägt fehl, wenn Sie Zusicherungen während der Entwicklung oder in der Produktion aktiviert lassen (ich empfehle im Allgemeinen, Zusicherungen aktiviert zu lassen in der Produktion, wenn sie die Leistung nicht zu stark beeinträchtigen).

Lie Ryan
quelle
1
Ich mag Ihre erste Option nicht, die zweite ist viel klarer, dass es ein Fall sein sollte, der unmöglich sein sollte.
Loren Pechtel
0

Ich habe ein Makro "behauptet" definiert, das eine Bedingung auswertet und in einem Debug-Build in den Debugger fällt.

Wenn ich also zu 100 Prozent sicher bin, dass eine von drei Bedingungen erfüllt sein muss, schreibe ich

If condition 1 ...
Else if condition 2 .,,
Else if asserted (condition3) ...

Das macht deutlich genug, dass eine Bedingung erfüllt ist und kein zusätzlicher Zweig für eine Zusicherung erforderlich ist.

gnasher729
quelle
-2

Ich empfehle, alles andere ganz zu vermeiden . Verwenden Sie ifdiese Option, um zu deklarieren, was der Codeblock verarbeiten soll, und beenden Sie den Block, indem Sie die Funktion beenden.

Dies führt zu Code, der sehr klar ist:

setCircle(circle, i, { current })
{
    if (i == current)
    {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
        return
    }
    if (i < current)
    {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
        return
    }
    if (i > current)
    {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
        return
    }
    throw new Exception("Condition not handled.");
}

Das Finale ifist natürlich überflüssig ... heute. Es kann sehr wichtig werden, wenn ein zukünftiger Entwickler die Blöcke neu anordnet. Es ist also hilfreich, es dort zu lassen.

John Wu
quelle
1
Es ist eigentlich sehr unklar, denn jetzt muss ich überlegen, ob die Ausführung des ersten Zweigs das Ergebnis des zweiten Tests ändern könnte. Plus sinnlose Mehrfachretouren. Plus Möglichkeit, dass keine Bedingung wahr ist.
Gnasher729
Ich schreibe tatsächlich, wenn solche Aussagen getroffen werden können, wenn ich absichtlich mehrere Fälle erwarte. Dies ist besonders nützlich in Sprachen mit switch-Anweisungen, die kein Durchfallen zulassen. Ich denke, wenn sich die Auswahlmöglichkeiten gegenseitig ausschließen, sollte sie so geschrieben werden, dass es offensichtlich ist, dass sich die Fälle gegenseitig ausschließen (unter Verwendung von else). Und wenn es nicht so geschrieben ist, bedeutet dies, dass sich die Fälle nicht gegenseitig ausschließen (Durchfallen ist möglich).
Jerry Jeremiah
@ gnasher729 Ich verstehe deine Reaktion total. Ich kenne einige sehr kluge Leute, die Probleme mit "sonst meiden" und "vorzeitige Rückkehr" hatten ... zuerst. Ich ermutige Sie, sich etwas Zeit zu nehmen, um sich an die Idee zu gewöhnen und sie zu untersuchen - denn diese Ideen reduzieren objektiv und messbar die Komplexität. Die vorzeitige Rückgabe befasst sich tatsächlich mit Ihrem ersten Punkt (Sie müssen überlegen, ob ein Zweig einen anderen Test ändern könnte). Und die "Möglichkeit, dass keine Bedingung wahr ist" besteht trotzdem; Mit diesem Stil erhalten Sie eher eine offensichtliche Ausnahme als einen versteckten Logikfehler.
John Wu
Aber ist die zyklomatische Komplexität beider Stile nicht genau gleich? dh gleich der Anzahl der Bedingungen
Gaazkam