Qual è il modo migliore per sbarazzarsi di if nidificati nel codice durante il controllo delle condizioni?

Question

Sto sviluppando un'app BlackBerry in Java e ho una classe Opzioni in cui sono memorizzate tutte le impostazioni dell'utente. Il problema è che devo verificare alcune condizioni per sapere come reagire. Mentre continuo ad aggiungere più funzioni, all'utente vengono mostrate più opzioni della GUI, vengono memorizzate più impostazioni nella classe Opzioni e devono essere verificate più condizioni.

Prendi ad esempio il seguente codice:

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

Come vorrei che funzionasse è verificare tutte le opzioni con una sola chiamata e da lì determinare la maledizione dell'azione. Come posso realizzare un tale design?

Answer 1

Un'altra opzione è fare in modo che metodi come injectDMTFTone () controllino se vogliono gestire quella condizione e restituiscono vero o falso a seconda che sia stata gestita o meno.

Ad esempio:

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

Naturalmente, invece di chiamare un altro metodo injectDMTFTone () o injectEndCall () , devi semplicemente incorporare quella logica proprio in quei metodi. In questo modo hai raggruppato tutta la logica di come e quando affrontare tali condizioni nello stesso posto.

Questo è uno dei miei modelli preferiti; usa le istruzioni if il più vicino possibile alla cima dei metodi per eliminare le condizioni e restituire. Il resto del metodo non è rientrato in molti livelli ed è facile e chiaro da leggere.

Puoi estenderlo ulteriormente creando oggetti che implementano tutti la stessa interfaccia e sono in un repository di gestori su cui il tuo metodo esegui può iterare per vedere chi lo gestirà. Ciò può o meno essere eccessivo per il tuo caso.

Answer 2

Puoi utilizzare il metodo "quot" extract " refactoring e avere tutti quei controlli in uno "leggibile" condizioni.

Vedi questa risposta relativa un po 'lungo ma il punto è sostituire i costrutti in questo modo:

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

Per qualcosa del genere:

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

Ricorda che gli oggetti hanno uno stato e possono usare altri oggetti per aiutarli a fare il loro lavoro.

Answer 3

Un'altra opzione è fare una sorta di "sostituzione condizionale con polimorfismo".

Anche se sembra solo scrivere più codice, puoi sostituire tutte quelle regole con un " validator " oggetto e avere tutte le convalide in un array e scorrere attraverso di esse.

Qualcosa di simile a questo codice scratch.

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

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

E le implementate in questo modo:

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

E infine installali in un elenco:

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

L'idea qui è di spostare la logica in sottoclassi. Naturalmente, otterrai una struttura migliore e probabilmente suceed e takeAction potrebbero essere sostituiti con altri metodi più significativi, l'obiettivo è di estrarre la convalida da dove si trova.

Diventa più astratto? .. sì.

A proposito, perché usi le opzioni Opzioni e Classi telefoniche invocando i loro metodi statici invece di usare le istanze?

Answer 4

Tutte quelle risposte sono probabilmente risposte OO migliori. Per una volta cercherò la risposta rapida e sporca.

Mi piace davvero semplificare complessi se nidificati del genere invertendoli.

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

So che ad alcune persone non piacciono i ritorni a metà metodo, ma quando convalidi le condizioni di ingresso che sono sempre state un modello fantastico per me, non mi sono mai pentito di averlo usato.

Semplifica davvero l'aspetto del tuo metodo.

Se scrivi metodi più lunghi di uno schermo, non farlo o scrivi un commento di grandi dimensioni sottolineandolo: è troppo facile perdere la dichiarazione di ritorno e dimenticare di averlo fatto. Meglio ancora, non scrivere metodi più lunghi di uno schermo.