2016-04-15 10 views
9

Situazione: sto esaminando i nomi dei file, il nome del file viene memorizzato in una variabile denominata Stringstr e secondo le condizioni controllato in if dichiarazioni sto impostando il valore di una variabile denominata mailType.qual è l'alternativa migliore per seguire if-else ladder in java?

if(str.contains("template")) 
     {     
      if(str.contains("unsupported")) 
       mailType="unsupported";  
       else 
        if(str.contains("final_result"))     
         mailType="final_result";    
        else 
         if(str.contains("process_success")) 
          mailType="Process Success"; 
         else 
          if(str.contains("receive"))       
           mailType="Receive";       
          else 
          if(str.contains("sen")) 
           mailType="sent"; 
          else 
           if(str.contains("welcome")) 
            mailType="welcome"; 
           else 
            if(str.contains("manual")) 
            mailType="Manual";      
     }  
     else     
if(str.contains("properties")) 
     { 

      if(str.contains("unsupported")) 
       mailType="unsupported";  
       else 
        if(str.contains("final_result"))     
         mailType="final_result";    
        else 
         if(str.contains("process_success")) 
          mailType="Process Success"; 
         else 
          if(str.contains("receive"))       
           mailType="Receive";       
          else 
          if(str.contains("sen")) 
           mailType="sent"; 
          else 
           if(str.contains("welcome")) 
            mailType="welcome"; 
           else 
            if(str.contains("manual")) 
            mailType="Manual"; 

     } 

Problema: C'è un modo migliore per farlo in java che accorcia il mio codice ed è la memoria amichevole?

+3

Il tuo codice sarebbe molto più leggibile se si è utilizzato 'se else' piuttosto che' else' (senza parentesi) e rientrare il blocco successivo. –

+3

Basta fare il punto di Andy: http://pastie.org/10798591 o meglio ancora http: // pastie.org/10798593 –

+1

@ T.J.Crowder grazie per aver aggiunto quelli! –

risposta

20

Utilizzare un LinkedHashMap<String, String>:

LinkedHashMap<String, String> mapping = new LinkedHashMap<>(); 
mapping.put("unsupported", "unsupported"); 
mapping.put("final_result", "final_result"); 
// ... etc 

Poi scorrere la mappa fino a trovare una chiave corrispondente:

for (Map.Entry<String, String> entry : mapping.entrySet()) { 
    if (str.contains(entry.getKey()) { 
    mailType = entry.getValue(); 
    break; 
    } 
} 

Il punto chiave qui è che LinkedHashMap conserva ordine di inserimento (a differenza HashMap) in modo da poter in realtà specificare l'ordine in cui si desidera testare le corrispondenze (anche altre implementazioni di mappe lo fanno, ad es. Guava's ImmutableMap; LinkedHashMap è semplicemente uno di cui si dispone e scatola).

Se avete bisogno di nido questo per i casi esterni, è possibile semplicemente applicare lo stesso schema:

LinkedHashMap<String, LinkedHashMap<String, String>> outerMapping = 
    new LinkedHashMap<>(); 
outerMapping.put("template", mapping); 
outerMapping.put("properties", someOtherMapping); 

E poi basta scorrere i tasti nello stesso modo:

for (Map.Entry<String, LinkedHashMap<String, String>> outerEntry : outerMapping.entrySet()) { 
    if (str.contains(outerEntry.getKey()) { 
    // Apply the iteration above, using outerEntry.getValue(). 
    } 
} 
+0

Senza offesa ma trovo questo approccio un po 'complesso, dato che sono un principiante, potresti suggerire qualcos'altro? Inoltre questo approccio richiederà più memoria, giusto? –

+0

@varunsinghal l'approccio scelto dipende da cosa si desidera ottimizzare. Questo è solo un approccio; Posso capire perché potresti considerarlo complesso come un principiante. È abbastanza buono se si vuole essere in grado di specificare i mapping dinamicamente (ad esempio in una configurazione esterna); come si suol dire, YMMV :) –

+2

@varunsinghal: Quanto sopra userà solo una quantità veramente banale più memoria, nulla di cui preoccuparsi. È anche molto facile da mantenere, e in realtà non è affatto complicato. FWIW, è quello che farei, o qualcosa di molto simile ad esso. –

0

First , se si utilizza else if anziché il semplice else, si avranno meno indentazioni.

if(str.contains("unsupported")) 
      mailType="unsupported";  
else if(str.contains("final_result"))     
      mailType="final_result";    
else if(str.contains("process_success")) 
      mailType="Process Success"; 
else if(str.contains("receive"))       
      mailType="Receive";       
else if(str.contains("sen")) 
      mailType="sent"; 
else if(str.contains("welcome")) 
      mailType="welcome"; 
else if(str.contains("manual")) 
      mailType="Manual";   

Nel vostro caso particolare, vorrei suggerire di provare a usare espressioni regolari per semplificare un po 'il vostro lavoro:

Pattern p = Pattern.compile("(unsupported|final_result|welcome)"); 
Matcher m = p.matcher(str); 
if (m.matches()) { 
    mailType = m.group(1); 
} 
+2

L'approccio regex non funziona del tutto - alcuni dei 'mailType's non sono uguali a quelli corrispondenti (ad esempio' process_success' => 'Process Success'). –

+0

Sì, ma almeno può aiutare a rimuovere alcune delle istruzioni if. In caso di elaborazione di stringhe, provo a usare regex quando posso perché è lo strumento appropriato. –

+1

"provo a usare regex quando posso perché è lo strumento appropriato" Non hai mai sentito chiaramente il ["Ora hai due problemi"] (http://programmers.stackexchange.com/questions/223634/what- è-inteso-da-ora-hai-due-problemi) citazione :) –

2

Hai detto a trovare Andy's answer (che è quello che farei) troppo complesso. La sua original comment suggerendo else if può essere un'opzione per voi:

if (str.contains("template")) { 
    if (str.contains("unsupported")) 
     mailType = "unsupported"; 
    else if (str.contains("final_result")) 
     mailType = "final_result"; 
    else if (str.contains("process_success")) 
     mailType = "Process Success"; 
    else if (str.contains("receive")) 
     mailType = "Receive"; 
    else if (str.contains("sen")) 
     mailType = "sent"; 
    else if (str.contains("welcome")) 
     mailType = "welcome"; 
    else if (str.contains("manual")) 
     mailType = "Manual"; 
} else if (str.contains("properties")) { 
    if (str.contains("unsupported")) 
     mailType = "unsupported"; 
    else if (str.contains("final_result")) 
     mailType = "final_result"; 
    else if (str.contains("process_success")) 
     mailType = "Process Success"; 
    else if (str.contains("receive")) 
     mailType = "Receive"; 
    else if (str.contains("sen")) 
     mailType = "sent"; 
    else if (str.contains("welcome")) 
     mailType = "welcome"; 
    else if (str.contains("manual")) 
     mailType = "Manual"; 
} 

o, meglio ancora, con l'uso costante di {}:

if (str.contains("template")) { 
    if (str.contains("unsupported")) { 
     mailType = "unsupported"; 
    } else if (str.contains("final_result")) { 
     mailType = "final_result"; 
    } else if (str.contains("process_success")) { 
     mailType = "Process Success"; 
    } else if (str.contains("receive")) { 
     mailType = "Receive"; 
    } else if (str.contains("sen")) { 
     mailType = "sent"; 
    } else if (str.contains("welcome")) { 
     mailType = "welcome"; 
    } else if (str.contains("manual")) { 
     mailType = "Manual"; 
    } 
} else if (str.contains("properties")) { 
    if (str.contains("unsupported")) { 
     mailType = "unsupported"; 
    } else if (str.contains("final_result")) { 
     mailType = "final_result"; 
    } else if (str.contains("process_success")) { 
     mailType = "Process Success"; 
    } else if (str.contains("receive")) { 
     mailType = "Receive"; 
    } else if (str.contains("sen")) { 
     mailType = "sent"; 
    } else if (str.contains("welcome")) { 
     mailType = "welcome"; 
    } else if (str.contains("manual")) { 
     mailType = "Manual"; 
    } 
} 
1

Prova questa

static String containsAndValue(Collection<String> collection, String oldValue, String... strings) { 
    if (strings.length % 2 != 0) 
     throw new IllegalArgumentException("strings"); 
    for (int i = 0; i < strings.length; i += 2) 
     if (collection.contains(strings[i])) 
      return strings[i + 1]; 
    return oldValue; 
} 

e

if (str.contains("template")) { 
    mailType = containsAndValue(str, mailType, 
     "unsupported", "unsupported", 
     "final_result", "final_result", 
     "process_success", "Process Success", 
     "receive", "Receive", 
     "sen", "sent", 
     "welcome", "welcome", 
     "manual", "Manual"); 
} else if (str.contains("properties")) { 
    mailType = containsAndValue(str, mailType, 
     "unsupported", "unsupported", 
     "final_result", "final_result", 
     "process_success", "Process Success", 
     "receive", "Receive", 
     "sen", "sent", 
     "welcome", "welcome", 
     "manual", "Manual"); 
} 
1

È questo meno complesso?

public enum MailType { 
    UNSUPPORTED("unsupported", "unsupported"), 
    FINAL_RESULT("final_result", "final_result"), 
    PROCESS_SUCCESS("process_success", "Process Success"), 
    RECEIVE("receive", "Receive"), 
    SENT("sen", "sent"), 
    WELCOME("welcome", "welcome"), 
    MANUAL("manual", "Manual"); 

    private final String query; 
    private final String value; 

    MailType(String query, String value) { 
     this.query = query; 
     this.value = value; 
    } 

    public static String find(String text){ 
     for(MailType mailType: values()){ 
      if(text.contains(mailType.query)) { 
       return mailType.value; 
      } 
     } 
     return null; 
    } 
} 

e usarlo in questo modo:

String mailType = MailType.find(str); 
Problemi correlati