Ajout de complexité pour supprimer le code en double

24

J'ai plusieurs classes qui héritent toutes d'une classe de base générique. La classe de base contient une collection de plusieurs objets de type T.

Chaque classe enfant doit être capable de calculer des valeurs interpolées à partir de la collection d'objets, mais comme les classes enfants utilisent différents types, le calcul varie un tout petit peu d'une classe à l'autre.

Jusqu'à présent, j'ai copié / collé mon code de classe en classe et apporté des modifications mineures à chacune. Mais maintenant, j'essaie de supprimer le code dupliqué et de le remplacer par une méthode d'interpolation générique dans ma classe de base. Cependant, cela s'avère très difficile, et toutes les solutions auxquelles j'ai pensé semblent bien trop complexes.

Je commence à penser que le principe DRY ne s'applique pas autant dans ce genre de situation, mais cela sonne comme un blasphème. Quelle est la complexité excessive lorsque vous essayez de supprimer la duplication de code?

MODIFIER:

La meilleure solution que je peux trouver va quelque chose comme ceci:

Classe de 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 enfant A:

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 enfant B:

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);
}
Phil
la source
9
Une micro-application trop importante de concepts comme DRY et la réutilisation de code conduit à des péchés bien plus importants.
Affe
1
Vous obtenez de bonnes réponses générales. La modification pour inclure un exemple de fonction pourrait nous aider à déterminer si vous allez trop loin dans ce cas particulier.
Karl Bielefeldt
Ce n'est pas vraiment une réponse, plus d'une observation: Si vous ne pouvez pas expliquer facilement ce qu'est une classe de base factoré-out ne , il pourrait être préférable de ne pas en avoir un. Une autre façon de voir les choses est (je suppose que vous connaissez SOLID?) 'Est-ce que tout consommateur probable de cette fonctionnalité nécessite une substitution Liskov'? S'il n'y a aucune analyse de rentabilité probable pour un consommateur généralisé de fonctionnalités d'interpolation, une classe de base n'a aucune valeur.
Tom W
1
La première chose est de collecter le triplet X, Y, Z dans un type Position et d'ajouter une interpolation à ce type en tant que membre ou peut-être une méthode statique: Position interpoler (Position autre, ratio).
kevin cline

Réponses:

30

D'une certaine manière, vous avez répondu à votre propre question par cette remarque dans le dernier paragraphe:

Je commence à penser que le principe DRY ne s'applique pas autant dans ce genre de situation, mais cela sonne comme un blasphème .

Chaque fois que vous trouvez une pratique pas vraiment pratique pour résoudre votre problème, n'essayez pas d'utiliser cette pratique religieusement (le mot blasphème est une sorte d'avertissement pour cela). La plupart des cabinets ont leur pour et leur pourquoi et même s'ils couvrent 99% de tous les cas possibles, il y a toujours ce 1% où vous pourriez avoir besoin d'une approche différente.

Plus précisément, en ce qui concerne DRY , j'ai également constaté qu'il est parfois préférable d'avoir même plusieurs morceaux de code dupliqué mais simple qu'une monstruosité géante qui vous rend malade lorsque vous le regardez.

Cela étant dit, l'existence de ces cas de bord ne doit pas être utilisée comme une excuse pour un codage bâton-copier ou un manque total de modules réutilisables. Simplement, si vous ne savez pas comment écrire un code à la fois générique et lisible pour un problème dans une langue, il est probablement moins mauvais d'avoir une certaine redondance. Pensez à celui qui doit maintenir le code. Voudraient-ils plus facilement vivre avec la redondance ou l'obscurcissement?

Un conseil plus spécifique sur votre exemple particulier. Vous avez dit que ces calculs étaient similaires mais légèrement différents . Vous voudrez peut-être essayer de diviser votre formule de calcul en sous-formules plus petites, puis demander à tous vos calculs légèrement différents d'appeler ces fonctions d'assistance pour effectuer les sous-calculs. Vous éviteriez la situation où chaque calcul dépend d'un code trop généralisé et vous auriez toujours un certain niveau de réutilisation.

Goran Jovic
la source
10
Un autre point à propos de similaires mais légèrement différents est que même s'ils semblent similaires dans le code, cela ne signifie pas qu'ils doivent être similaires dans les "entreprises". Cela dépend de ce que c'est bien sûr, mais parfois c'est une bonne idée de garder les choses séparées parce que même si elles se ressemblent, elles peuvent être basées sur des décisions / exigences commerciales différentes. Donc, vous voudrez peut-être les considérer comme des calculs très différents, même s'ils peuvent être similaires au niveau du code. (Pas une règle ou quoi que ce soit, mais juste quelque chose à garder à l'esprit au moment de décider si les choses doivent être combinées ou refactorisées :)
Svish
@Svish Point intéressant. Je n'y ai jamais pensé de cette façon.
Phil
17

les classes enfants utilisent différents types, le calcul varie légèrement d'une classe à l'autre.

D'abord et avant tout: identifiez clairement quelle partie change et quelle partie ne change PAS entre les classes. Une fois que vous l'avez identifié, votre problème est résolu.

Faites-le comme premier exercice avant de commencer la refacturation. Tout le reste se mettra automatiquement en place après cela.

java_mouse
la source
2
Bien placé. Cela peut simplement être un problème de trop grande fonction répétée.
Karl Bielefeldt
8

Je crois que presque toutes les répétitions de plus de quelques lignes de code peuvent être prises en compte d'une manière ou d'une autre, et devraient presque toujours l'être.

Cependant, cette refactorisation est plus facile dans certaines langues que dans d'autres. C'est assez facile dans des langages comme LISP, Ruby, Python, Groovy, Javascript, Lua, etc. Généralement pas trop difficile en C ++ en utilisant des modèles. Plus douloureux en C, où le seul outil peut être des macros de préprocesseur. Souvent pénible en Java, et parfois tout simplement impossible, par exemple en essayant d'écrire du code générique pour gérer plusieurs types numériques intégrés.

Dans les langages plus expressifs, il ne fait aucun doute: refactoriser autre chose que quelques lignes de code. Avec des langages moins expressifs, vous devez équilibrer la douleur du refactoring avec la longueur et la stabilité du code répété. Si le code répété est long ou susceptible de changer fréquemment, j'ai tendance à refactoriser même si le code résultant est quelque peu difficile à lire.

Je n'accepterai du code répété que s'il est court, stable et que le refactoring est tout simplement trop moche. Fondamentalement, je supprime presque toute duplication, sauf si j'écris Java.

Il est impossible de donner une recommandation spécifique pour votre cas car vous n'avez pas publié le code, ou même indiqué la langue que vous utilisez.

Kevin Cline
la source
Lua n'est pas un acronyme.
DeadMG
@DeadMG: noté; n'hésitez pas à modifier. C'est pourquoi nous vous avons donné toute cette réputation.
kevin cline
5

Lorsque vous dites que la classe de base doit exécuter un algorithme, mais que l'algorithme varie pour chaque sous-classe, cela ressemble à un candidat parfait pour le modèle de modèle .

Avec cela, la classe de base exécute l'algorithme, et lorsqu'il s'agit de la variation pour chaque sous-classe, elle s'en remet à une méthode abstraite qu'il appartient à la sous-classe de mettre en œuvre. Pensez à la façon dont la page ASP.NET diffère de votre code pour implémenter

Paul T Davies
la source
4

On dirait que votre "une méthode d'interpolation générique" dans chaque classe en fait trop et devrait être transformée en méthodes plus petites.

Selon la complexité de votre calcul, pourquoi chaque "élément" du calcul ne pourrait-il pas être une méthode virtuelle comme celle-ci

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

Et remplacez les pièces individuelles lorsque vous devez faire cette "légère variation" dans la logique du calcul.

(Ceci est un exemple minuscule et inutile de la façon dont vous pourriez ajouter beaucoup de fonctionnalités tout en remplaçant les petites méthodes)

Brian
la source
3

À mon avis, vous avez raison, dans un sens, que DRY peut être poussé trop loin. Si deux morceaux de code similaires sont susceptibles d'évoluer dans des directions très différentes, vous pouvez vous poser des problèmes en essayant de ne pas vous répéter au départ.

Cependant, vous avez également tout à fait raison de vous méfier de ces pensées blasphématoires. Essayez de réfléchir sérieusement à vos options avant de décider de le laisser tranquille.

Serait-il, par exemple, préférable de mettre ce code répété dans une classe / méthode utilitaire, plutôt que dans la classe de base? Voir Préférer la composition à l'héritage .

pdr
la source
2

DRY est une directive à suivre, pas une règle incassable. À un moment donné, vous devez décider que cela ne vaut pas la peine d'avoir X niveaux d'héritage et Y modèles dans chaque classe que vous utilisez juste pour dire qu'il n'y a pas de répétition dans ce code. Quelques bonnes questions à poser seraient-elles qu'il me faudrait plus de temps pour extraire ces méthodes similaires et les mettre en œuvre comme une seule, puis il faudrait les parcourir toutes si le besoin de changement se présentait ou si leur potentiel de changement se produirait annuler mon travail en extrayant ces méthodes en premier lieu? Suis-je au point où une abstraction supplémentaire commence à faire comprendre où ou ce que fait ce code est un défi?

Si vous pouvez répondre oui à l'une de ces questions, vous avez de bonnes raisons de laisser du code potentiellement dupliqué

Ryathal
la source
0

Vous devez vous poser la question "pourquoi devrais-je la refactoriser"? Dans votre cas, lorsque vous avez un code "similaire mais différent" si vous modifiez un algorithme, vous devez vous assurer que vous reflétez également ce changement dans les autres emplacements. C'est normalement une recette pour un désastre, invariablement quelqu'un d'autre manquera un endroit et introduira un autre bug.

Dans ce cas, la refactorisation des algorithmes en un seul géant le rendra trop compliqué, le rendant trop difficile pour une maintenance future. Donc, si vous ne pouvez pas prendre en compte les éléments communs de manière sensible, un simple:

// this code is similar to class x function b

Un commentaire suffira. Problème résolu.

Rocklan
la source
0

Pour décider s'il vaut mieux avoir une méthode plus grande ou deux méthodes plus petites avec des fonctionnalités qui se chevauchent, la première question à 50000 $ est de savoir si la partie se chevauchant du comportement pourrait changer, et si un changement doit être appliqué aux méthodes plus petites également. Si la réponse à la première question est oui mais la réponse à la deuxième question est non, les méthodes doivent rester distinctes . Si la réponse aux deux questions est oui, alors quelque chose doit être fait pour s'assurer que chaque version du code reste synchronisée; dans de nombreux cas, la façon la plus simple de le faire est de n'avoir qu'une seule version.

Il y a quelques endroits où Microsoft semble être allé à l'encontre des principes DRY. Par exemple, Microsoft a explicitement déconseillé aux méthodes d'accepter un paramètre qui indiquerait si un échec doit lever une exception. S'il est vrai qu'un paramètre "failures throw exceptions" est moche dans l'API "general use" d'une méthode, ces paramètres peuvent être très utiles dans les cas où une méthode Try / Do doit être composée d'autres méthodes Try / Do. Si une méthode externe est censée lever une exception en cas d'échec, tout appel de méthode interne qui échoue doit lever une exception que la méthode externe peut laisser se propager. Si la méthode externe n'est pas censée lever une exception, la méthode interne ne l'est pas non plus. Si un paramètre est utilisé pour faire la distinction entre essayer / faire, alors la méthode externe peut la transmettre à la méthode interne. Sinon, il sera nécessaire que la méthode externe appelle les méthodes "try" lorsqu'elle est censée se comporter comme "try" et les méthodes "do" lorsqu'elle est censée se comporter comme "do".

supercat
la source