Miglior approccio: converti più condizionali se -else in un design più maneggevole

Il problema con if / else chains è che sono inflessibili e accoppiano strettamente la classe con eventuali implementazioni usate se ce ne sono.

Per un approccio dinamico, consiglierei la stessa soluzione che ho citato in questa risposta .

Fondamentalmente, crei una serie di gestori di risposta e un gestore di risposte . L'ultimo è incaricato di inviare la risposta al proprio gestore .

La soluzione si basa su polimorfismo . Tutti i gestori devono implementare un'interfaccia ( IStockResponseHandler ) con due metodi:

• bool CanHandle(StockResponse response)
• Result Handle(StockResponse response)

Quindi, inizializza StockResponseManager con un elenco di istanze IStockResponseHandler .

Infine, una volta ricevuta una risposta, devi solo chiamare StockResponseManager.getResult(response) per ottenere il risultato corretto.

Come ho già detto nella mia risposta collegata, i benefici sono a prova di futuro. Consenti a te stesso di gestire nuovi tipi di risposte con poco o nessun lavoro aggiuntivo nel tuo progetto e senza aggiungere ancora un altro statico se la condizione alla tua catena if / else. Se hai caricato le regole da un database, in teoria puoi gestire dinamicamente le risposte senza apportare alcuna modifica al tuo programma.

Come regola generale, creerei una classe IStockResponseHandler per% tipo diResult. In altre parole, due condizioni potrebbero comportare lo stesso valore restituito Result , e in questo caso dovrebbe essere gestito dalla stessa classe, non da due classi separate. Nel tuo esempio, sto pensando che sarebbe una classe per ogni possibile valore di response.state.

Non esiste uno schema particolarmente complicato che utilizzerei. È sufficiente suddividere le alternative di livello superiore in singoli metodi come ProcessOKResponse() , ProcessUnknownResponse() ecc. E lasciare solo la logica di invio nel metodo del punto di ingresso.

Puoi in seguito trasformare la grande switch , ad es. in una tabella di ricerca o in una distribuzione dinamica tramite ereditarietà o altro, ma l'importante è ridurre la quantità di decisioni prese in un blocco di codice. Refactoring a metodi più piccoli è la chiave per questo.

(Esempio di pseudo codice per una tabella di ricerca metodo:

handlers = [
   OK: handle_ok, 
   unknown: handle_unknown, 
   backorder: handle_backorder
]

def handle_ok():
   if(response.Sender==Sender1){ /* code....*/ }
   else if ( response.Sender==Sender2){/*code ...*/ }
   else {/**default handling code...*/}

def handle_unknown():
   ...

def handle_response(response):
  handlers[response.state]()

Esempio di codice per la spedizione dinamica:

class OKResponse:
    def dispatch():
        if condition1:
            action1()
        elif condition2:
            action2()
        else:
            action0()

class UnknownResponse:
    def dispatch():
        ...

def handle_response(response_object):
    response_object.dispatch()

Pensa al factoring del secondo livello ifs per separare le funzioni. Ad esempio handleResponseOK() e handleResponseUnknown() . Quindi, il tuo codice sarà simile a questo:

public Result ProcessStockState(StockResponse response)
{
    if(response.state==StockStateEnum.Ok)
    {
        handleResponseOK(response);
    }
    else if(response.state==StockStateEnum.Unkown)
    {
        handleResponseUnknown(response);
    }
    else  if(response.state==X)
    {
        handleResponseX(response);
    }
}

Quando devi prendere una decisione, devi ancora mettere la logica per renderla da qualche parte e se ne hai bisogno più di una se ... beh, non c'è modo di sfuggirla.

Certo, puoi vestire il tuo codice con abiti fantasia a fantasia, ma ne hai davvero bisogno? Forse basta un buon vecchio refactoring se hai solo bisogno di ridurre il rientro e non vuoi codificare costruzioni elaborate per nascondere i tuoi if.

Uso di Java enum pseudocode:

public enum StockStateEnum {

    OK(
        public void process(Response response) {
          if(response.Sender==Sender1){ /* code....*/ }
          else if ( response.Sender==Sender2){/*code ...*/ }
          else {/**default handling code...*/}
        }
    ),

    UNKOWN(
      public void process(Response response) { 
        if(response.Sender==Sender5){ /* code....*/ }
        else if ( response.Sender==Sender7){/*code ...*/ }
        else if ( response.Sender==Sender10){/*code ...*/ }
        else {/*default handling code ...*/}
      }
    ),
    ...

    public abstract process(Response response);

    public static StockStateEnum findByState(Response response) {
      // Loop through enum values trying to find a match
      for (StockStateEnum stockState : StockStateEnum.values() {
        if (stockState == response.state) {
          return stockState;
        }
      }
      return StockStateEnum.DEFAULT;  // Or null, depending on requirement
    }
}

Utilizzo:

public Result ProcessStockState(StockResponse response) {
  StockStateEnum stockState = StockStateEnum.findByState(response);
  stockState.process(response);
  ...
}

La mia ipotesi è che potresti fare qualcosa di simile con response.Sender riducendo ancora di più il numero di istruzioni if e la complessità ciclomatica.

Considerando che il metodo principale sta restituendo Result, puoi semplificare else if istruzioni con solo if + return .

E come molti altri hanno suggerito, ovviamente ha senso estrarre se corpi di istruzioni in un metodo.

Credo che potrebbe essere qualcosa del genere:

public Result ProcessStockState(StockResponse response) {
   if(response.state==StockStateEnum.Ok) {
       return handleResponseOk();
   }

   if(response.state==StockStateEnum.Unkown) {
       return handleResponseUnknown();
   }

   if(response.state==X) {
       return handleResponseX();
   }

   return defaultResult; 
}

lo stesso approccio che puoi adottare con i metodi handleResponse .

Il polimorfismo è una tecnica che può aiutarti a ottenere ciò di cui hai bisogno. Ti aiuta a disaccoppiare il codice e rendere il tuo codice più scalabile per il futuro. Ecco la mia implementazione della tecnica che interrompe la condizione if-else.

Crea un dizionario con tutti gli StockResponseHandlers i.e Ok, sconosciuto, ecc.

        StockEnum stockEnum = response.state;
        Dictionary<StockEnum, IStockResponseHandler> dictionary = new Dictionary<StockEnum, IStockResponseHandler>();
        dictionary.Add(StockEnum.Ok,new StockResponseOkHandler());
        dictionary.Add(StockEnum.Unknown, new StockResponseUnknownHandler());
        dictionary.Add(StockEnum.X, new StockResponderXHandler());

        if(dictionary.ContainsKey(stockEnum))
        {
            IStockResponseHandler stockResponseHandler = null;
            dictionary.TryGetValue(stockEnum,out stockResponseHandler);
            stockResponseHandler.Validate(response.Sender);
        }

Nota: l'ultima istruzione invia l'ISender al rispettivo gestore

Gestore Ok e altri gestori avrebbero un aspetto simile a questo:

public class StockResponseOkHandler : IStockResponseHandler
{
    public StockResponseOkHandler()
    {
    }

    public void Validate(ISender sender)
    {
        sender.WriteSenderSpecificCode();
    }
}

Puoi scrivere un codice separato per ciascun mittente nella loro, sotto è per il mittente 1

using System;
namespace ChainReactionWithInterface
{
    public class Sender1Handler : ISender
    {
        public Sender1Handler()
        {
        }

        public void WriteSenderSpecificCode()
        {
            Console.WriteLine("Called 1");
        }
    }
}

Quindi abbiamo disaccoppiato tutti gli elementi e in qualsiasi momento possiamo aggiungere nuove condizioni e sarà scalabile. L'unico inconveniente che vedo in questa tecnica è l'inizializzazione delle classi. Ma forse ci sono altre possibilità di ottimizzarlo per il caricamento lazy o qualcosa del genere.