2009-03-06 2 views
3

Ich entwickle eine BlackBerry App in Java und habe eine Options-Klasse, in der alle Benutzereinstellungen gespeichert sind. Das Problem ist, dass ich einige Bedingungen überprüfen muss, um zu wissen, wie ich reagieren soll. Wenn ich weitere Funktionen hinzufüge, werden dem Benutzer mehr GUI-Optionen angezeigt, weitere Einstellungen werden in der Optionsklasse gespeichert und es müssen weitere Bedingungen überprüft werden.Was ist der beste Weg, verschachtelte ifs in Code loszuwerden, während Sie nach Bedingungen suchen?

Nehmen Sie den folgenden Code beispielsweise:

private void doCallMonitoring(int callId){ 
    /*This is the part that I want to avoid. Having 
     multiple nested ifs. Here's just two conditions 
     but as I add more, it will get unmantainable 
     very quickly.*/ 
    if(Options.isActive().booleanValue()){ 
     callTime = new Timer(); 
     TimerTask callTimeTask = new TimerTask(){ 
      public void run(){ 
       callTimeSeconds++; 
     if((callTimeSeconds == Options.getSoftLimit().intValue()) && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)){ 
        injectDTMFTone(Phone.getActiveCall()); 
     }else if((callTimeSeconds >= Options.getHardLimit().intValue()) && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)){ 
        injectEndCall(); 
       } 
      } 
     };  
     callTime.schedule(callTimeTask, 0,1000); 
    }else{ 
    System.out.println("Service not active"); 
    } 
} 

Wie ich möchte es arbeiten alle Optionen mit einem einzigen Anruf zu überprüfen und von dort den Fluch der Handlung zu bestimmen. Wie kann ich ein solches Design erreichen?

+0

Ich empfehle Ihnen, das Buch "Refactoring" von Martin Fowler zu kaufen: http://martinfowler.com/books.html ist ein Muss. – OscarRyz

+0

Danke für die Empfehlung. Ich werde es nachsehen. – Cesar

Antwort

4

Eine andere Möglichkeit besteht darin, Methoden wie injectDMTFTone() zu prüfen, ob sie diese Bedingung verarbeiten möchten, und je nachdem, ob sie behandelt wurde oder nicht, true oder false zurückgeben.

Zum Beispiel:

public void run() { 
    callTimeSeconds++; 
    do { 
     if (handleInjectDMTFTone()) 
      break; 
     if (handleInjectEndCall()) 
      break; 
    } while(false); 

    callTime.schedule(callTimeTask, 0,1000); 
} 

boolean handleInjectDMTFTone() { 
    if ((callTimeSeconds != Options.getSoftLimit().intValue()) || 
     (Phone.getActiveCall().getStatus() != PhoneCall.STATUS_CONNECTED)) 
     return false; 

    injectDTMFTone(Phone.getActiveCall()); 
    return true; 
} 

boolean handleInjectEndCall() { 

    if ((callTimeSeconds < Options.getHardLimit().intValue()) || 
     (Phone.getActiveCall().getStatus() != PhoneCall.STATUS_CONNECTED)) 
     return false; 

    injectEndCall(); 
    return true; 
} 

Natürlich anstelle eines anderen injectDMTFTone() Verfahren oder injectEndCall() Methode aufrufen, würden Sie diese Logik direkt in diesen Methoden Inline gerade.Auf diese Weise haben Sie die gesamte Logik, wie und wann mit diesen Bedingungen umzugehen, gruppiert.

Dies ist eines meiner Lieblingsmuster; Verwenden Sie if Anweisungen so nah an der Spitze der Methoden, wie sinnvoll, um Bedingungen zu beseitigen und zurückzukehren. Der Rest der Methode ist nicht um viele Ebenen eingerückt und einfach zu lesen.

Sie können dies weiter erweitern, indem Sie Objekte erstellen, die alle dieselbe Schnittstelle implementieren und sich in einem Repository von Handlern befinden, die Ihre Methode durchlaufen kann, um zu sehen, welche davon behandelt wird. Das kann oder kann nicht zu viel für Ihren Fall sein.

+0

Nicht übertrieben. Es sieht sowohl interessant als auch einfache Lösung aus. Ich werde es versuchen. – Cesar

2

Sie können Refactoring "extrahieren Methode" verwenden und alle diese Überprüfungen in einem "lesbaren" Zustand haben.

sehen Sie dieses verwandten answer Ist etwas langatmig, aber der Punkt ist, Konstrukte wie folgt zu ersetzen:

 }else if((callTimeSeconds >= Options.getHardLimit().intValue()) && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)){ 
       injectEndCall(); 
      } 
     } 

Für etwas wie folgt aus:

 .... 
     }else if(shouldInjectEndCall()){ 
       injectEndCall(); 
      } 
     } 
     ... 

Denken Sie daran, Objekte Zustand haben und ein anderes Objekt verwenden um ihnen zu helfen, ihre Arbeit zu tun.

1

Andere Option ist eine Art "ersetzen bedingte mit Polymorphismus".

Obwohl es scheint, als ob Sie nur mehr Code schreiben, könnten Sie alle diese Regeln durch ein "Validator" -Objekt ersetzen und alle Validierungen in einem Array haben und durchlaufen.

So etwas wie dieser Scratch-Code.

private void doCallMonitoring(int callId){ 
    // Iterate the valiators and take action if needed. 

     for(Validation validation : validationRules) { 
      if(validation.succeed()) { 
       validation.takeAction(); 
      } 
     } 
    } 

Und Sie sie wie folgt implementieren:

abstract class Validation { 

     public boolean suceed(); 
     public void takeAction(); 
} 

class InjectDTMFToneValidation extends Validation { 
    public boolean suceed() { 
     return (callTimeSeconds == Options.getSoftLimit().intValue()) 
       && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED) 
    } 
    public void takeAction() { 
     injectDTMFTone(Phone.getActiveCall()); 
    } 
} 

class InjectEndCallValidation extends Validation { 
    public boolean suceed() { 
     return (callTimeSeconds >= Options.getHardLimit().intValue()) 
       && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED) 
    } 
    public void takeAction() { 
     injectEndCall(); 
    } 
} 

Und schließlich diese in einer Liste installieren:

private List<Validation> validationRules = new ArrayList<Validation>();{ 
    validationrules.add(new InjectDTMFToneValidation()); 
    validationrules.add(new InjectEndCallValidation()); 
    ... 
    ... 
} 

Die Idee hier ist die Logik zu Subklassen zu bewegen. Natürlich erhalten Sie eine bessere Struktur und wahrscheinlich Suceed und TakeAction könnte für andere sinnvollere Methoden ersetzt werden, ist das Ziel, die Validierung von dort zu ziehen, wo es ist.

Es wird abstrakter? .. Ja.

BTW, warum verwenden Sie Optionen und Telefonklassen, die ihre statischen Methoden aufrufen, anstatt Instanzen zu verwenden?

+0

Das Telefon ist eine Klasse, die von RIMs JDE bereitgestellt wird. Es besteht aus statischen Methoden zur Steuerung der von BlackBeerries verwendeten Telefonanwendung. Die Optionsklasse wird für die dauerhafte Speicherung dieser Optionen verwendet. – Cesar

1

All diese Antworten sind wahrscheinlich bessere OO Antworten. Für einmal werde ich für die schnelle und schmutzige Antwort gehen.

Ich möchte komplexe verschachtelte ifs wirklich vereinfachen, indem ich sie invertiere.

if(!Options.isActive().booleanValue()) { 
    System.out.println("Service not active"); 
    return; 
} 
the rest... 

Ich kenne einige Leute Mitte Methode gibt nicht mögen, aber wenn Sie Teilnahmebedingungen sind die Validierung, die ein tolles Muster für mich schon immer, ich habe es nie bereut, es zu benutzen.

Es vereinfacht wirklich die Art, wie Ihre Methode aussieht.

Wenn Sie Methoden schreiben, die länger als ein Bildschirm sind, tun Sie dies nicht oder schreiben Sie einen großen Kommentar, der darauf hinweist - Es ist zu einfach, die Rückgabeanweisung zu verlieren und zu vergessen, dass Sie es getan haben. Besser noch, schreibe keine Methoden länger als ein Bildschirm.

Verwandte Themen