Interpretazione del principio ASCIUTTO

10

In questo momento sto lottando con questo concetto di DRY (Do not Repeat Yourself) nella mia codifica. Sto creando questa funzione in cui temo che stia diventando troppo complessa, ma sto cercando di seguire il principio DRY.

createTrajectoryFromPoint(A a,B b,C c,boolean doesSomething,boolean doesSomething2)

Questa funzione ho detto prende 3 parametri di input, e quindi la funzione farà qualcosa di leggermente diverso date le combinazioni booleane doesSomething e doesSomething2 . Comunque il problema che sto avendo è che questa funzione sta crescendo in complessità enormemente con ogni nuovo parametro booleano che viene aggiunto.

Quindi la mia domanda è, è meglio avere un sacco di funzioni diverse che condividono molte delle stesse logiche (quindi violando il principio di DRY) o una funzione che si comporta in modo leggermente diverso a causa di un certo numero di parametri ma rendendola molto più complesso (ma preservando DRY)?

    
posta Albinoswordfish 17.05.2012 - 19:33
fonte

6 risposte

19

Gli argomenti booleani per attivare percorsi di codice diversi in una singola funzione / metodo sono cattivo odore di codice .

Quello che stai facendo viola Loose Coupling e High Cohesion e Single Responsibility principi, che sono molto più importante di DRY in precedenza.

Ciò significa che le cose devono dipendere da altre cose solo quando devono ( Coupling ) e che dovrebbero fai una cosa e solo una cosa (molto bene) ( Cohesion ).

Per tua omissione, questo è troppo strettamente accoppiato (tutte le bandiere booleane sono un tipo di dipendenza da stato, che è una delle peggiori!) e ha troppe responsabilità individuali mescolate (eccessivamente complesse).

Ciò che stai facendo è not nello spirito di DRY comunque. DRY è più sulla ripetizione (il R sta per REPEAT ). Evitare copia e incolla è la sua forma più elementare. Quello che stai facendo non è legato alla ripetizione.

Il tuo problema è che la decomposizione del tuo codice non è al livello corretto. Se pensi di avere un codice duplicato, allora dovrebbe essere la sua funzione / metodo che è parametrizzata, non copiata e incollata, e gli altri dovrebbero essere nominati in modo descrittivo e delegare alla funzione / metodo core.

    
risposta data 17.05.2012 - 19:37
fonte
3

Il fatto che tu stia passando in booleani per far sì che la funzione faccia cose diverse è una violazione del Principio di Responsabilità Unica. Una funzione dovrebbe fare una cosa. Dovrebbe fare solo una cosa, e dovrebbe farlo bene.

Ha l'odore di cui hai bisogno per dividerlo in diverse funzioni più piccole con nomi descrittivi, separando i percorsi di codice corrispondenti ai valori di quei booleani.

Una volta fatto, dovresti cercare il codice comune nelle funzioni risultanti e scomporlo nelle sue proprie funzioni. A seconda di quanto sia complessa questa cosa, potresti persino essere in grado di calcolare una o due classi.

Ovviamente, questo presuppone che tu stia utilizzando un sistema di controllo della versione e che tu abbia una buona suite di test, in modo che tu possa refactoring senza timore di rompere qualcosa.

    
risposta data 17.05.2012 - 19:45
fonte
3

Perché non crei un'altra funzione contenente tutta la logica della tua funzione prima di decidere di fare qualcosa o qualcosa2 e poi avere tre funzioni come:

createTrajectoryFromPoint(A a,B b,C c){...}

dosomething(A a, B b, C c){...}

dosomething2(A a, B b, C c){...}

E ora passando tre tipi di parametri uguali a tre funzioni diverse, ripeterai te stesso, quindi dovresti definire una struct o una classe contenente A, B, C.

In alternativa puoi creare una classe contenente i parametri A, B, C e un elenco di operazioni da eseguire. Aggiungi quali operazioni (qualcosa, qualcosa2) vuoi che succedano con questi parametri (A, B, C) registrando le operazioni con l'oggetto. Quindi hai un metodo per chiamare tutte le operazioni registrate sul tuo oggetto.

public class MyComplexType
{
    public A a{get;set;}
    public B b{get;set;}
    public C c{get;set;}

    public delegate void Operation(A a, B b, C c);
    public List<Operation> Operations{get;set;}

    public MyComplexType(A a, B b, C c)
    {
        this.a = a;
        this.b = b;
        this.c = c   
        Operations = new List<Operation>();
    }

    public CallMyOperations()
    {
        foreach(var operation in Operations)
        {
            operation(a,b,c);
        }
    }
}
    
risposta data 17.05.2012 - 19:44
fonte
2

Il DRY può essere preso troppo lontano, è meglio usare il principio di responsabilità singola (SRP) insieme a DRY. L'aggiunta di flag bool ad una funzione per far sì che faccia versioni leggermente diverse dello stesso codice può essere un segno che stai facendo troppo con una funzione. In questo caso suggerirei di creare una funzione separata per ogni caso rappresentata dai tuoi flag, quindi quando hai scritto ciascuna funzione dovrebbe essere abbastanza evidente se c'è una sezione comune che può essere spostata in una funzione privata senza passare tutte le flag , se non c'è una sezione apparente di codice, allora non stai davvero ripetendo te stesso, hai diversi casi diversi ma simili.

    
risposta data 17.05.2012 - 19:42
fonte
1

Di solito passo attraverso diversi passaggi con questo problema, fermandomi quando non riesco a capire come andare oltre.

Innanzitutto, fai ciò che hai fatto. Vai duro con ASCIUTTO. Se non finisci con un grosso pasticcio peloso, hai finito. Se, come nel tuo caso, non hai codice duplicato ma ogni booleano ha il suo valore verificato in 20 posizioni diverse, vai al passaggio successivo.

Secondo, dividi il codice in blocchi. I booleani sono tutti referenziati una sola volta (beh, forse due volte a volte) per dirigere l'esecuzione nel blocco di destra. Con due booleani, si finisce con quattro blocchi. Ogni blocco è quasi identico. ASCIUTTO è andato. Non rendere ciascun blocco un metodo separato. Sarebbe più elegante, ma inserire tutto il codice in un unico metodo rende più facile, o persino possibile, per chiunque stia facendo manutenzione per vedere che devono fare ogni cambiamento in quattro punti. Con un codice ben organizzato e un monitor alto, le differenze e gli errori saranno quasi ovvi. Ora disponi di codice e manutenibile più veloce rispetto al pasticcio originale ingarbugliato.

Terzo, prova ad afferrare linee di codice duplicate da ognuno dei tuoi blocchi e trasformali in semplici e semplici metodi. A volte non puoi fare nulla. A volte non puoi fare molto. Ma ogni cosa che fai ti riporta indietro verso DRY e rende il codice un po 'più facile da seguire e più sicuro da mantenere. Idealmente, il tuo metodo originale potrebbe non avere codice duplicato. A quel punto, potresti voler dividerlo in diversi metodi senza i parametri booleani o potresti non farlo. La convenienza del codice chiamante è ora la preoccupazione principale.

Ho aggiunto la mia risposta al gran numero già qui a causa del secondo passaggio. Odio il codice duplicato, ma se è l'unico modo intelligibile per risolvere un problema, fallo in modo che chiunque possa sapere a colpo d'occhio cosa stai facendo. Utilizza più blocchi e un solo metodo. Rendi i blocchi il più identici possibile nei nomi, spaziatura, allineamenti, ... tutto. Le differenze dovrebbero poi saltare al lettore. Potrebbe rendere ovvio come riscriverlo in modo DRY e, in caso contrario, mantenerlo sarà ragionevolmente semplice.

    
risposta data 17.05.2012 - 21:28
fonte
0

Un approccio alternativo consiste nel sostituire i parametri booleani con i parametri dell'interfaccia, con il codice per gestire i diversi valori booleani rifatti nelle implementazioni dell'interfaccia. Quindi avresti

createTrajectoryFromPoint(A a,B b,C c,IX x,IY y)

dove hai implementazioni di IX e IY che rappresentano i diversi valori per i booleani. Nel corpo della funzione, ovunque tu abbia

if (doesSomething)
{
     ...
}
else
{
     ...
}

lo sostituisci con una chiamata a un metodo definito su IX, con le implementazioni contenenti i blocchi di codice omessi.

    
risposta data 17.05.2012 - 20:05
fonte

Leggi altre domande sui tag