Zwiększanie złożoności w celu usunięcia duplikatu kodu


24

Mam kilka klas, które wszystkie dziedziczą od ogólnej klasy podstawowej. Klasa podstawowa zawiera zbiór kilku obiektów typu T.

Każda klasa potomna musi być w stanie obliczyć interpolowane wartości ze zbioru obiektów, ale ponieważ klasy potomne używają różnych typów, obliczenia różnią się nieznacznie w zależności od klasy.

Do tej pory kopiowałem / wklejałem swój kod z klasy do klasy i wprowadzałem niewielkie zmiany w każdym z nich. Ale teraz próbuję usunąć zduplikowany kod i zastąpić go jedną ogólną metodą interpolacji w mojej klasie bazowej. Jest to jednak bardzo trudne, a wszystkie rozwiązania, o których myślałem, wydają się zbyt skomplikowane.

Zaczynam myśleć, że zasada DRY nie stosuje się tak często w tego rodzaju sytuacjach, ale to brzmi jak bluźnierstwo. Jaka złożoność jest zbyt duża, gdy próbujesz usunąć duplikację kodu?

EDYTOWAĆ:

Najlepsze rozwiązanie, jakie mogę wymyślić, to coś takiego:

Klasa podstawowa:

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);

Klasa dziecka 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);
}

Klasa dziecka 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);
}

9
Nadmiernie mikro-stosowanie takich pojęć, jak DRY i Code Reuse prowadzi do znacznie większych grzechów.
Affe

1
Dostajesz kilka dobrych ogólnych odpowiedzi. Edycja w celu włączenia przykładowej funkcji może pomóc nam ustalić, czy w tym konkretnym przypadku nie posuwasz się za daleko.
Karl Bielefeldt

To nie jest tak naprawdę odpowiedź, a raczej spostrzeżenie: jeśli nie możesz łatwo wyjaśnić, co robi wyodrębniona klasa podstawowa , być może najlepiej jej nie mieć. Innym sposobem spojrzenia na to jest (zakładam, że znasz SOLID?) „Czy jakiś prawdopodobny konsument tej funkcjonalności wymaga podstawienia Liskowa”? Jeśli nie ma prawdopodobnego uzasadnienia biznesowego dla uogólnionego konsumenta funkcji interpolacji, klasa podstawowa nie ma wartości.
Tom W

1
Pierwszą rzeczą jest zebranie tripletu X, Y, Z w typ pozycji i dodanie interpolacji do tego typu jako elementu lub metody statycznej: interpolacja pozycji (pozycja inna, współczynnik).
kevin cline

Odpowiedzi:


30

W pewnym sensie odpowiedziałeś na swoje pytanie, używając uwagi w ostatnim akapicie:

Zaczynam myśleć, że zasada DRY nie stosuje się tak często w tego rodzaju sytuacjach, ale brzmi to jak bluźnierstwo .

Ilekroć okaże się, że jakaś praktyka nie jest tak naprawdę praktyczna do rozwiązania problemu, nie próbuj jej używać religijnie (słowo bluźnierstwo jest tego rodzaju ostrzeżeniem). Większość praktyk ma swój czas i dlaczego, a nawet jeśli obejmują one 99% wszystkich możliwych przypadków, nadal jest 1%, w którym możesz potrzebować innego podejścia.

W szczególności, w odniesieniu do DRY , odkryłem również, że czasami lepiej jest nawet mieć kilka kawałków zduplikowanego, ale prostego kodu, niż jedną gigantyczną potworność, która sprawia, że ​​czujesz się niedobrze, kiedy na nią patrzysz.

To powiedziawszy, istnienie tych przypadków brzegowych nie powinno być wykorzystywane jako usprawiedliwienie dla niechlujnego kopiowania i wklejania kodu lub całkowitego braku modułów wielokrotnego użytku. Po prostu, jeśli nie masz pojęcia, jak napisać zarówno ogólny, jak i czytelny kod dla jakiegoś problemu w jakimś języku, prawdopodobnie mniej źle jest mieć nadmiarowość. Pomyśl o tym, kto musi utrzymywać kod. Czy łatwiej by im było żyć ze zwolnieniem lub zaciemnieniem?

Bardziej szczegółowe porady dotyczące Twojego konkretnego przykładu. Powiedziałeś, że te obliczenia były podobne, ale nieco inne . Możesz spróbować rozbić formułę obliczeniową na mniejsze podformuły, a następnie sprawić, by wszystkie nieco inne obliczenia wywoływały te funkcje pomocnicze w celu wykonania obliczeń podrzędnych. Unikniesz sytuacji, w której każde obliczenie zależy od jakiegoś nadmiernie uogólnionego kodu i nadal będziesz miał pewien poziom ponownego wykorzystania.


10
Kolejną kwestią dotyczącą podobnych, ale nieco innych jest to, że chociaż wyglądają podobnie w kodzie, nie oznacza to, że muszą być podobne w „biznesie”. Zależy oczywiście od tego, co to jest, ale czasem dobrym pomysłem jest oddzielenie rzeczy, ponieważ chociaż wyglądają tak samo, mogą opierać się na różnych decyzjach / wymaganiach biznesowych. Możesz więc spojrzeć na nie jako na bardzo różne obliczenia, nawet jeśli są mądre pod względem kodu, mogą wyglądać podobnie. (Nie jest to
żadna

@Svish Interesujący punkt. Nigdy o tym nie myślałem.
Phil

17

klasy potomne używają różnych typów, obliczenia różnią się nieco w zależności od klasy.

Przede wszystkim najważniejsze: po pierwsze jasno określ, która część się zmienia, a która NIE zmienia się między klasami. Po zidentyfikowaniu problemu problem zostanie rozwiązany.

Zrób to jako pierwsze ćwiczenie przed ponownym faktoringiem. Po tym wszystko pozostanie na swoim miejscu.


2
Dobrze wyłożone. Może to być po prostu problem zbyt dużej powtarzanej funkcji.
Karl Bielefeldt

8

Uważam, że prawie wszystkie powtórzenia więcej niż kilku linii kodu można rozróżnić w taki czy inny sposób i prawie zawsze tak powinno być.

Jednak refaktoryzacja jest łatwiejsza w niektórych językach niż w innych. Jest to dość łatwe w językach takich jak LISP, Ruby, Python, Groovy, JavaScript, Lua itp. Zwykle nie jest to zbyt trudne w C ++ przy użyciu szablonów. Bardziej bolesne w C, gdzie jedynym narzędziem mogą być makra preprocesora. Często bolesne w Javie, a czasem po prostu niemożliwe, np. Próba napisania ogólnego kodu do obsługi wielu wbudowanych typów numerycznych.

W bardziej wyrazistych językach nie ma pytania: refaktoryzuj cokolwiek więcej niż kilka linii kodu. W mniej wyrazistych językach musisz zrównoważyć ból związany z refaktoryzacją z długością i stabilnością powtarzanego kodu. Jeśli powtarzany kod jest długi lub może się często zmieniać, mam tendencję do refaktoryzacji, nawet jeśli wynikowy kod jest nieco trudny do odczytania.

Przyjmę powtarzający się kod tylko wtedy, gdy jest krótki, stabilny, a refaktoryzacja jest po prostu zbyt brzydka. Zasadniczo wykluczam prawie wszystkie duplikacje, chyba że piszę w Javie.

Nie można podać konkretnej rekomendacji dla Twojej sprawy, ponieważ nie opublikowałeś kodu ani nawet nie wskazałeś, jakiego języka używasz.


Lua nie jest akronimem.
DeadMG

@DeadMG: odnotowano; nie krępuj się edytować. Właśnie dlatego daliśmy ci całą tę reputację.
kevin cline

5

Kiedy mówisz, że klasa podstawowa musi wykonać algorytm, ale algorytm różni się dla każdej podklasy, brzmi to jak idealny kandydat na wzorzec szablonu .

Dzięki temu klasa podstawowa wykonuje algorytm, a jeśli chodzi o odmianę dla każdej podklasy, odsyła do metody abstrakcyjnej, za którą wdrożenie jest odpowiedzialna podklasa. Pomyśl o tym, jak strona i ASP.NET odkładają Twój kod na przykład na Page_Load.


4

Wygląda na to, że „jedna ogólna metoda interpolacji” w każdej klasie robi zbyt wiele i należy ją przerobić na mniejsze metody.

W zależności od złożoności obliczeń, dlaczego każdy „element” obliczeń nie może być metodą wirtualną jak ta

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

I nadpisuj poszczególne elementy, gdy trzeba wprowadzić tę „niewielką zmianę” w logice obliczeń.

(Jest to bardzo mały i bezużyteczny przykład tego, jak można dodać wiele funkcji, zastępując małe metody)


3

Moim zdaniem masz rację, w pewnym sensie, że DRY można posunąć za daleko. Jeśli dwa podobne fragmenty kodu mogą ewoluować w bardzo różnych kierunkach, możesz spowodować problemy, próbując początkowo się nie powtarzać.

Masz jednak również całkowitą ostrożność wobec takich bluźnierczych myśli. Bardzo staraj się przemyśleć opcje, zanim zdecydujesz się zostawić to w spokoju.

Czy na przykład lepiej byłoby umieścić ten powtarzający się kod w klasie / metodzie użytkowej niż w klasie podstawowej? Zobacz Preferuj skład zamiast dziedziczenia .


2

DRY jest wytyczną, której należy przestrzegać, a nie niezłomną zasadą. W pewnym momencie musisz zdecydować, że nie warto mieć X poziomów dziedziczenia i szablonów Y w każdej klasie, której używasz, aby powiedzieć, że w tym kodzie nie ma powtórzeń. Kilka dobrych pytań, które należy zadać, to czy zajmie mi więcej czasu, aby wyodrębnić te podobne metody i wdrożyć je jako jedną, a następnie przeszuka je wszystkie, jeśli zajdzie potrzeba zmiany, lub jest to ich potencjał do zmiany, która nastąpiłaby czy w pierwszej kolejności cofnąć moją pracę przy wydobywaniu tych metod? Czy jestem w punkcie, w którym dodatkowe abstrakcje zaczynają rozumieć, gdzie lub co ten kod stanowi wyzwanie?

Jeśli potrafisz odpowiedzieć „tak” na którekolwiek z tych pytań, masz mocne podstawy do pozostawienia potencjalnie powielonego kodu


0

Musisz zadać sobie pytanie: „dlaczego mam to refaktoryzować”? W przypadku, gdy masz „podobny, ale inny” kod, jeśli dokonujesz zmiany w jednym algorytmie, musisz upewnić się, że odzwierciedlasz również tę zmianę w innych miejscach. Zwykle jest to przepis na katastrofę, niezmiennie ktoś inny przegapi jedno miejsce i wprowadzi kolejny błąd.

W takim przypadku refaktoryzacja algorytmów w jeden gigantyczny sprawi, że będzie to zbyt skomplikowane, co utrudni przyszłą konserwację. Jeśli więc nie możesz rozsądnie rozróżnić typowych rzeczy, proste:

// this code is similar to class x function b

Komentarz wystarczy. Problem rozwiązany.


0

Podejmując decyzję, czy lepiej mieć jedną większą metodę, czy dwie mniejsze metody z nakładającymi się funkcjami, pierwsze 50 000 USD pytanie dotyczy tego, czy nakładająca się część zachowania może się zmienić i czy jakakolwiek zmiana powinna być zastosowana w równym stopniu do mniejszych metod. Jeśli odpowiedź na pierwsze pytanie brzmi „tak”, ale odpowiedź na drugie pytanie brzmi „nie”, wówczas metody powinny pozostać odrębne . Jeśli odpowiedź na oba pytania brzmi „tak”, należy coś zrobić, aby każda wersja kodu pozostała zsynchronizowana; w wielu przypadkach najłatwiejszym sposobem jest posiadanie tylko jednej wersji.

Istnieje kilka miejsc, w których Microsoft wydaje się być niezgodny z zasadami DRY. Na przykład Microsoft wyraźnie odradza, aby metody akceptowały parametr, który wskazywałby, czy błąd powinien zgłosić wyjątek. Chociaż prawdą jest, że parametr „zgłaszania wyjątków w przypadku awarii” jest brzydki w interfejsie API „ogólnego zastosowania” metody, takie parametry mogą być bardzo pomocne w przypadkach, w których metoda Try / Do musi składać się z innych metod Try / Do. Jeśli metoda zewnętrzna ma generować wyjątek, gdy wystąpi awaria, wówczas każde wywołanie metody wewnętrznej, które się nie powiedzie, powinno zgłosić wyjątek, który metoda zewnętrzna może rozpowszechnić. Jeśli metoda zewnętrzna nie powinna generować wyjątku, to wewnętrzna też nie jest. Jeśli parametr służy do rozróżnienia między try / do, wtedy metoda zewnętrzna może przekazać ją do metody wewnętrznej. W przeciwnym razie konieczne będzie wywołanie przez metodę zewnętrzną metod „try”, gdy ma ona zachowywać się jak „try”, oraz metod „do”, gdy ma zachowywać się jak „do”.

Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.