Wie bearbeite ich eine Kette von if-else if-Anweisungen, um die Clean Code-Prinzipien von Onkel Bob einzuhalten?

45

Ich versuche, den sauberen Codevorschlägen von Onkel Bob zu folgen und Methoden kurz zu halten.

Ich bin jedoch nicht in der Lage, diese Logik zu verkürzen:

if (checkCondition()) {addAlert(1);}
else if (checkCondition2()) {addAlert(2);}
else if (checkCondition3()) {addAlert(3);}
else if (checkCondition4()) {addAlert(4);}

Ich kann die anderen nicht entfernen und so das Ganze in kleinere Teile zerlegen, weil das "sonst" im "sonst wenn" die Leistung verbessert - die Bewertung dieser Bedingungen ist teuer, und wenn ich vermeiden kann, die folgenden Bedingungen zu bewerten, eine der ersten verursachen ist wahr, ich möchte sie vermeiden.

Auch semantisch gesehen macht es aus betriebswirtschaftlicher Sicht keinen Sinn, die nächste Bedingung zu bewerten, wenn die vorherige erfüllt wurde.


Bearbeiten: Diese Frage wurde als ein mögliches Duplikat von Elegant-Methoden für den Umgang mit if (if else) else identifiziert .

Ich glaube, das ist eine andere Frage (das sieht man auch, wenn man die Antworten dieser Fragen vergleicht).

Ev0oD
quelle
46
Was ist eigentlich falsch oder unklar an diesem Code in seinem Kontext? Ich kann nicht sehen, wie es möglicherweise verkürzt oder vereinfacht werden kann! Der Code, der die Bedingungen auswertet, scheint bereits gut berücksichtigt zu sein, ebenso wie die Methode, die als Ergebnis der Entscheidung aufgerufen wird. Sie müssen sich nur einige der folgenden Antworten ansehen, die den Code lediglich komplizieren!
Steve
38
An diesem Code ist nichts auszusetzen. Es ist sehr lesbar und leicht zu folgen. Alles, was Sie tun, um es weiter zu verkleinern, führt zu einer zusätzlichen Indirektion und erschwert das Verständnis.
17 von 26
20
Ihr Code ist in Ordnung. Setzen Sie Ihre verbleibende Energie in etwas Produktiveres ein, als zu versuchen, es weiter zu verkürzen.
Robert Harvey
5
Wenn es wirklich nur 4 Bedingungen sind, ist das in Ordnung. Wenn es sich wirklich um 12 oder 50 handelt, möchten Sie wahrscheinlich auf einer höheren Ebene umgestalten als mit dieser einen Methode.
JimmyJames
9
Lassen Sie Ihren Code genau so, wie er ist. Hören Sie, was Ihre Eltern Ihnen immer gesagt haben: Vertrauen Sie keinem Onkel, der Kindern auf der Straße Süßigkeiten anbietet. @ Harvey Lustigerweise haben die verschiedenen Versuche, den Code zu "verbessern", ihn viel größer, komplizierter und weniger lesbar gemacht.
gnasher729

Antworten:

81

Idealerweise sollten Sie Ihre Logik extrahieren, um den Warnungscode / die Warnungsnummer in eine eigene Methode umzuwandeln. So wird Ihr bestehender Code auf das Wesentliche reduziert

{
    addAlert(GetConditionCode());
}

und GetConditionCode () kapselt die Logik zum Überprüfen von Bedingungen. Vielleicht ist es auch besser, eine Aufzählung als eine magische Zahl zu verwenden.

private AlertCode GetConditionCode() {
    if (CheckCondition1()) return AlertCode.OnFire;
    if (CheckCondition2()) return AlertCode.PlagueOfBees;
    if (CheckCondition3()) return AlertCode.Godzilla;
    if (CheckCondition4()) return AlertCode.ZombieSharkNado;
    return AlertCode.None;
}
Steven Eccles
quelle
2
Wenn es möglich ist, zu kapseln, wie Sie es beschreiben (ich vermute, dass dies nicht der Fall ist, ich denke, OP lässt der Einfachheit halber Variablen weg), ändert es den Code nicht, was an sich in Ordnung ist, sondern fügt Code-Ergonomie und ein bisschen Lesbarkeit hinzu 1
opa
17
Mit diesen Warnungscodes kann dankenswerterweise immer nur einer zurückgegeben werden
Josh Part
12
Dies passt auch perfekt zur Verwendung einer switch-Anweisung - wenn diese in der Sprache von OP verfügbar ist.
Frank Hopkins
4
Es ist wahrscheinlich nur eine gute Idee, den Fehlercode auf eine neue Methode zu übertragen, wenn dies so geschrieben werden kann, dass es in mehreren Situationen nützlich ist, ohne dass eine Reihe von Informationen über die jeweilige Situation bereitgestellt werden müssen. Tatsächlich gibt es einen Kompromiss und eine Gewinnschwelle, wenn es sich lohnt. Aber oft genug werden Sie feststellen, dass die Reihenfolge der Validierungen für den jeweiligen Job spezifisch ist und es besser ist, sie mit diesem Job zusammenzuhalten. In solchen Fällen ist es unerwünschter Ballast, einen neuen Typ zu erfinden, um einem anderen Teil des Codes mitzuteilen, was getan werden muss.
PJTraill
6
Ein Problem bei dieser Neuimplementierung besteht darin, dass die Funktion addAlertnach einer falschen Alarmbedingung suchen muss AlertCode.None.
David Hammen
69

Das wichtige Maß ist die Komplexität des Codes, nicht die absolute Größe. Unter der Annahme, dass die verschiedenen Bedingungen nur einzelne Funktionsaufrufe sind und die Aktionen nicht komplexer sind als die von Ihnen gezeigten, würde ich sagen, dass am Code nichts falsch ist. Es ist schon so einfach wie es sein kann.

Jeder Versuch einer weiteren "Vereinfachung" wird die Dinge in der Tat komplizieren.

Natürlich können Sie das elseSchlüsselwort durch ein ersetzen, returnwie andere vorgeschlagen haben, aber das ist nur eine Frage des Stils, nicht der Komplexität.


Beiseite:

Mein allgemeiner Rat wäre, niemals religiös über eine Regel für sauberen Code zu werden: Die meisten Codierungsratschläge, die Sie im Internet sehen, sind gut, wenn sie in einem passenden Kontext angewendet werden, aber wenn Sie diese Ratschläge radikal überall anwenden, können Sie einen Einstieg gewinnen die IOCCC . Der Trick ist immer, ein Gleichgewicht zu finden, das es den Menschen ermöglicht, leicht über Ihren Code nachzudenken.

Verwenden Sie zu große Methoden, und Sie sind geschraubt. Verwenden Sie zu kleine Funktionen, und Sie sind geschraubt. Vermeiden Sie ternäre Ausdrücke, und Sie sind geschraubt. Verwenden Sie überall ternäre Ausdrücke, und Sie sind geschraubt. Stellen Sie fest, dass es Orte gibt, die einzeilige Funktionen erfordern, und Orte, die Funktionen mit 50 Zeilen erfordern (ja, sie existieren!). Stellen Sie fest, dass es Stellen gibt, an denen if()Anweisungen erforderlich sind , und Orte, an denen der ?:Operator erforderlich ist. Nutzen Sie das gesamte Arsenal, das Ihnen zur Verfügung steht, und versuchen Sie immer, das am besten geeignete Werkzeug zu verwenden, das Sie finden können. Und denken Sie daran, auch bei diesem Rat nicht religiös zu werden.

cmaster
quelle
2
Ich würde argumentieren, dass das Ersetzen else ifdurch ein Inneres returngefolgt von einem Einfachen if(Entfernen des else) den Code schwieriger zu lesen machen könnte . Wenn der Code sagt else if, weiß ich sofort, dass der Code im nächsten Block nur ausgeführt wird, wenn der vorherige nicht ausgeführt wurde. Kein Muss, keine Aufregung. Wenn es eine Ebene ifist, wird sie möglicherweise ausgeführt oder nicht, unabhängig davon, ob die vorherige ausgeführt wurde. Jetzt muss ich einige mentale Anstrengungen unternehmen, um den vorherigen Block zu analysieren und festzustellen, dass er mit a endet return. Ich würde diese mentale Anstrengung lieber darauf verwenden, die Geschäftslogik zu analysieren.
einen Lebenslauf vom
1
Ich weiß, es ist eine kleine Sache, aber zumindest für mich else ifbildet es eine semantische Einheit. (Für den Compiler ist es nicht unbedingt eine Einheit, aber das ist in Ordnung.) ...; return; } if (...Tut es nicht. geschweige denn, wenn es auf mehrere Zeilen verteilt ist. Das ist etwas, worauf ich eigentlich achten muss, um zu sehen, was es tut, anstatt es direkt durch das bloße Anzeigen des Schlüsselwortpaars aufnehmen zu können else if.
ein Lebenslauf vom
@ MichaelKjörling Full Ack.Ich würde das else ifKonstrukt vorziehen , zumal seine verkettete Form ein so bekanntes Muster ist. Code des Formulars if(...) return ...;ist jedoch auch ein bekanntes Muster, daher würde ich das nicht vollständig verurteilen. Ich sehe dies jedoch als kleines Problem: Die Steuerungsablauflogik ist in beiden Fällen dieselbe, und ein genauerer Blick auf eine if(...) { ...; return; }Leiter zeigt mir, dass dies tatsächlich einer else ifLeiter entspricht. Ich sehe die Struktur eines einzelnen Begriffs, leite seine Bedeutung ab, merke, dass er sich überall wiederholt, und ich weiß, was los ist.
CMASTER
Aus JavaScript / node.js kommend, würden einige den Code "belt and suspenders" verwenden, bei dem sowohl else if als auch verwendet werden return . zBelse if(...) { return alert();}
user949300
1
"Und denk dran, werde auch bei diesem Rat nicht religiös." +1
Worte wie Jared
22

Es ist umstritten, ob dies für einen bestimmten Fall "besser" ist als die Ebene if..else. Aber wenn Sie wollen etwas anderes versuchen dies ist ein gemeinsamer Weg , es zu tun.

Fügen Sie Ihre Bedingungen in Objekte ein und fügen Sie diese Objekte in eine Liste ein

foreach(var condition in Conditions.OrderBy(i=>i.OrderToRunIn))
{
    if(condition.EvaluatesToTrue())
    {
        addAlert(condition.Alert);
        break;
    }
}

Wenn mehrere Aktionen erforderlich sind, können Sie eine verrückte Rekursion durchführen

void RunConditionalAction(ConditionalActionSet conditions)
{
    foreach(var condition in conditions.OrderBy(i=>i.OrderToRunIn))
    {
        if(condition.EvaluatesToTrue())
        {
            RunConditionalAction(condition);
            break;
        }
    }
}

Natürlich ja. Dies funktioniert nur, wenn Sie ein Muster für Ihre Logik haben. Wenn Sie versuchen, eine super-generische rekursive bedingte Aktion auszuführen, ist das Setup für das Objekt so kompliziert wie die ursprüngliche if-Anweisung. Sie erfinden Ihre eigene neue Sprache / Framework.

Aber Ihr Beispiel hat ein Muster

Ein häufiger Anwendungsfall für dieses Muster wäre die Validierung. Anstatt :

bool IsValid()
{
    if(condition1 == false)
    {
        throw new ValidationException("condition1 is wrong!");
    }
    elseif(condition2 == false)
    {
    ....

}

Wird

[MustHaveCondition1]
[MustHaveCondition2]
public myObject()
{
    [MustMatchRegExCondition("xyz")]
    public string myProperty {get;set;}
    public bool IsValid()
    {
        conditions = getConditionsFromReflection()
        //loop through conditions
    }
}
Ewan
quelle
27
Dies verschiebt die if...elseLeiter nur in die Konstruktion der ConditionsListe. Der Nettogewinn ist negativ, da die Konstruktion von Conditionsgenauso viel Code wie der OP-Code benötigt, aber die zusätzliche Indirektion ist mit Kosten für die Lesbarkeit verbunden. Ich würde definitiv eine sauber codierte Leiter vorziehen.
cmaster
3
@cmaster ja, ich glaube, ich habe genau das gesagt "dann wird das Setup für das Objekt so kompliziert sein wie die ursprüngliche if-Anweisung £
Ewan
7
Dies ist weniger lesbar als das Original. Um herauszufinden, welcher Zustand tatsächlich überprüft wird, müssen Sie in einem anderen Bereich des Codes graben. Es fügt eine unnötige Indirektionsebene hinzu, die das Verständnis des Codes erschwert.
17 von 26
8
Das Konvertieren einer if .. else if .. else .. -Kette in eine Tabelle mit Prädikaten und Aktionen ist sinnvoll, jedoch nur für viel größere Beispiele. Die Tabelle fügt einige Komplexität und Indirektion hinzu, sodass Sie genügend Einträge benötigen, um diesen konzeptionellen Aufwand zu amortisieren. Behalten Sie also für 4 Prädikat- / Aktionspaare den einfachen Originalcode bei. Wenn Sie jedoch 100 hatten, gehen Sie auf jeden Fall zur Tabelle. Der Übergangspunkt liegt irgendwo dazwischen. @cmaster, die Tabelle kann statisch initialisiert werden, sodass der inkrementelle Aufwand für das Hinzufügen eines Vergleichselement- / Aktionspaars eine einzige Zeile ist, die sie lediglich benennt: Besser geht es kaum.
Stephen C. Steel
2
Die Lesbarkeit ist NICHT persönlich. Es ist eine Pflicht gegenüber der Programmöffentlichkeit. Es ist subjektiv. Genau deshalb ist es wichtig, an Orte wie diesen zu kommen und zuzuhören, was das Programmpublikum dazu zu sagen hat. Persönlich finde ich dieses Beispiel unvollständig. Zeig mir, wie conditionses aufgebaut ist ... ARG! Keine Annotation-Attribute! Warum Gott? Au meine Augen!
candied_orange
7

Ziehen Sie in Betracht, return;nach dem Erfolg einer Bedingung alle elses zu sparen . Möglicherweise können Sie sogar return addAlert(1)direkt, wenn diese Methode einen Rückgabewert hat.

Kilian Foth
quelle
3
Dies setzt natürlich voraus, dass nach der Kette von ifs nichts mehr passiert ... Das könnte eine vernünftige Annahme sein, und dann auch nicht.
ein Lebenslauf vom
5

Ich habe Konstruktionen wie diese gesehen, die manchmal als sauberer angesehen werden:

switch(true) {
    case cond1(): 
        statement1; break;
    case cond2():
        statement2; break;
    case cond3():
        statement3; break;
    // .. etc
}

Ternär mit richtigem Abstand kann auch eine gute Alternative sein:

cond1() ? statement1 :
cond2() ? statement2 :
cond3() ? statement3 : (null);

Sie könnten auch versuchen, ein Array mit einem Paar zu erstellen, das Bedingung und Funktion enthält, und es durchlaufen, bis die erste Bedingung erfüllt ist - was, wie ich sehe, Ewans erster Antwort entspricht.

zworek
quelle
1
Ternary ist ordentlich
Ewan
6
@Ewan das Debuggen eines gebrochenen "tief rekursiven Ternärs" kann ein unnötiger Schmerz sein.
Freitag,
5
es sieht aus , obwohl auf dem Bildschirm ordentlich.
Ewan
Ähm, in welcher Sprache können Funktionen mit caseLabels verwendet werden?
undercat
1
@undercat das ist ein gültiges ECMAScript / JavaScript afaik
zworek
1

Als Variante von @ Ewans Antwort könnten Sie eine Kette (anstelle einer "flachen Liste") von Bedingungen wie folgt erstellen :

abstract class Condition {
  private static final  Condition LAST = new Condition(){
     public void alertOrPropagate(DisplayInterface display){
        // do nothing;
     }
  }
  private Condition next = Last;

  public Condition setNext(Condition next){
    this.next = next;
    return this; // fluent API
  }

  public void alertOrPropagate(DisplayInterface display){
     if(isConditionMeet()){
         display.alert(getMessage());
     } else {
       next.alertOrPropagate(display);
     }
  }
  protected abstract boolean isConditionMeet();
  protected abstract String getMessage();  
}

Auf diese Weise können Sie Ihre Bedingungen in einer definierten Reihenfolge anwenden, und die Infrastruktur (die dargestellte abstrakte Klasse) überspringt die verbleibenden Prüfungen, nachdem die erste erfüllt wurde.

Hier ist es dem Ansatz der "flachen Liste" überlegen, bei dem Sie das "Überspringen" in der Schleife implementieren müssen, die die Bedingungen anwendet.

Sie richten einfach die Konditionskette ein:

Condition c1 = new Condition1().setNext(
  new Condition2().setNext(
   new Condition3()
 )
);

Und starten Sie die Auswertung mit einem einfachen Aufruf:

c1.alertOrPropagate(display);
Timothy Truckle
quelle
Ja, das nennt man das Chain-of-Responsibility-Muster
Max
4
Ich gebe nicht vor, für irgendjemanden zu sprechen, aber während der Code in der Frage sofort lesbar und in seinem Verhalten offensichtlich ist, würde ich dies nicht als sofort offensichtlich betrachten, was er tut.
ein Lebenslauf vom
0

Erstens ist der ursprüngliche Code IMO nicht schrecklich. Es ist ziemlich verständlich und nichts ist von Natur aus schlecht.

Wenn Sie es nicht mögen, bauen Sie auf @ Ewans Idee auf, eine Liste zu verwenden, aber sein etwas unnatürliches foreach breakMuster zu entfernen :

public class conditions
{
    private List<Condition> cList;
    private int position;

    public Condition Head
    {
        get { return cList[position];}
    }

    public bool Next()
    {
        return (position++ < cList.Count);
    }
}


while not conditions.head.check() {
  conditions.next()
}
conditions.head.alert()

Passen Sie dies nun in der Sprache Ihrer Wahl an, machen Sie jedes Element der Liste zu einem Objekt, einem Tupel, was auch immer, und Sie sind gut.

EDIT: Es sieht so aus, als wäre es nicht so klar, wie ich dachte. Lassen Sie mich das näher erläutern. conditionsist eine geordnete Liste irgendeiner Art; headwird das aktuelle Element untersucht - am Anfang ist es das erste Element der Liste, und jedes Mal, wenn next()es aufgerufen wird, wird es das folgende; check()und alert()sind das checkConditionX()und addAlert(X)vom OP.

Nico
quelle
1
(Hat aber nicht runtergestimmt) Ich kann dem nicht folgen. Was ist Kopf ?
Belle-Sophie
@Belle Ich habe die Antwort bearbeitet, um weiter zu erklären. Es ist die gleiche Idee wie bei Ewan, aber mit einem while notanstelle von foreach break.
Nico
Eine brillante Entwicklung einer brillanten Idee
Ewan
0

Der Frage fehlen einige Einzelheiten. Wenn die Bedingungen sind:

  • Änderungen vorbehalten oder
  • wiederholt in anderen Teilen der Anwendung oder des Systems oder
  • in bestimmten Fällen geändert (z. B. verschiedene Builds, Tests, Bereitstellungen)

oder wenn der inhalt addAlertkomplizierter ist, dann wäre eine möglicherweise bessere lösung in say c #:

//in some central spot
IEnumerable<Tuple<Func<bool>, int>> Conditions = new ... {
  Tuple.Create(CheckCondition1, 1),
  Tuple.Create(CheckCondition2, 2),
  ...
}

//at the original place
var matchingCondition = Conditions.Where(c=>c.Item1()).FirstOrDefault();
if(matchingCondition != null) 
  addAlert(matchingCondition.Item2)

Tupel sind in c # <8 nicht so schön, aber aus Gründen der Sicherheit ausgewählt.

Der Vorteil dieser Methode ist, dass die Struktur statisch typisiert ist, auch wenn keine der oben genannten Optionen zutrifft. Sie können nicht versehentlich vermasseln, indem Sie sagen, ein else.

NiklasJ
quelle
0

Der beste Weg, um die Komplexität der Zyklomatik in Fällen zu reduzieren, in denen Sie viel zu tun haben, if->then statementsbesteht darin, ein Wörterbuch oder eine Liste (sprachabhängig) zu verwenden, um den Schlüsselwert (if-Anweisungswert oder einen Wert von) und dann ein Wert- / Funktionsergebnis zu speichern.

Zum Beispiel anstelle von (C #):

if (i > 10) { return "Two"; }
else if (i > 8) { return "Four" }
else if (i > 4) { return "Eight" }
return "Ten";  //etc etc say anything after 3 or 4 values

Ich kann einfach

var results = new Dictionary<int, string>
{
  { 10, "Two" },
  { 8, "Four"},
  { 4, "Eight"},
  { 0, "Ten"},
}

foreach(var key in results.Keys)
{
  if (i > results[key]) return results.Values[key];
}

Wenn Sie modernere Sprachen verwenden, können Sie mehr Logik als nur Werte speichern (c #). Dies sind eigentlich nur Inline-Funktionen, aber Sie können auch einfach auf andere Funktionen verweisen, wenn die Logik zu eng ist, um sie in die Linie zu setzen.

var results = new Dictionary<Func<int, bool>, Func<int, string>>
{
  { (i) => return i > 10; ,
    (i) => return i.ToString() },
  // etc
};

foreach(var key in results.Keys)
{ 
  if (key(i)) return results.Values[key](i);
}
Erik Philips
quelle
0

Ich versuche, den sauberen Codevorschlägen von Onkel Bob zu folgen und Methoden kurz zu halten.

Ich bin jedoch nicht in der Lage, diese Logik zu verkürzen:

if (checkCondition()) {addAlert(1);}
else if (checkCondition2()) {addAlert(2);}
else if (checkCondition3()) {addAlert(3);}
else if (checkCondition4()) {addAlert(4);}

Ihr Code ist bereits zu kurz, aber die Logik selbst sollte nicht geändert werden. Auf den ersten Blick sieht es so aus, als würden Sie sich mit vier Anrufen wiederholen checkCondition(), und es ist nur offensichtlich, dass jeder nach erneutem sorgfältigen Lesen des Codes anders ist. Sie sollten korrekte Formatierungs- und Funktionsnamen hinzufügen, z.

if (is_an_apple()) {
  addAlert(1);
}
else if (is_a_banana()) {
  addAlert(2);
}
else if (is_a_cat()) {
  addAlert(3);
}
else if (is_a_dog()) {
  addAlert(4);
}

Ihr Code sollte vor allem lesbar sein. Ich glaube, nachdem er mehrere Bücher von Onkel Bob gelesen hat, ist dies die Botschaft, die er konsequent vermitteln will.

CJ Dennis
quelle
0

Angenommen, alle Funktionen sind in derselben Komponente implementiert, könnten Sie dafür sorgen, dass die Funktionen einen bestimmten Status beibehalten, um die mehreren Zweige im Ablauf zu entfernen.

EG: checkCondition1()würde werden evaluateCondition1(), auf dem es prüfen würde, ob vorherige Bedingung erfüllt wurden; In diesem Fall wird ein Wert zwischengespeichert, der von abgerufen werden soll getConditionNumber().

checkCondition2()würde werden evaluateCondition2(), auf dem es prüfen würde, ob die bisherigen Bedingungen erfüllt waren. Wenn die vorherige Bedingung nicht erfüllt wurde, wird nach Bedingungsszenario 2 gesucht und ein Wert zwischengespeichert, von dem abgerufen werden soll getConditionNumber(). Und so weiter.

clearConditions();
evaluateCondition1();
evaluateCondition2();
evaluateCondition3();
evaluateCondition4();
if (anyCondition()) { addAlert(getConditionNumber()); }

BEARBEITEN:

Hier ist, wie die Prüfung auf teure Bedingungen implementiert werden müsste, damit dieser Ansatz funktioniert.

bool evaluateCondition34() {
    if (!anyCondition() && A && B && C) {
        conditionNumber = 5693;
        return true;
    }
    return false;
}

...

bool evaluateCondition76() {
    if (!anyCondition() && !B && C && D) {
        conditionNumber = 7658;
        return true;
    }
    return false;
}

Wenn Sie zu viele teure Überprüfungen durchführen müssen und die in diesem Code enthaltenen Informationen vertraulich bleiben, können Sie diese auf diese Weise beibehalten und die Reihenfolge der Überprüfungen bei Bedarf ändern.

clearConditions();
evaluateCondition10();
evaluateCondition9();
evaluateCondition8();
evaluateCondition7();
...
evaluateCondition34();
...
evaluateCondition76();

if (anyCondition()) { addAlert(getConditionNumber()); }

Diese Antwort bietet nur einen alternativen Vorschlag von den anderen Antworten und wird wahrscheinlich nicht besser als der ursprüngliche Code sein, wenn wir nur 4 Codezeilen berücksichtigen. Dies ist zwar kein schrecklicher Ansatz (und erschwert auch nicht die Wartung, wie andere bereits gesagt haben) in Anbetracht des erwähnten Szenarios (zu viele Überprüfungen, nur die Hauptfunktion wird als öffentlich angezeigt, alle Funktionen sind Implementierungsdetails derselben Klasse).

Emerson Cardoso
quelle
Dieser Vorschlag gefällt mir nicht - er verbirgt die Testlogik in mehreren Funktionen. Dies kann die Pflege des Codes erschweren, wenn Sie beispielsweise die Reihenfolge ändern und Schritt 3 vor Schritt 2 ausführen müssen.
Lawrence
Nein. Sie können überprüfen, ob eine vorherige Bedingung ausgewertet wurde, wenn anyCondition() != false.
Emerson Cardoso
1
Ok, ich verstehe, worauf du hinaus willst. Wenn jedoch (sagen wir) die Bedingungen 2 und 3 beide bewertet werden true, möchte das OP nicht, dass die Bedingung 3 bewertet wird.
Lawrence
Was ich damit gemeint habe ist, dass man anyCondition() != falseinnerhalb von Funktionen nachsehen kann evaluateConditionXX(). Dies ist realisierbar. Wenn der Ansatz der Verwendung des internen Status nicht gewünscht ist, verstehe ich, aber das Argument, dass dies nicht funktioniert, ist nicht gültig.
Emerson Cardoso
1
Ja, mein Einwand ist, dass es die Testlogik unbeholfen verbirgt, nicht, dass es nicht funktionieren kann. In Ihrer Antwort (Absatz 3) wird der Check für die Erfüllungsbedingung 1 innerhalb von eval ... 2 () platziert. Wenn er jedoch die Bedingungen 1 und 2 auf der obersten Ebene wechselt (aufgrund von Änderungen der Kundenanforderungen usw.), müssen Sie in die Auswertung gehen ... 2 (), um die Prüfung für Bedingung 1 zu entfernen, und in die Auswertung gehen. ..1 (), um eine Prüfung für Bedingung 2 hinzuzufügen. Dies kann zum Funktionieren gebracht werden, kann jedoch leicht zu Problemen bei der Wartung führen.
Lawrence
0

Mehr als zwei "else" -Klauseln zwingen den Leser des Codes, die gesamte Kette zu durchlaufen, um den gewünschten zu finden. Verwenden Sie eine Methode wie: void AlertUponCondition (Bedingungsbedingung) {switch (Bedingung) {case Condition.Con1: ... break; case Condition.Con2: ... break; etc ...} Wobei "Bedingung" eine richtige Aufzählung ist. Geben Sie bei Bedarf einen Bool oder einen Wert zurück. Nennen Sie es wie folgt: AlertOnCondition (GetCondition ());

Einfacher geht es wirklich nicht und schneller als die If-else-Kette, wenn Sie einige Fälle überschreiten.

Jimmy T
quelle
0

Ich kann nicht für Ihre spezielle Situation sprechen, da der Code nicht spezifisch ist, aber ...

Code wie dieser ist oft ein Geruch für ein fehlendes OO-Modell. Sie haben wirklich vier Arten von Dingen, von denen jede ihrem eigenen Alarmertyp zugeordnet ist. Anstatt jedoch diese Entitäten zu erkennen und für jede Instanz eine Klasse zu erstellen, behandeln Sie sie als eine Sache und versuchen, dies zu einem späteren Zeitpunkt nachzuholen Sie müssen wissen, womit Sie es zu tun haben, um fortzufahren.

Polymorphismus könnte Ihnen besser gefallen haben.

Seien Sie misstrauisch gegenüber Code mit langen Methoden, die lange oder komplexe If-Then-Konstrukte enthalten. Oft möchten Sie dort einen Klassenbaum mit einigen virtuellen Methoden.

Martin Maat
quelle