2011-09-08 15 views
19

Eventuali duplicati:
Long list of if statements in JavaCome rimuovere grandi if-else-if catena

mi è stato affidato il compito di lavorare con un codice, e non v'è un gigante if-else-if catena (100+-ifs) che controlla le stringhe.

Quali sono alcune buone tecniche per aggiornare questo codice su dove la catena if-else-if può essere ridotta a qualcosa di molto più gestibile.

La catena simile a questa:

if(name.equals("abc")){ 
    do something 
} else if(name.equals("xyz")){ 
    do something different 
} else if(name.equals("mno")){ 
    do something different 
} ...... 
..... 
else{ 
    error 
} 
+1

Lo schema dei comandi è il modo per gestire questo di sicuro, si veda: http://stackoverflow.com/questions/1199646/long-list-of-if-statements-in-java/1199677#1199677 –

+0

Cosa c'è di sbagliato in questo modo così com'è, oltre a sembrare fastidioso? –

risposta

16

È possibile estrarre il codice in ogni ramo in un metodo separato, quindi trasformare i metodi in implementazioni di un'interfaccia di base comune (chiamiamolo Handler). Dopodiché puoi riempire uno Map<String, Handler> e solo cercare e eseguire il gestore destro per la stringa specificata.

Sfortunatamente l'implementazione di oltre 100 sottoclassi per l'interfaccia richiede un bel po 'di codice boilerplate, ma attualmente non esiste un modo più semplice in Java per raggiungere questo obiettivo. L'implementazione dei casi come elementi di un Enum può essere di aiuto - here is an example. La soluzione ideale sarebbe utilizzare le chiusure/lambda, ma purtroppo dobbiamo aspettare fino a Java 8 per quello ...

+0

+1 Questo è il mio approccio abituale. Rende il codice pulito e facile da estendere. Quando Java diventa lambda lo renderà bello. – linuxuser27

+0

Che dire del fatto che è necessario mantenere molte istanze Handler nella mappa che probabilmente non verranno utilizzate? – Galya

+0

Hai misurato questo con JMH? Sarebbe bello sapere a che punto il costo di [dynamic dispatch] (http://shipilev.net/blog/2015/black-magic-method-dispatch/) è inferiore alla branching if-else. –

2

è possibile utilizzare l'istruzione switch, ma Interruttore dichiarazioni casi String sono state attuate in Java SE 7

la soluzione migliore è quella di utilizzare il command pattern

+1

non è una cattiva idea, ma fare un modello di comando penso che dovrebbe essere una soluzione migliore – Jorge

6

Come ha detto Matt Ball nel suo commento, è possibile utilizzare un modello di comando. Definire un insieme di classi Runnable:

Runnable task1 = new Runnable() { 
    public void run() { /* do something */ } 
}; 
Runnable task2 = // etc. 

quindi è possibile utilizzare una mappa dalle chiavi a runnables:

Map<String,Runnable> taskMap = new HashMap<String,Runnable>(); 
taskMap.put("abc", task1); 
taskMap.put("xyz", task2); 
// etc. 

Infine, sostituire il altrimenti se-catena con:

Runnable task = taskMap.get(name); 
if (task != null) { 
    task.run(); 
} else { 
    // default else action from your original chain 
} 
0

Questo è un popolare Arrow Anti-Pattern e Jeff discute alcuni approcci per gestirlo molto bene nel suo post here.

7

Alcune opzioni/idee:

  • Lascia così com'è - non è fondamentalmente rotto, ed è ragionevolmente chiaro e semplice da mantenere
  • utilizzare un'istruzione switch (se si utilizza Java 7) - non sicuro se questo ti fa guadagnare molto anche se
  • Creare una HashMap di String su FunctionObjects dove gli oggetti funzione implementano il comportamento richiesto come metodo. Quindi il tuo codice di chiamata è solo: hashMap.get(name).doSomething();
  • Rompere in una gerarchia di chiamate di funzione tramite il sottogruppo delle stringhe.È possibile eseguire questa operazione prendendo ogni lettera a turno, in modo che un ramo gestisca tutti i nomi che iniziano con "a" ecc.
  • Refactor in modo da non passare il nome come stringa ma passare un oggetto con nome. Quindi puoi semplicemente fare namedObject.doSomething()
8

Con Enum, puoi avere un metodo per istanza.

public enum ActionEnum { 
    ABC { 
     @Override 
     void doSomething() { 
      System.out.println("Doing something for ABC");  
     } 

    }, 
    XYZ { 
     @Override 
     void doSomething() { 
     System.out.println("Doing something for XYZ"); 
     } 
    }; 

    abstract void doSomething(); 
} 

public class MyActionClass { 

    public void myMethod(String name) { 
     ActionEnum.valueOf("ABC").doSomething(); 
    } 

} 

E 'ancora un pò disordinato (grande enum con 100 voci, anche tutto ciò che fa è dispacciamento), ma può evitare il codice di inizializzazione HashMap (100 + mette anche disordinato a mio parere).

E ancora un'altra opzione (a scopo di documentazione) sarebbe riflessione:

public interface Action { 
    void doSomething(); 
} 

public class ABCAction implements Action { 
    @Override 
    public void doSomething() { 
     System.out.println("Doing something for ABC");  
    } 
} 

public class MyActionClass { 

    void doSomethingWithReflection(String name) { 
     try { 
      Class<? extends Action> actionClass = Class. 
       forName("actpck."+ name + "Action").asSubclass(Action.class); 
      Action a = actionClass.newInstance(); 
      a.doSomething(); 
     } catch (Exception e) { 
      // TODO Catch exceptions individually and do something useful. 
      e.printStackTrace(); 
     } 
    } 
} 

Ogni approccio ha è di compromessi:

  • HashMap = veloce + Kinda disordinato codice ("set-up" con centinaia di put)
  • Enum = Fast + Kinda messy 2 (file enorme).
  • Reflection = Più lento + errore di runtime incline, ma fornisce una separazione netta senza ricorrere a HashMap giganteschi.
+0

Bella analisi, grazie! – bertie