2008-08-08 14 views
28

Es ist etwas, das mich in jeder Sprache abgehört hat, ich habe eine if-Anweisung, aber der bedingte Teil hat so viele Prüfungen, dass ich es über mehrere Zeilen teilen muss, eine verschachtelte if-Anweisung verwenden oder einfach akzeptieren, dass es hässlich ist mach weiter mit meinem Leben.Wie gehen Sie mit großen Wenn-Bedingungen um?

Gibt es andere Methoden, die Sie gefunden haben, die mir und jedem anderen nützlich sein könnten, der das gleiche Problem hat?

Beispiel, alle auf einer Linie:

if (var1 = true && var2 = true && var2 = true && var3 = true && var4 = true && var5 = true && var6 = true) 
{ 

Beispiel, multi-line:

if (var1 = true && var2 = true && var2 = true 
&& var3 = true && var4 = true && var5 = true 
&& var6 = true) 
{ 

Beispiel verschachtelte:

if (var1 = true && var2 = true && var2 = true && var3 = true) 
{ 
    if (var4 = true && var5 = true && var6 = true) 
    { 

Antwort

61

Separate die Bedingung in mehreren booleans und dann ein Master-boolean als Bedingung verwenden.

bool isOpaque = object.Alpha == 1.0f; 
bool isDrawable = object.CanDraw && object.Layer == currentLayer; 
bool isHidden = hideList.Find(object); 

bool isVisible = isOpaque && isDrawable && ! isHidden; 

if(isVisible) 
{ 
    // ... 
} 

Besser noch:

public bool IsVisible { 
    get 
    { 
     bool isOpaque = object.Alpha == 1.0f; 
     bool isDrawable = object.CanDraw && object.Layer == currentLayer; 
     bool isHidden = hideList.Find(object); 

     return isOpaque && isDrawable && ! isHidden; 
    } 
} 

void Draw() 
{ 
    if(IsVisible) 
    { 
     // ... 
    } 
} 

Stellen Sie sicher, Ihre Variablen Namen geben, actualy Absicht angeben, anstatt Funktion. Dies wird dem Entwickler helfen, Ihren Code zu verwalten ... es könnte DICH sein!

+0

Einfache, leicht-to-do und effektiv. –

5

Zuerst entferne ich würde alles == true Teile, das würde es 50% kürzer machen;)

Wenn ich einen großen Zustand habe, suche ich nach den Gründen. Manchmal sehe ich, dass ich Polymorphie verwenden soll, manchmal muss ich ein Zustandsobjekt hinzufügen. Grundsätzlich bedeutet dies, dass ein Refactoring erforderlich ist (ein Code-Geruch).

Manchmal verwende ich De-Morgan's laws, um boolesche Ausdrücke ein wenig zu vereinfachen.

1

ich Zuflucht zu trennen Werte sind:

Bool cond1 == (var1 && var2); 
Bool cond2 == (var3 && var4); 

if (cond1 && cond2) {} 
3

ich eine Menge Leute und Redakteure entweder Einrücken jede Bedingung in Ihrer if-Anweisung mit einem Tab gesehen habe, oder es mit den offenen paren passend:

if (var1 == true 
    && var2 == true 
    && var3 == true 
    ) { 
    /* do something.. */ 
} 

ich in der Regel die Nähe paren auf der gleichen Linie wie die letzte Bedingung gestellt:

if (var1 == true 
    && var2 == true 
    && var3 == true) { 
    /* do something.. */ 
} 

Aber ich glaube nicht, dass das so sauber ist.

6

Ich werde oft diese bis in der Komponente boolean Variablen aufgeteilt:

bool orderValid = orderDate < DateTime.Now && orderStatus != Status.Canceled; 
bool custValid = customerBalance == 0 && customerName != "Mike"; 
if (orderValid && custValid) 
{ 
... 
2

Nun, zunächst einmal, warum nicht:

if (var1 & & var2 & & var2 & & var3 & & var4 & & var5 & & var6) {
...

Außerdem ist es sehr schwierig, abstrakte Codebeispiele umzuformen. Wenn Sie ein bestimmtes Beispiel zeigen, wäre es einfacher, ein besseres Muster für das Problem zu finden.

Es ist nicht besser, aber was ich in der Vergangenheit getan habe: (Die folgende Methode verhindert Kurzschlüsse Boolean Tests, alle Tests werden ausgeführt, auch wenn der erste falsch ist. Nicht ein empfohlenes Muster, außer Sie wissen, dass Sie brauchen um immer den ganzen Code vor der Rückkehr auszuführen - Dank ptomato für die Entdeckung meines Fehlers!)

boolean ok = cond1;
ok & = cond2;
ok & = cond3;
ok & = cond4;
ok & = cond5;
ok & = cond6;

, die die gleiche wie: (nicht das gleiche, siehe oben Hinweis!)

ok = (cond1 & & cond2 & & cond3 & & cond4 & & cond5 & & cond6) ;

+0

Es ist nicht das gleiche, wenn der '&&' Operator kurzgeschlossen ist. – ptomato

+0

Wow. Ich hätte das wissen müssen. Meine erste Facepalm bei einer meiner eigenen Antworten ;-) –

0

Ich mag sie von Ebene brechen, so würde ich formatieren Sie beispielsweise wie folgt aus:

if (var1 = true 
&& var2 = true 
&& var2 = true 
&& var3 = true 
&& var4 = true 
&& var5 = true 
&& var6 = true){ 

Es ist praktisch, wenn Sie mehr Verschachtelung haben, wie diese (natürlich die realen Bedingungen wäre mehr interessanter als für alles "= true"):

if ((var1 = true && var2 = true) 
&& ((var2 = true && var3 = true) 
    && (var4 = true && var5 = true)) 
&& (var6 = true)){ 
4

Check out Implementation Patterns von Kent Beck. Es gibt ein bestimmtes Muster, an das ich denke, das in dieser Situation helfen kann ... es heißt "Wächter". Anstatt viele Bedingungen zu haben, können Sie sie in einen Wächter aufteilen, der klarstellt, welche Bedingungen in einer Methode ungünstig sind.

So zum Beispiel, wenn Sie eine Methode, die etwas tut, aber es gibt bestimmte Bedingungen, bei denen es nicht etwas tun sollten, anstatt:

public void doSomething() { 
    if (condition1 && condition2 && condition3 && condition4) { 
     // do something 
    } 
} 

Sie könnten es ändern:

public void doSomething() { 
    if (!condition1) { 
     return; 
    } 

    if (!condition2) { 
     return; 
    } 

    if (!condition3) { 
     return; 
    } 

    if (!condition4) { 
     return; 
    } 

    // do something 
} 

Es ist ein wenig ausführlicher, aber viel besser lesbar, vor allem, wenn Sie seltsame Verschachtelung beginnen, kann der Wächter helfen (kombiniert mit Extraktionsmethoden).

Ich empfehle das Buch übrigens sehr.

+0

'Schnelle Rückkehr' tötet 'Pfeilspitze' Anti-Muster auch :) –

+0

Ich stimme nicht zu, dass diese Guards es leichter zu lesen machen. Ein Kommentar zu dem "komplizierten" Zustand wäre besser. –

12

Ich bin überrascht, dass noch niemand diesen bekommen hat. Es gibt ein Refactoring speziell für diese Art von Problem:

http://www.refactoring.com/catalog/decomposeConditional.html

+3

Ich mag Decompose Conditional nicht, weil es die Codestruktur mit einmaligen Funktionen verunreinigt, die nicht wiederverwendbar sind.Ich hätte lieber eine große IF-Anweisung mit Kommentaren für jede "Gruppe" verwandter Prüfungen. –

7

Es gibt zwei Probleme hier zu adressieren: Lesbarkeit und Verständlichkeit

Die „Lesbarkeit“ Lösung ein Stil Problem ist und als solche ist offen für Interpretation . Meine Präferenz ist dies:

if (var1 == true && // Explanation of the check 
    var2 == true && // Explanation of the check 
    var3 == true && // Explanation of the check 
    var4 == true && // Explanation of the check 
    var5 == true && // Explanation of the check 
    var6 == true) // Explanation of the check 
    { } 

oder dies:

if (var1 && // Explanation of the check 
    var2 && // Explanation of the check 
    var3 && // Explanation of the check 
    var4 && // Explanation of the check 
    var5 && // Explanation of the check 
    var6) // Explanation of the check 
    { } 

Das heißt, diese Art von komplexen Check ziemlich schwierig sein kann, um geistig zu analysieren, während Sie den Code scannen (vor allem, wenn Sie nicht der ursprüngliche Autor). Betrachten sie eine Hilfsmethode zu abstrahieren einige der Komplexität zu schaffen weg:

/// <Summary> 
/// Tests whether all the conditions are appropriately met 
/// </Summary> 
private bool AreAllConditionsMet (
    bool var1, 
    bool var2, 
    bool var3, 
    bool var4, 
    bool var5, 
    bool var6) 
{ 
    return (
     var1 && // Explanation of the check 
     var2 && // Explanation of the check 
     var3 && // Explanation of the check 
     var4 && // Explanation of the check 
     var5 && // Explanation of the check 
     var6); // Explanation of the check 
} 

private void SomeMethod() 
{ 
    // Do some stuff (including declare the required variables) 
    if (AreAllConditionsMet (var1, var2, var3, var4, var5, var6)) 
    { 
     // Do something 
    } 
} 

Wenn nun visuell das Scannen der „Somemethod“ Methode, die tatsächliche Komplexität der Testlogik ist versteckt, aber die semantische Bedeutung ist für die Menschen erhalten zu verstehen, bei ein hohes Niveau. Wenn der Entwickler die Details wirklich verstehen muss, kann die AreAllConditionsMet-Methode untersucht werden.

Dies ist formal bekannt als das "Decompose Conditional" Refactoring-Muster, denke ich. Werkzeuge wie Resharper oder Refactor Pro! kann machen diese Art von Refactoring einfach!

In allen Fällen besteht der Schlüssel zu lesbarem und verständlichem Code darin, realistische Variablennamen zu verwenden. Während ich verstehe, dass dies ein künstliches Beispiel ist, sind "var1", "var2" usw. nicht akzeptable Variablennamen. Sie sollten einen Namen haben, der die zugrunde liegende Natur der Daten widerspiegelt, die sie darstellen.

0

Wenn Sie in Python programmiert werden passieren, dann ist es ein Kinderspiel mit der über die Liste Ihrer Variablen angewendet eingebaute in all() Funktion (Ich werde einfach hier Boolesche Literale verwenden):

>>> L = [True, True, True, False, True] 
>>> all(L) # True, only if all elements of L are True. 
False 
>>> any(L) # True, if any elements of L are True. 
True 

Gibt es eine entsprechende Funktion in Ihrer Sprache (C#? Java?). Wenn ja, ist das wahrscheinlich der sauberste Ansatz.

-2

Wenn Sie dies tun:

if (var1 == true) { 
    if (var2 == true) { 
     if (var3 == true) { 
      ... 
     } 
    } 
} 

Dann können Sie auch auf Fälle reagieren, wenn etwas nicht stimmt. Wenn Sie beispielsweise die Eingabe bestätigen, können Sie dem Benutzer einen Hinweis geben, wie er richtig formatiert wird.

+3

Dies ist vielleicht die schlechteste Lösung für diese Frage. –

+1

Horizontale Bildlaufleiste - aktivieren !!! 11eleven –

0

McDowell,

Sie sind richtig, dass, wenn die Single ‚&‘ Operator, dass beide Seiten des Ausdrucks zu bewerten. Wenn Sie jedoch den Operator '& &' (mindestens in C#) verwenden, ist der erste Ausdruck, der false zurückgibt, der letzte ausgewertete Ausdruck. Dies macht es möglich, die Eva- lation vor der FOR-Anweisung genauso gut zu stellen wie jede andere Art, dies zu tun.

1

Wie andere erwähnt haben, würde ich Ihre Bedingungen analysieren, um zu sehen, ob es eine Möglichkeit gibt, die Sie auf andere Methoden auslagern können, um die Lesbarkeit zu erhöhen.

0

@tweakt

Es ist nicht besser, aber was ich in der Vergangenheit getan:

boolean ok = cond1; ok & = cond2; ok & = cond3; ok & = cond4; ok & = cond5; ok & = cond6;

, die die gleiche wie:

ok = (cond1 & & cond2 & & cond3 & & cond4 & & cond5 & & cond6);

Eigentlich sind diese beiden Dinge in den meisten Sprachen nicht gleich. Der zweite Ausdruck hört normalerweise auf, ausgewertet zu werden, sobald eine der Bedingungen falsch ist, was eine große Leistungsverbesserung darstellen kann, wenn die Bewertung der Bedingungen teuer ist.

Für die Lesbarkeit bevorzuge ich persönlich Mike Stones Vorschlag oben. Es ist einfach, ausführlich alle rechnerischen Vorteile zu kommentieren und zu bewahren, die es ermöglichen, frühzeitig herauszukommen. Sie können die gleiche Technik auch inline in einer Funktion ausführen, wenn die Organisation Ihres Codes dadurch verwirrt wird, dass die bedingte Bewertung weit von Ihrer anderen Funktion entfernt wird. Es ist ein bisschen kitschig, aber man kann immer etwas tun, wie:

do { 
    if (!cond1) 
     break; 
    if (!cond2) 
     break; 
    if (!cond3) 
     break; 
    ... 
    DoSomething(); 
} while (false); 

die while (false) ist eine Art kitschig. Ich wünschte, in den Sprachen hätte ein Scoping-Operator "einmal" genannt oder etwas, aus dem man leicht ausbrechen könnte.

+0

Das ist nicht besser als der Originalcode. –

2

Versuchen Sie, Funktoren und Prädikaten zu betrachten. Das Apache Commons-Projekt enthält eine große Menge von Objekten, mit denen Sie bedingte Logik in Objekte einkapseln können. Ein Beispiel ihrer Verwendung ist auf O'reilly here verfügbar. Auszug aus dem Codebeispiel:

import org.apache.commons.collections.ClosureUtils; 
import org.apache.commons.collections.CollectionUtils; 
import org.apache.commons.collections.functors.NOPClosure; 

Map predicateMap = new HashMap(); 

predicateMap.put(isHonorRoll, addToHonorRoll); 
predicateMap.put(isProblem, flagForAttention); 
predicateMap.put(null, ClosureUtils.nopClosure()); 

Closure processStudents = 
    ClosureUtils.switchClosure(predicateMap); 

CollectionUtils.forAllDo(allStudents, processStudents); 

Nun werden die Details aller dieser isHonorRoll Prädikate und die Verschlüsse verwendet, um sie zu bewerten:

import org.apache.commons.collections.Closure; 
import org.apache.commons.collections.Predicate; 

// Anonymous Predicate that decides if a student 
// has made the honor roll. 
Predicate isHonorRoll = new Predicate() { 
    public boolean evaluate(Object object) { 
    Student s = (Student) object; 

    return((s.getGrade().equals("A")) || 
      (s.getGrade().equals("B") && 
       s.getAttendance() == PERFECT)); 
    } 
}; 

// Anonymous Predicate that decides if a student 
// has a problem. 
Predicate isProblem = new Predicate() { 
    public boolean evaluate(Object object) { 
    Student s = (Student) object; 

    return ((s.getGrade().equals("D") || 
       s.getGrade().equals("F")) || 
      s.getStatus() == SUSPENDED); 
    } 
}; 

// Anonymous Closure that adds a student to the 
// honor roll 
Closure addToHonorRoll = new Closure() { 
    public void execute(Object object) { 
    Student s = (Student) object; 

    // Add an award to student record 
    s.addAward("honor roll", 2005); 
    Database.saveStudent(s); 
    } 
}; 

// Anonymous Closure flags a student for attention 
Closure flagForAttention = new Closure() { 
    public void execute(Object object) { 
    Student s = (Student) object; 

    // Flag student for special attention 
    s.addNote("talk to student", 2005); 
    s.addNote("meeting with parents", 2005); 
    Database.saveStudent(s); 
    } 
}; 
+0

Schön! Eine zum Herunterladen und Spielen mit Methinks. – toolkit

0

Ich mag jede Bedingung in beschreibenden Variablen brechen.

bool isVar1Valid, isVar2Valid, isVar3Valid, isVar4Valid; 
isVar1Valid = (var1 == 1) 
isVar2Valid = (var2.Count >= 2) 
isVar3Valid = (var3 != null) 
isVar4Valid = (var4 != null && var4.IsEmpty() == false) 
if (isVar1Valid && isVar2Valid && isVar3Valid && isVar4Valid) { 
    //do code 
} 
+0

Worauf kommt es bei der Erstellung einzelner Bools an? Außerdem vergleichen Sie jedes 'varX' mit' true' und weisen dies dem 'isVarXValid'-Bool zu, das im wesentlichen nur * longhand * für 'isVar1Valid = var' ist, was redundant ist. Du hast schon Boole, also warum nicht nur 'if (var1 && var2 && var3 && var4)' – dreamlax

+0

@dreamlax Du hast Recht. Ich habe ein armes Beispiel benutzt. – wusher

2

Steve Mcconell Anraten von Code Complete: Verwenden Sie eine mehrdimensionale Tabelle. Jede Variable dient als Index für die Tabelle und die if-Anweisung wird zu einer Tabellensuche. Zum Beispiel, wenn (Größe == 3 & & Gewicht> 70) schlägt sich in den Tabelleneintrag Entscheidung [size] [weight_group]

0

Wenn ich es in Perl tat, Dies ist, wie ich könnte die Kontrollen laufen.

{ 
    last unless $var1; 
    last unless $var2; 
    last unless $var3; 
    last unless $var4; 
    last unless $var5; 
    last unless $var6; 

    ... # Place Code Here 
} 

Wenn Sie sich mit dieser über ein Unterprogramm planen jede Instanz last mit return ersetzen;

1

In reflektierende Sprachen wie PHP, können Sie mit variabler Variablen verwenden:

$vars = array('var1', 'var2', ... etc.); 
foreach ($vars as $v) 
    if ($$v == true) { 
     // do something 
     break; 
    } 
0
if ( (condition_A) 
     && (condition_B) 
     && (condition_C) 
     && (condition_D) 
     && (condition_E) 
     && (condition_F) 
     ) 
    { 
     ... 
    } 

im Gegensatz zu

if (condition_A) { 
     if (condition_B) { 
      if (condition_C) { 
      if (condition_D) { 
       if (condition_E) { 
        if (condition_F) { 
         ... 
        } 
       } 
      } 
      } 
     } 
    } 

und

if ( ( (condition_A) 
      && (condition_B) 
      ) 
     || ( (condition_C) 
      && (condition_D) 
      ) 
     || ( (condition_E) 
      && (condition_F) 
      ) 
     ) 
    { 
     do_this_same_thing(); 
    } 

im Gegensatz zu

Die meisten statischen Analysetools zur Codeüberprüfung werden sich beschweren, wenn mehrere bedingte Ausdrücke keine expliziten Klammern verwenden, die die Ausdrucksanalyse diktieren, anstatt sich auf Vorrangregeln des Operators und weniger Klammern zu verlassen.

Vertikale Ausrichtung auf der gleichen Einzugsebene von öffnenden/schließenden Klammern {}, öffnende enge Klammern(), bedingte Ausdrücke mit Klammern und Operatoren auf der linken Seite ist eine sehr nützliche Übung, die die Lesbarkeit und Klarheit des Codes erheblich verbessert im Gegensatz zu Jamming alles, was möglicherweise auf eine einzige Zeile gestaut werden kann, ohne vertikale Ausrichtung, Leerzeichen oder Klammern

Operator Vorrang Regeln sind knifflig, z & & hat höhere Priorität als ||, aber | Vorrang hat als & &

So ...

if (expr_A & expr_B || expr_C | expr_D & expr_E || expr_E && expr_F & expr_G || expr_H { 
    } 

ist ein wirklich einfach, mehrere bedingter Ausdruck für bloße Menschen nicht richtig zu lesen und zu bewerten.

if ( ( (expr_A) 
      & (expr_B) 
      ) 
     || ( (expr_C) 
      | ( (expr_D) 
       & (expr_E) 
      ) 
      ) 
     || ( (expr_E) 
      && ( (expr_F) 
       & (expr_G) 
       ) 
      ) 
     || (expr_H) 
     ) 
    { 
    } 

Es ist nichts falsch mit horizontalem Raum (Zeilenvorschübe), vertikaler Ausrichtung, oder expliziten Klammern Ausdrucksauswertung zu führen, von denen alle ERHöHT Lesbarkeit und Klarheit