Verwendet ELSE schlechte Programmierung? [geschlossen]

Ich bin oft auf Fehler gestoßen, die durch die Verwendung des ELSEKonstrukts verursacht wurden. Ein Paradebeispiel ist etwa Folgendes:

If (passwordCheck() == false){
    displayMessage();
}else{
    letThemIn();
}

Für mich schreit dies nach Sicherheitsproblemen. Ich weiß, dass passwordCheck wahrscheinlich ein Boolescher Wert ist, aber ich würde die Sicherheit meiner Anwendungen nicht darauf platzieren. Was würde passieren, wenn es sich um einen String, Int usw. handelt?

Normalerweise versuche ich, die Verwendung zu vermeiden ELSE, und wähle stattdessen zwei vollständig separate IF-Anweisungen, um zu testen, was ich erwarte. Alles andere wird dann entweder ignoriert oder gezielt behandelt.

Dies ist sicherlich eine bessere Möglichkeit, um zu verhindern, dass Bugs / Sicherheitsprobleme in Ihre App gelangen.

Wie macht ihr das?

Der elseBlock sollte immer aus dem bestehen, was das Standardverhalten sein soll.

Sie müssen nicht umgangen werden. Achten Sie nur darauf, dass Sie sie ordnungsgemäß verwenden.

In Ihrem Beispiel sollte der Standardstatus sein, keinen Zugriff zuzulassen. Ein kleines Refactoring lässt Sie mit:

If (passwordCheck)
{
   letThemIn();
}
else
{
   displayMessage();
}

dh wenn die Passwortprüfung funktioniert, lassen Sie sie ein, andernfalls ist es immer gültig, eine Fehlermeldung anzuzeigen.

Sie können Ihrer Logik natürlich zusätzliche Prüfungen hinzufügen, indem Sie Anweisungen verwenden, die else ifnicht vollständig voneinander getrennt sind if.

Nein, daran ist nichts auszusetzen ELSE. ELSEist nicht das neue GOTO. Tatsächlich kann die Verwendung von zwei IFstatt ELSEzu mehreren Problemen führen.

Beispiel eins:

if (this(is(a(very())==complex(check())))) {
   doSomething();
}

if (!this(is(a(very())==complex(check())))) {
   doTheOtherThing();
}

Sie sehen das Kopieren-Einfügen? Es wartet nur auf den Tag, an dem Sie das eine wechseln und das andere vergessen.

Beispiel zwei:

foreach(x in y) {
  if (first_time) {
    first_time = false;
    doSomething();
  }

  if (!first_time) {
    doTheOtherThing();
  }
}

Wie Sie sehen, wird die zweite IFauch für die erste Position ausgeführt, da sich die Bedingung bereits geändert hat. In realen Programmen sind solche Fehler schwerer zu erkennen.

Es gibt immer ein ELSE. Wenn du schreibst

if(foo)
  bar();

du schreibst tatsächlich

if(foo)
{
  bar();
}
else
{
   // do nothing
}

Was auch immer Sie in den ELSE-Pfad stellen, liegt in Ihrer Verantwortung.

Ich persönlich neige dazu, elseso viel wie möglich zu vermeiden , aber es ist kein Sicherheitsproblem .

Beim Lesen von Code erschweren verschachtelte Anweisungen das Befolgen der Logik, da Sie sich merken müssen, welche Bedingungen dahin führen. Aus diesem Grund bin ich ein großer Fan des vorzeitigen Ausscheidens:

if (checkPassword != OK) { displayMessage(); return; }

letThemIn();

Dies gilt auch für forund whileSchleifen, in denen ich verwenden werde, continueund breakwann immer es eine Einrückungsebene vermeidet.

Chris Lattner sagt es besser als ich in den LLVM Coding Standards .

Dann ersetzen Sie sie einfach,

If (passwordCheck == true)
{
     letThemIn();
}
else
{
     displayMessage();
}

Wie Matthieu M. ziehe ich einen frühen Ausstieg tief verschachtelten anderen Blöcken vor ... Es veranschaulicht eine gut defensive Programmierung (bei schlechten Bedingungen macht es keinen Sinn, fortzufahren). Viele Leute werden mit uns nicht einverstanden sein und einen einzigartigen Ausstiegspunkt bevorzugen; Es ist nicht der Punkt der Debatte (glaube ich).

Jetzt nutze ich sicherlich, elsewenn es Sinn macht, vor allem für einfache, kurze Alternativen. Wie bereits erwähnt, ist das Duplizieren des Tests eine Zeitverschwendung (Programmierer und Prozessoren), eine Quelle der Verwirrung und später der Fehler (wenn einer geändert wird, nicht der andere).
Irgendwann füge ich einen Kommentar zum elseTeil hinzu und erinnere daran, was der Zustand war (insbesondere, wenn das ifTeil lang ist, z. B. im Legacy-Code) oder was die Alternative ist.

Beachten Sie, dass einige extreme Befürworter der funktionalen Programmierung vorschlagen, sich vollständig iffür den Pattern Matching zu entscheiden ... Ein bisschen zu extrem für meinen Geschmack. :-)

Es ist nichts Falsches an der Verwendung von ELSE. Dies kann jedoch zu übermäßig komplexem Code führen, der schwer zu lesen und zu verstehen ist. Dies kann auf ein schlechtes Design hindeuten. Es weist auf zusätzliche Anwendungsfälle hin, die getestet werden müssen.

Versuchen Sie ELSEs zu entfernen, wenn Sie können - aber seien Sie nicht paranoid. Steve McConnell nennt diesen geraden Code in Code Complete. Dh es gibt einen einfachen klaren Weg durch Ihren Code.

Lösungsansätze für Ihr spezielles Problem:

• verwende Polymorphismus. Überprüfen Sie an der Grenze zu Ihrem System die Sicherheitsdaten des Benutzers. Wenn sie legitim sind, geben Sie ein Sitzungsobjekt mit Zugriff auf die relevanten Teile des Systems zurück oder lösen eine Ausnahme aus. Dies könnte Ihr System jedoch komplexer machen. Sie entscheiden, was einfacher zu verstehen und zu warten ist.

Im Allgemeinen kann Folgendes dazu beitragen, ELSEs in Ihrem Code zu reduzieren:

• Gute Anforderungen können die Notwendigkeit solcher Entscheidungen im Kodex verringern. Möglicherweise müssen Sie den Anwendungsfall (else) überhaupt nicht implementieren.
• übersichtlicheres Design. Maximale Kohäsion und minimale Kopplung. Dies stellt sicher, dass Komponenten Entscheidungen, die in anderen Komponenten getroffen wurden, nicht duplizieren.
• Ausnahmebehandlung zur Verwaltung von Fehlerfällen.
• Polymorphismus (siehe Beispiel oben).
• switch-Anweisungen - Dies sind verherrlichte ELSEs, sind aber in bestimmten Situationen besser.

Ihre Vermutung, dass der Code ein Sicherheitsleck ist, kann je nach verwendeter Sprache zutreffen oder auch nicht . In C-Code kann dies ein Problem sein (insbesondere, weil in C ein Boolescher Wert nur ein Int ist, der nicht Null oder Null ist). In den am häufigsten typisierten Sprachen (dh Laufzeit-Typprüfung) kann die passwordCheckVariable jedoch als Boolescher Wert deklariert werden. Es gibt keine Möglichkeit, etwas anderes zuzuweisen. Tatsächlich muss alles in einem ifPrädikat in einen Booleschen Wert aufgelöst werden, unabhängig davon, ob Sie die Booleschen Operatoren oder einfach den Wert verwenden. Wenn es Ihnen gelingt, einen anderen Objekttyp an passwordCheckdie Laufzeit zu binden, würde dies eine Art unzulässiger Ausnahmebedingung auslösen.

Einfache if / else-Konstrukte sind viel einfacher zu lesen als if / if-Konstrukte - und weniger anfällig für versehentliche Probleme, wenn jemand versucht, das Konstrukt umzudrehen. Nehmen wir das gleiche Beispiel für eine Sekunde:

if(passwordCheck == false) {
    denyAccess();
}

if(passwordCheck) {
    letThemIn();
}

Die Bedeutung der sich gegenseitig ausschließenden Klauseln, die Sie oben ausführen möchten, geht verloren. Das vermittelt das if / else-Konstrukt. Zwei sich gegenseitig ausschließende Ausführungszweige, von denen immer einer ausgeführt wird. Dies ist ein wichtiger Teil der Sicherheit - um sicherzustellen, dass letThemInnach Ihrem Anruf keine Möglichkeit mehr besteht denyAccess.

Aus Gründen der Klarheit des Codes und um sicherzustellen, dass kritische Abschnitte am besten geschützt sind, sollten sie in der Primärklausel (dem ifTeil) enthalten sein. Das standardmäßige nicht konforme Verhalten sollte in der alternativen Klausel (dem elseTeil) enthalten sein. Beispielsweise:

if(passwordCheck) {
    letThemIn();
} else {
    denyAccess();
}

HINWEIS: In der Arbeit mit verschiedenen Sprachen habe ich ein Codierungs-Habbit entwickelt, mit dem die Frage "Was ist, wenn es sich um eine Zeichenfolge handelt?" Im Wesentlichen muss die Konstante im booleschen Ausdruck an erster Stelle stehen. Zum Beispiel, anstatt zu überprüfen, überprüfe passwordCheck == falseich false == passwordCheck. Dies vermeidet auch das in C ++ mögliche versehentliche Zuweisungsproblem. Bei dieser Vorgehensweise beschwert sich der Compiler, wenn ich =statt tippe ==. In Sprachen wie Java und C # würde der Compiler die Zuweisung in der if-Klausel als Fehler behandeln, aber C ++ wird dies gerne akzeptieren. Aus diesem Grund neige ich auch dazu, Nullprüfungen beim nullErsten durchzuführen .

Wenn Sie routinemäßig die Sprache wechseln, ist es sehr hilfreich, die Konstante an die erste Stelle zu setzen. In meinem Team widerspricht dies jedoch dem Kodierungsstandard, und der Compiler erkennt diese Probleme trotzdem. Es kann schwierig sein, sich davon zu lösen.

Zu sagen, dass das Verwenden elsebeim Programmieren schlecht ist, ist wie zu sagen, dass das Verwenden otherwisebeim Sprechen schlecht ist.

Sicher, sie können beide auf schlechte Weise verwendet werden, aber das bedeutet nicht, dass sie vermieden werden sollten, nur weil Sie einen Fehler gemacht haben, der sie zufällig eingeschlossen hat. Es würde mich nicht wundern, wenn viele Fehler von einem fehlenden defaultFall in einer switchAussage abhängen .

Stellen Sie sich vor, Sie Elselisten Ihren Bewerbungsfluss auf. Sie prüfen, ob Bedingungen erfüllt sind, die das Fortfahren des Anwendungsflusses SOLLTEN. Wenn diese Bedingungen nicht erfüllt sind, werden Sie Elseausgeführt, um das Problem zu beheben, die Anwendungsausführung abzubrechen oder ähnliches.

Else an sich ist nicht schlecht, aber wenn Sie es schlecht verwenden, können Sie unerwünschte Effekte sehen.

Auch in Bezug auf Ihre Aussage über

"Ich weiß, dass passwordCheck wahrscheinlich ein Boolescher Wert ist, aber ich würde die Sicherheit meiner Anwendungen nicht darauf platzieren."

Für von Ihnen entwickelte Methoden wird IMMER ein Datentyp zurückgegeben. Obwohl PHP Core mit Code übersät ist, der zwei oder mehr Datentypen zurückgibt, ist dies eine schlechte Praxis, da Funktionsaufrufe erraten werden. Wenn Sie mehr als einen Datentyp zurückgeben müssen, ziehen Sie in Betracht, eine Ausnahme auszulösen (ich finde, dies ist oft der Grund, warum ich einen anderen Datentyp zurückgeben möchte - etwas lief schrecklich, schrecklich falsch), oder überlegen Sie, Ihren Code so zu strukturieren, dass Sie es können gibt nur einen Datentyp zurück.

Als allererstes. LOL! Theres KEIN GRUND, zum sonst überhaupt zu vermeiden. Es ist keine schlechte Praxis in irgendeiner Weise, Form oder Gestalt.

Wenn überhaupt, sollte der Code sein

if(!IsLoggedIn) { ShowBadLoginOrNoAccessPage(); return }

Theres keine zwei wenns dort und es hat kein anderes. Dies ist, was ich in allen meinen Apps mache, mit Ausnahme einer, in der ich eine Ausnahme auslöse. Die Ausnahme ist in meiner Funktion abgefangen, die die URL überprüft, damit die richtige Seite angezeigt wird (oder ich kann alternativ die catch / check in asp.net-Fehlerfunktion setzen). Es wird eine generische Seite gedruckt, die besagt, dass keine Autorisierung vorliegt oder welche Meldung ich in der Ausnahme verwende (ich überprüfe immer die Art der Ausnahme und setze den http-Statuscode).

-Edit- wie in ammoQ Beispiel zwei gezeigt, wenns lächerlich ist. Wirklich, sonst ist es genauso gut oder besser als wenn. Wenn irgendetwas zu vermeiden ist (obwohl ich es persönlich nicht tue. Aber ich benutze return und break viel), da gesagt wurde, dass mehr Codepfade die Wahrscheinlichkeit von Fehlern erhöhen. Siehe zyklomatische Komplexität

-Bearbeiten 2- Wenn Sie besorgt sind, ob / else Verwendung. Ich beachte auch, dass ich es vorziehen würde, den kürzesten Codeblock nach oben zu setzen, wie z

if(cond == false) {
    a()
    b()
    c()
    onetwothree()
}
else
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}

Eher, als

if(cond) 
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}
else
{
    a()
    b()
    c()
    onetwothree()
}

Ich setze gerne einen Standard vor Bedingungen, wenn ich kann. Ich denke, es ist ein bisschen einfacher zu lesen und etwas expliziter, aber das ist nur eine Präferenz. Ich neige dazu, negative Bedingungen in meinem Code zu vermeiden. Ich bin kein großer Fan davon, nach! Foo oder false == foo zu suchen, und ich habe das Gefühl, dass sonst irgendwie das bedingte Äquivalent eines Negativs ist.

foo = bar;

if ('fubar' == baz) {
    foo = baz;
}

anstatt ...

if ('fubar' == baz) {
    foo = baz;
} else {
    foo = bar;
}

Der vorherige Codeblock scheint mir nur ein bisschen leichter zu lesen zu sein. Es erscheint mir natürlicher, eine Art skeptische Paranoia in Bezug auf meinen Code zu haben. Durch das Festlegen einer Standardeinstellung unabhängig von einer Bedingung fühle ich mich wohl: P

Ich würde argumentieren, dass die Verwendung von Verzweigungslogik jeglicher Art so weit wie möglich vermieden werden sollte. Während mit ELSE oder IF nichts falsch ist, gibt es viele Möglichkeiten, Code zu schreiben, um die Notwendigkeit der Verwendung von Verzweigungslogik zu minimieren. Ich sage nicht, dass Verzweigungslogik vollständig beseitigt werden kann - es wird an einigen Stellen benötigt -, aber es ist möglich, Code umzugestalten, um einen guten Teil davon zu beseitigen. In den meisten Fällen wird dadurch die Verständlichkeit und Genauigkeit Ihres Codes verbessert.

Zum Beispiel sind ternäre Operatoren normalerweise auch gute Kandidaten:

If VIP Then 
  Discount = .25
Else
  Discount = 0
End If
Total = (1 - Discount) * Total

Verwenden Sie einen ternären Ansatz:

Discount = VIP ? .25 : 0
Total = (1 - Discount) * Total

Ternäre Operatoren verschieben die Verzweigung gut nach rechts.