Refactoring di un metodo lungo basato su un numero elevato di casi di switch [duplicato]

16

Stiamo utilizzando Java come linguaggio di sviluppo back-end.

Un anno fa, abbiamo scritto un metodo che utilizza casi di switch basati su valori Enums. Dato che stiamo aggiungendo continuamente membri di enum e secondo i casi di aggiunta nel metodo, il metodo è cresciuto in misura molto grande. Attualmente, abbiamo circa 100 campi enumerati e il numero corrispondente di casi di switch.e.g.

 class AClass{

   enum option{ o1, o2, o3...on}

   Value method(Option o){

      switch(o){
        case o1:
          value = deriveValue(p1,p2,p3);
        case o2:
          value = deriveValue(p2,p3,p4);
        .
        .
        .
        case on:
          value = deriveValue(p1,p2,p3);
      }
   } 
 }

Quindi ogni volta che arriva un requisito aziendale, aggiungiamo enum e il caso switch corrispondente. Ora il metodo è diventato troppo lungo e sembra ingestibile, se continueremo ad aggiungere la stessa logica in futuro.

Per pulire, abbiamo pensato di sostituire il caso con polimorfismo creando classi per lo stesso, ma, di nuovo, finiremo per creare n numero di classi.

Stiamo cercando una soluzione piccola, semplice e gestibile.

----------------- Aggiornamento ---------------------

Come suggerito, per approfondire, i valori di Enum sono nomi di campi per i quali un cliente ha bisogno di un valore. Pertanto, se un cliente ha bisogno del valore di un nuovo campo, aggiungiamo il campo in enum e la corrispondente definizione di come recuperare il valore per il campo appena aggiunto, aggiungendo il caso dell'interruttore corrispondente.

Nel caso dell'interruttore, abbiamo un metodo riutilizzabile comune, ad es. deriveValue () (Si prega di fare riferimento all'esempio fornito) a cui si passano i parametri necessari per derivare il valore per il campo appena aggiunto.

    
posta Free Coder 05.01.2016 - 08:18
fonte

8 risposte

31

Che cosa stai cercando di ottenere? Hai difficoltà a comprendere e mantenere la struttura del codice che hai o stai solo cercando di seguire un'idea astratta di "buona pratica" che dice che i metodi dovrebbero essere brevi?

Mi sembra che la soluzione che hai è abbastanza adeguata per i requisiti che ti sono stati dati. Se ci sono davvero centinaia di calcoli simili che possono essere eseguiti da una singola riga di codice, quindi creare centinaia di sottoclassi con un singolo metodo di piccole dimensioni non è necessariamente migliore della semplice elencazione di tutte quelle regole parallele in un unico punto. (Il problema con i metodi lunghi è che diventeranno ripetitivi, incoerenti e quindi confusi, ma per elencare molti casi paralleli che devono essere gestiti, una lunga lista va perfettamente bene.)

    
risposta data 05.01.2016 - 08:35
fonte
18

Puoi creare una tabella. La prima colonna sarà il valore dello switch e le altre colonne saranno gli argomenti passati sul metodo deriveValue. Ad esempio

table OPTIONS
OPTION -  ARGUMENT1 -  ARGUMENT2 -  ARGUMENT3
o1          p1          p2          p3
o2          p2          p3          p4
on          p1          p2          p3

Puoi quindi scrivere una funzione generale che utilizza la tabella sopra per produrre la chiamata alla funzione appropriata in base all'opzione.

    
risposta data 05.01.2016 - 08:44
fonte
11

Potresti usare metodi astratti nell'enumerazione, quindi avere una migliore localizzazione del codice (per una migliore leggibilità e manutenzione, non parlare delle prestazioni qui).

class AClass{
    enum Option{
        o1 {
            @Override
            Value deriveValue() {
                return deriveValue(p1, p2, p3);
            }
        }, ... on {
            @Override
            Value deriveValue() {
                return deriveValue(p1, p2, p3);
            }
        };
        abstract Value deriveValue();
    }

    Value method(Option o){
        return o.deriveValue();
    }
}
    
risposta data 05.01.2016 - 11:10
fonte
1

Vale la pena di notare che, se si dispone di questo tipo di situazione in cui c'è sempre una sola funzione nel sistema che eseguirà tali controlli, e non ha alcun bisogno di essere aperta per l'estensione esterna, quindi lanciare un polimorfico la soluzione potrebbe effettivamente farti notare che i tuoi costi di manutenzione giornaliera sono effettivamente aumentati piuttosto che ridotti.

Qui, escludendo la necessità di un'estensione aperta, la differenza pratica tra polimorfico e condizionale potrebbe ridursi a se si preferisce un codice più centralizzato, instabile o un codice più stabile e decentralizzato.

In alcuni casi, un codice più stabile e decentralizzato potrebbe in effetti essere favorevole se porta a un passo-passo minore, ad es. Un codice più centralizzato e instabile in questo caso tenderebbe ad essere più breve, anche se solo come risultato di evitare lo standard associato alla definizione di nuove classi per ogni piccolo caso semplice.

Currently, we have around 100 enum fields and corresponding number of switch cases.e.g. Thus each time a business requirement comes, we add enum and corresponding switch case.

Questo mi sembra il problema se possiamo evitarlo. Non esiste un tipo di categorizzazione che possa accompagnare così tanti campi enumerati? Con il tipo di esempio ordinale che hai dato, non avresti trovato una categorizzazione significativa. Spero che sia solo uno analogico.

Se riesci a trovare qualche tipo di categorizzazione significativa per organizzare questi campi enumerati, puoi potenzialmente utilizzare più di una funzione. Ogni funzione (ancora piuttosto grossolana) può gestire un intervallo di casi, con una funzione esterna che controlla quale funzione chiamare in base all'intervallo.

Questa è una soluzione semplice e non intrusiva, purché sia possibile trovare una sorta di categorizzazione, per avere comunque una soluzione abbastanza centralizzata e semplice, ma lasciare funzioni stabili per la gamma di opzioni già gestite. Ridurrà anche l'instabilità delle implementazioni delle funzioni coinvolte, poiché il minor numero di opzioni che gestiscono, meno motivi troveranno per cambiare individualmente.

È anche possibile utilizzare potenzialmente soluzioni di tabelle di ricerca possibilmente senza un sottotipo coinvolto per tutte le 100 opzioni, che offre una posizione centralizzata da modificare, ma possibilmente con una sintassi possibilmente preferibile. Dipende dalla natura di p1 , p2 , ..., pn . Ad esempio, se sono omogenei, potresti essere in grado di utilizzare una matrice e creare una LUT basata sull'ordinamento dell'indice con i campi enum di opzione che modellano un indice nella LUT. Esempio (pseudocodice):

lut[o1] = {0, 1, 2};
lut[o2] = {1, 2, 3};
...
lut[on] = {0, 1, 2};

Value method(Option o){
    int i1 = lut[o][0];
    int i2 = lut[o][1];
    int i3 = lut[o][2];
    deriveValue(p[i1], p[i2], p[i3]);
}

Questo in realtà non è molto architetturale (sebbene sia aperto per l'estensione se si consente di aggiungere lut voci dall'esterno) ma si potrebbe preferire la sintassi. Si applica se la tua soluzione non era un'analogia remota e in realtà chiama solo deriveValue in base all'opzione con un diverso ordinamento di argomenti (nel qual caso penserei che ci sarebbero almeno più di 3 parametri in realtà da chiamare per così tante opzioni).

    
risposta data 05.01.2016 - 12:11
fonte
1

Potresti applicare la strategia generale usata per gestire grandi numeri: la struttura. Forse il numero prolifico di opzioni gestite in modo uniforme è un segno di una gerarchia troppo piccola? Forse i casi ricadono ordinatamente in gruppi che sembrano appartenere insieme (come I / O, manipolazione, computazione, ecc.)? (Perhap no - allora per favore ignora ;-)).

Ciò potrebbe riflettersi con un sistema enum gerarchico. Un enum caratterizza il gruppo, il successivo l'operazione specifica.

I gruppi potrebbero essere codificati separatamente e sarebbero entrambi più gestibili. Il fatto di averli in diversi file / classi sorgente rende anche meno probabili errori di modifica in luoghi non collegati, può velocizzare la compilazione e dovrebbe ridurre le dipendenze (dalle librerie di sistema e da altre classi personalizzate).

Naturalmente questo approccio è anche concepibile come idea progettuale per il polimorfismo: si potrebbero avere specifiche classi di opzioni / azioni derivanti da classi di gruppo derivanti da una classe di opzioni generiche, con l'opportunità di raggruppare il codice comune più avanti nella gerarchia.

    
risposta data 05.01.2016 - 17:52
fonte
1

Il passaggio alle classi non ti aiuterà. In termini di chiarezza, robustezza, compattezza, manutenibilità e prestazioni non c'è niente di meglio dell'enumerazione e di un'istruzione switch. Quello che dovresti chiedere a te stesso è se hai a che fare con i tipi. Considerando che frequentemente e continuamente aggiungi enumerazioni, mi sembra che tu abbia a che fare con dati, non con tipi. Potrebbe essere necessaria una singola classe e più istanze. Questo è ciò che DesignerAnalyst implica con la proposta di tabella. Puoi ancora codificare i tuoi dati di input per le tue istanze di oggetti o dare uno schiaffo all'input in un file di testo, la grande domanda rimane: quali sono questi valori di enum in realtà? Come si classificano?

    
risposta data 05.01.2016 - 19:38
fonte
0

Aggiungere più e più classi non è davvero un problema, se sono parti di una struttura ben definita che ti permette di ignorarle quando vuoi: basta metterle in un modulo e accedere al modulo solo quando devi modificalo.

IMHO, ogni opzione rappresenta un modo per calcolare un valore, quindi in OO ogni opinione dovrebbe essere rappresentata da un oggetto con un'implementazione specifica di deriveValue (). In Java, ciò significa che ogni oggetto deve avere una classe specifica che definisca l'implementazione. Quindi il tuo codice diventa qualcosa del tipo:

Nel pacchetto delle opzioni:

public interface Option {
     Value deriveValue(? p1, ? p2, ? p3);
}

public FirstOption implements Option {
    Value deriveValue(? p1, ? p2, ? p3){ ... } 
}

In un altro pacchetto;

class AClass{
   private Option o;

   Value method(){ o.deriveValue(p1,p2,p3); }
}

Se hai comportamenti diversi da specificare, ognuno di essi può essere rappresentato da una singola chiamata anziché da un grosso interruttore. Avvertenza: ogni volta che si desidera aggiungere un nuovo comportamento, è necessario aprire ciascuna classe di opzioni per aggiungere un nuovo metodo.

    
risposta data 05.01.2016 - 11:34
fonte
-3

Hai sempre più opzioni, che portano a un numero sempre maggiore di casi enumerati ea un numero sempre maggiore di codice che implementa qualcosa per ciascuna opzione. Puoi provare a fare qualcosa sul crescente numero di opzioni - beh, forse non puoi.

Se non riesci a impedire che il numero di opzioni aumenti, allora un'istruzione switch vale quanto qualsiasi altro metodo per scegliere tra le diverse opzioni. E sembra che sia necessario pochissimo codice aggiuntivo oltre al codice reale per valutare ciascuna opzione. Un compilatore decente ti dirà dei casi mancanti.

Se non ti piace questo consiglio: un giorno raggiungi il punto in cui ti preoccupi del codice semplice e gestibile e non delle violazioni di un principio astratto che hai appena sentito nel tuo ultimo corso. Le regole sono lì per essere infrante se non sei un novizio. Ora se invece di "questa è una violazione del principio aperto-chiuso" potresti effettivamente dare un esempio pratico ...

    
risposta data 05.01.2016 - 10:01
fonte