2009-03-06 8 views
3

Je développe une application BlackBerry en Java et j'ai une classe Options où tous les paramètres utilisateur sont stockés. Le problème est que j'ai besoin de vérifier certaines conditions afin de savoir comment réagir. Comme je continue d'ajouter plus de fonctionnalités, plus d'options d'interface graphique sont montrées à l'utilisateur, plus de paramètres sont stockés dans la classe Options et plus de conditions doivent être vérifiées.Quelle est la meilleure façon de se débarrasser des ifs imbriqués dans le code tout en vérifiant les conditions?

Prenez le code suivant par exemple:

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"); 
    } 
} 

Comment je veux que cela fonctionne est de vérifier toutes les options avec un seul appel et de là déterminer la malédiction de l'action. Comment puis-je réaliser une telle conception?

+0

Je vous recommande d'acheter le livre "Refactoring" par Martin Fowler: http://martinfowler.com/books.html est un must have. – OscarRyz

+0

Merci pour la recommandation. Je vais le chercher. – Cesar

Répondre

4

Une autre option consiste à faire des méthodes telles que injectDMTFTone() vérifier pour voir si elles veulent gérer cette condition, et retourner vrai ou faux selon si elle a été traitée ou non.

Par exemple:

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; 
} 

Bien sûr, au lieu d'appeler une autre méthode injectDMTFTone() ou injectEndCall() méthode, vous simplement inline ce droit logique dans ces méthodes.De cette façon, vous avez regroupé toute la logique de comment et quand traiter ces conditions au même endroit.

Ceci est l'un de mes modèles préférés; utilisez les instructions if aussi proches du sommet des méthodes que cela a du sens pour éliminer les conditions et revenir. Le reste de la méthode n'est pas indenté de nombreux niveaux, et est facile à lire.

Vous pouvez étendre cette fonctionnalité en créant des objets qui implémentent tous la même interface et se trouvent dans un référentiel de gestionnaires que votre méthode run peut parcourir pour voir qui la gérera. Cela peut ou non être exagéré dans votre cas.

+0

Pas trop. Il semble intéressant ainsi que la solution simple. Je vais essayer. – Cesar

2

Vous pouvez utiliser le refactoring "méthode d'extraction" et placer tous ces contrôles dans une condition "lisible".

voir cette answer connexe est un peu long, mais le point est de remplacer des constructions comme ceci:

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

Pour quelque chose comme ceci:

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

Rappelez-vous les objets n'ont l'état et peuvent utiliser un autre objet pour les aider à faire son travail.

1

L'autre option consiste à faire une sorte de "remplacer conditionnel avec polymorphisme".

Bien qu'il semble que d'écrire plus de code, vous pouvez remplacer toutes ces règles par un objet "validator" et avoir toutes les validations dans un tableau et les parcourir.

Quelque chose comme ce code à gratter.

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

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

Et vous les mettre en œuvre comme ceci:

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(); 
    } 
} 

Et enfin les installer dans une liste:

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

L'idée ici est de déplacer la logique sous-classes. Bien sûr, vous obtiendrez une meilleure structure et probablement sujour et takeAction pourrait être remplacé pour d'autres méthodes plus significatives, l'objectif est de tirer la validation d'où il est.

Cela devient plus abstrait? .. Oui.

BTW, pourquoi utiliser les options et les classes téléphoniques invoquant leurs méthodes statiques au lieu d'utiliser des instances?

+0

Phone est une classe fournie par RIMs JDE et elle consiste en des méthodes statiques pour contrôler l'application Phone que les BlackBerries utilisent. La classe Options est utilisée pour le stockage permanent desdites options. – Cesar

1

Toutes ces réponses sont probablement de meilleures réponses OO. Pour une fois, je vais aller chercher la réponse rapide et sale. J'aime vraiment simplifier les ifs imbriqués complexes comme ça en les inversant.

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

Je sais que certaines personnes n'aiment pas les retours mi-méthode, mais quand vous validez les conditions d'entrée qui a toujours été un modèle impressionnant pour moi, je ne l'ai jamais regretté de l'utiliser.

Cela simplifie vraiment l'apparence de votre méthode. Si vous écrivez des méthodes plus longues qu'un écran, ne le faites pas ou n'écrivez pas un grand commentaire pour le signaler - Il est trop facile de perdre l'instruction de retour et d'oublier que vous l'avez fait. Mieux encore, n'écrivez pas des méthodes plus longues qu'un écran.

Questions connexes