Aggiunta di complessità per rimuovere il codice duplicato

24

Ho diverse classi che ereditano tutte da una classe base generica. La classe base contiene una raccolta di diversi oggetti di tipo T .

Ogni classe figlia deve essere in grado di calcolare i valori interpolati dalla collezione di oggetti, ma poiché le classi figlie usano tipi diversi, il calcolo varia leggermente da una classe all'altra.

Finora ho copiato / incollato il mio codice da una classe all'altra e ho apportato lievi modifiche a ciascuna di esse. Ma ora sto cercando di rimuovere il codice duplicato e sostituirlo con un metodo di interpolazione generico nella mia classe base. Tuttavia ciò si sta dimostrando molto difficile e tutte le soluzioni che ho pensato sembrano troppo complesse.

Sto iniziando a pensare che il principio ASCIUTTO non si applica tanto in questo tipo di situazione, ma sembra una bestemmia. Quanta complessità è troppo quando si tenta di rimuovere la duplicazione del codice?

EDIT:

La soluzione migliore che riesco a fare è qualcosa di simile a questo:

Classe base:

protected T GetInterpolated(int frame)
{
    var index = SortedFrames.BinarySearch(frame);
    if (index >= 0)
        return Data[index];

    index = ~index;

    if (index == 0)
        return Data[index];
    if (index >= Data.Count)
        return Data[Data.Count - 1];

    return GetInterpolatedItem(frame, Data[index - 1], Data[index]);
}

protected abstract T GetInterpolatedItem(int frame, T lower, T upper);

Classe A bambino:

public IGpsCoordinate GetInterpolatedCoord(int frame)
{
    ReadData();
    return GetInterpolated(frame);
}

protected override IGpsCoordinate GetInterpolatedItem(int frame, IGpsCoordinate lower, IGpsCoordinate upper)
{
    double ratio = GetInterpolationRatio(frame, lower.Frame, upper.Frame);

    var x = GetInterpolatedValue(lower.X, upper.X, ratio);
    var y = GetInterpolatedValue(lower.Y, upper.Y, ratio);
    var z = GetInterpolatedValue(lower.Z, upper.Z, ratio);

    return new GpsCoordinate(frame, x, y, z);
}

Classe B bambino:

public double GetMph(int frame)
{
    ReadData();
    return GetInterpolated(frame).MilesPerHour;
}

protected override ISpeed GetInterpolatedItem(int frame, ISpeed lower, ISpeed upper)
{
    var ratio = GetInterpolationRatio(frame, lower.Frame, upper.Frame);
    var mph = GetInterpolatedValue(lower.MilesPerHour, upper.MilesPerHour, ratio);
    return new Speed(frame, mph);
}
    
posta Phil 02.02.2012 - 21:45
fonte

9 risposte

30

In un certo senso, hai risposto alla tua stessa domanda con quell'osservazione nell'ultimo paragrafo:

I am starting to think the DRY principle does not apply as much in this kind of situation, but that sounds like blasphemy.

Ogni volta che trovi qualche pratica non molto pratica per risolvere il tuo problema, non provare a usare quella pratica religiosamente (la parola blasphemy è una sorta di avvertimento per questo). La maggior parte delle pratiche ha i loro whens e perché e anche se coprono il 99% di tutti i possibili casi, c'è ancora quell'1% in cui potrebbe essere necessario un approccio diverso.

In particolare, per quanto riguarda DRY , ho anche scoperto che a volte è meglio persino avere diversi pezzi del codice duplicato ma semplice di una mostruosità gigante che ti fa sentire male quando lo guardi.

Detto questo, l'esistenza di questi casi limite non dovrebbe essere usata come scusa per la copia scansa e codifica di incolla o per la completa mancanza di moduli riutilizzabili. Semplicemente, se non hai idea di come scrivere un codice sia generico che leggibile per qualche problema in qualche lingua, allora probabilmente meno male ha una certa ridondanza. Pensa a chi deve mantenere il codice. Vivrebbero più facilmente con la ridondanza o l'offuscamento?

Un consiglio più specifico sul tuo particolare esempio. Hai detto che questi calcoli erano simili ancora leggermente diversi . Potresti provare a suddividere la tua formula di calcolo in sottoforme più piccole e poi fare in modo che tutti i tuoi calcoli leggermente diversi chiamino queste funzioni di supporto per eseguire i calcoli secondari. Eviteresti la situazione in cui ogni calcolo dipende da un codice troppo generalizzato e avresti ancora un certo livello di riutilizzo.

    
risposta data 02.02.2012 - 22:02
fonte
17

child classes use different types, the calculation varies a tiny bit from class to class.

Prima di tutto è la seguente: innanzitutto identificare chiaramente quale parte sta cambiando e quale parte NON sta cambiando tra le classi. Una volta identificato, il problema è risolto.

Fatelo come primo esercizio prima di iniziare il ri-factoring. Tutto il resto andrà a posto automaticamente dopo quello.

    
risposta data 02.02.2012 - 22:17
fonte
8

Credo che quasi tutte le ripetizioni di più di un paio di righe di codice possano essere scomposte in un modo o nell'altro, e quasi sempre dovrebbero esserlo.

Tuttavia, questo refactoring è più semplice in alcune lingue rispetto ad altri. È abbastanza facile in linguaggi come LISP, Ruby, Python, Groovy, Javascript, Lua, ecc. Di solito non è troppo difficile in C ++ usando i template. Più doloroso in C, dove l'unico strumento può essere macro preprocessore. Spesso doloroso in Java, e talvolta semplicemente impossibile, ad es. provare a scrivere codice generico per gestire più tipi numerici incorporati.

Nelle lingue più espressive, non c'è dubbio: refactatore qualcosa di più di un paio di righe di codice. Con linguaggi meno espressivi devi bilanciare il dolore del refactoring con la lunghezza e la stabilità del codice ripetuto. Se il codice ripetuto è lungo, o soggetto a modifiche frequenti, tendo a rifattorici anche se il codice risultante è alquanto difficile da leggere.

Accetterò codice ripetuto solo se è breve, stabile e il refactoring è troppo brutto. Fondamentalmente ritengo quasi tutte le duplicazioni a meno che non stia scrivendo Java.

È impossibile dare una raccomandazione specifica per il tuo caso perché non hai pubblicato il codice, o anche indicato quale lingua stai usando.

    
risposta data 02.02.2012 - 22:52
fonte
5

Quando dici che la classe base deve eseguire un algoritmo, ma l'algoritmo varia per ogni sottoclasse, questo sembra un candidato perfetto per Il modello del modello .

Con questo, la classe base esegue l'algoritmo e quando arriva la variazione per ogni sottoclasse, rimanda a un metodo astratto che è responsabilità della sottoclasse implementare. Pensa al modo in cui la pagina ASP.NET definisce il tuo codice per implementare Page_Load, ad esempio.

    
risposta data 02.02.2012 - 22:00
fonte
4

Sembra che il tuo "metodo di interpolazione generico" in ogni classe stia facendo troppo e dovrebbe essere trasformato in metodi più piccoli.

A seconda di quanto sia complesso il calcolo, perché non è possibile che ogni "pezzo" del calcolo sia un metodo virtuale come questo

Public Class Fraction
{
     public virtual Decimal GetNumerator(params?)
     public virtual Decimal GetDenominator(params?)
     //Some concrete method to actually compute GetNumerator / GetDenominator
}

Esegui l'override dei singoli pezzi quando devi fare quella "leggera variazione" nella logica del calcolo.

(Questo è un esempio molto piccolo e inutile di come potresti aggiungere molte funzionalità ignorando i piccoli metodi)

    
risposta data 02.02.2012 - 22:03
fonte
3

Secondo me, hai ragione, in un certo senso, che DRY può essere preso troppo lontano. Se due pezzi di codice simili si evolveranno in direzioni molto diverse, allora potresti causare dei problemi cercando di non ripetere te stesso inizialmente.

Tuttavia, hai anche ragione di diffidare di tali pensieri blasfemi. Prova molto a pensare alle tue opzioni prima di decidere di lasciar perdere.

Sarebbe, ad esempio, meglio mettere quel codice ripetuto in una classe di utilità / metodo, piuttosto che nella classe base? Vedi Preferisci la composizione sull'ereditarietà .

    
risposta data 02.02.2012 - 22:01
fonte
2

DRY è una linea guida da seguire, non una regola infrangibile. Ad un certo punto devi decidere che non vale la pena avere i livelli X di ereditarietà e i modelli Y in ogni classe che stai usando solo per dire che non c'è ripetizione in questo codice. Un paio di buone domande da porsi sarebbe che mi ci vorrà più tempo per estrarre questi metodi simili e implementarli come se fossero uno solo, quindi cercherei tutti attraverso di essi se dovesse sorgere la necessità di cambiare o se il loro potenziale per un cambiamento dovesse verificarsi annullare il mio lavoro estraendo questi metodi in primo luogo? Sono al punto in cui l'astrazione addizionale sta iniziando a capire dove o cosa fa questo codice è una sfida?

Se puoi rispondere di si a una di queste domande, hai un strong motivo per lasciare codice potenzialmente duplicato

    
risposta data 02.02.2012 - 22:13
fonte
0

Devi porsi la domanda, "perché dovrei rifattarla"? Nel tuo caso, quando hai un codice "simile ma diverso" se apporti una modifica a un algoritmo, devi assicurarti di riflettere anche quel cambiamento negli altri punti. Questa è normalmente una ricetta per il disastro, invariabilmente qualcun altro perderà un punto e introdurrà un altro bug.

In questo caso, il refactoring degli algoritmi in uno gigante lo renderà troppo complicato, rendendolo troppo difficile per la manutenzione futura. Quindi, se non riesci a calcolare la roba comune in modo sensato, un semplice:

// this code is similar to class x function b

Il commento sarà sufficiente. Problema risolto.

    
risposta data 29.03.2012 - 14:52
fonte
0

Nel decidere se sia meglio avere un metodo più grande o due metodi più piccoli con funzionalità che si sovrappongono, la prima domanda da $ 50.000 è se la parte sovrapposta del comportamento potrebbe cambiare e se qualsiasi modifica debba essere applicata ugualmente ai metodi più piccoli. Se la risposta alla prima domanda è sì ma la risposta alla seconda domanda è no, allora i metodi dovrebbero rimanere separati . Se la risposta ad entrambe le domande è sì, allora deve essere fatto qualcosa per assicurare che ogni versione del codice rimanga sincronizzata; in molti casi, il modo più semplice per farlo è avere solo una versione.

Ci sono alcuni punti in cui Microsoft sembra essere andata contro i principi di DRY. Ad esempio, Microsoft ha esplicitamente sconsigliato che i metodi accettino un parametro che indicherebbe se un errore dovrebbe generare un'eccezione. Mentre è vero che un parametro "failures throw exception" è brutto nell'API "general use" di un metodo, tali parametri possono essere molto utili nei casi in cui un metodo Try / Do deve essere composto da altri metodi Try / Do. Se si suppone che un metodo esterno lanci un'eccezione quando si verifica un errore, qualsiasi chiamata al metodo interno che non riesce dovrebbe generare un'eccezione che il metodo esterno può consentire la propagazione. Se non si suppone che il metodo esterno lanci un'eccezione, allora nemmeno quella interiore. Se un parametro viene utilizzato per distinguere tra try / do, il metodo esterno può passarlo al metodo interno. Altrimenti, sarà necessario che il metodo esterno chiami i metodi "try" quando si suppone che si comportino come metodi "try" e "do" quando si suppone che si comportino come "do".

    
risposta data 20.09.2014 - 19:40
fonte

Leggi altre domande sui tag