Interpretacja zasady OSUSZANIA

10

W tej chwili mam problem z koncepcją DRY (Don't Repeat Yourself) w moim kodowaniu. Tworzę tę funkcję, w której obawiam się, że stanie się zbyt skomplikowana, ale staram się przestrzegać zasady OSUSZANIA.

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

Ta funkcja, o której mówię, przyjmuje 3 parametry wejściowe, a następnie funkcja zrobi coś nieco innego, biorąc pod uwagę kombinacje logiczne doesSomethingi doesSomething2. Jednak mam problem z tym, że ta funkcja znacznie się komplikuje z każdym dodanym nowym parametrem boolowskim.

Więc moje pytanie brzmi: czy lepiej mieć kilka różnych funkcji, które mają wiele takich samych zasad (w związku z tym naruszając zasadę DRY) lub jedną funkcję, która zachowuje się nieco inaczej, biorąc pod uwagę szereg parametrów, ale czyniąc ją znacznie bardziej złożoną (ale zachowując SUCHO)?

Albinoswordfish
źródło
3
Czy logikę wspólną / wspólną można podzielić na funkcje prywatne, które createTrajectory...wywołują różne funkcje publiczne ?
FrustratedWithFormsDesigner
Możliwe, że te prywatne funkcje nadal będą musiały uzyskać parametry boolowskie
Albinoswordfish
2
Myślę, że byłoby / będzie o wiele łatwiej odpowiedzieć, biorąc pod uwagę konkretny przykład. Moją natychmiastową reakcją jest to, że dychotomia, którą przedstawiasz, nie jest całkowicie prawdziwa - tj. Nie są to jedyne dwie możliwości. Nawiasem mówiąc, rozważę użycie jakiegokolwiek booleanparametru jako co najwyżej podejrzanego.
Jerry Coffin
Powiązane: Dlaczego DRY jest ważne?
Steven Jeuris
Dlaczego nie uwzględniacie uwarunkowanych rzeczy w ich własnych funkcjach?
Rig

Odpowiedzi:

19

argumenty boolowskie uruchamiające różne ścieżki kodu w jednej funkcji / metodzie to okropny zapach kodu .

To, co robisz, narusza zasady luźnego łączenia oraz zasady wysokiej spójności i pojedynczej odpowiedzialności , które są o wiele ważniejsze niż SUCHE pierwszeństwo.

Oznacza to, że rzeczy powinny zależeć od innych rzeczy tylko wtedy, gdy muszą ( Łączenie ) i że powinny robić jedną rzecz i tylko jedną rzecz (bardzo dobrze) ( Spójność ).

Przez własne pominięcie jest to zbyt mocno powiązane (wszystkie flagi boolowskie są rodzajem zależności stanu, co jest jednym z najgorszych!) I ma zbyt wiele indywidualnych obowiązków (zbyt skomplikowanych).

To, co robisz, nie jest w duchu SUCHEGO. OSUSZANIE polega bardziej na powtarzaniu ( Rskrótach REPEAT). Unikanie kopiowania i wklejania to jego najbardziej podstawowa forma. To, co robisz, nie ma związku z powtarzaniem.

Problem polega na tym, że rozkład kodu nie jest na odpowiednim poziomie. Jeśli uważasz, że będziesz miał zduplikowany kod, powinna to być jego własna funkcja / metoda, która jest odpowiednio sparametryzowana, a nie kopiowana i wklejana, a pozostałe powinny być nazwane opisowo i delegowane do podstawowej funkcji / metody.

Społeczność
źródło
Ok, wydaje mi się, że sposób, w jaki piszę, nie jest zbyt dobry (moje początkowe podejrzenie). Nie jestem do końca pewien, co to za „Sprit of DRY”
Albinoswordfish
4

Fakt, że przechodzisz w booleanach, aby funkcja spełniała różne funkcje, stanowi naruszenie zasady pojedynczej odpowiedzialności. Funkcja powinna zrobić jedną rzecz. Powinien robić tylko jedną rzecz i powinien robić to dobrze.

Pachnie, jakbyś musiał podzielić go na kilka mniejszych funkcji z opisowymi nazwami, oddzielając ścieżki kodu odpowiadające wartościom tych boolean.

Gdy to zrobisz, powinieneś poszukać wspólnego kodu w wynikowych funkcjach i rozdzielić go na własne funkcje. W zależności od tego, jak skomplikowana jest ta rzecz, możesz nawet wyróżnić klasę lub dwie.

Oczywiście zakłada to, że korzystasz z systemu kontroli wersji i masz dobry zestaw testów, dzięki czemu możesz refaktoryzować bez obawy, że coś zepsujesz.

Dima
źródło
3

Dlaczego nie utworzysz innej funkcji zawierającej całą logikę w swojej funkcji, zanim zdecydujesz się coś zrobić2, a następnie mieć trzy funkcje, takie jak:

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

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

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

A teraz, przekazując trzy takie same typy parametrów do trzech różnych funkcji, ponownie będziesz się powtarzał, więc powinieneś zdefiniować strukturę lub klasę zawierającą A, B, C.

Alternatywnie możesz utworzyć klasę zawierającą parametry A, B, C i listę operacji do wykonania. Dodaj, które operacje (coś, coś2) chcesz wykonać z tymi parametrami (A, B, C), rejestrując operacje na obiekcie. Następnie zastosuj metodę wywoływania wszystkich zarejestrowanych operacji na obiekcie.

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);
        }
    }
}
Mert Akcakaya
źródło
Aby uwzględnić możliwe kombinacje wartości dla doometometrii i doometru2 wartości logicznych, potrzebujesz 4 funkcji, a nie 2, oprócz podstawowej funkcji createTrajectoryFromPoint. To podejście nie skaluje się dobrze wraz ze wzrostem liczby opcji, a nawet nazywanie funkcji staje się uciążliwe.
JGWeissman
2

DRY można posunąć za daleko, najlepiej stosować zasadę pojedynczej odpowiedzialności (SRP) w połączeniu z DRY. Dodanie flag bool do funkcji, aby zrobić nieco inne wersje tego samego kodu, może być znakiem, że robisz za dużo z jedną funkcją. W takim przypadku sugerowałbym utworzenie osobnej funkcji dla każdego przypadku reprezentowanego przez flagi, a następnie, gdy masz każdą wypisaną funkcję, powinno być dość oczywiste, czy istnieje wspólna sekcja, którą można przenieść do funkcji prywatnej bez przekazywania wszystkich flag , jeśli nie ma widocznej sekcji kodu, to tak naprawdę nie powtarzasz się, masz kilka różnych, ale podobnych przypadków.

Ryathal
źródło
1

Zwykle przechodzę kilka kroków z tym problemem, zatrzymując się, gdy nie mogę dowiedzieć się, jak iść dalej.

Najpierw zrób to, co zrobiłeś. Idź ostro z DRY. Jeśli nie skończysz z wielkim owłosionym bałaganem, skończysz. Jeśli, podobnie jak w twoim przypadku, nie masz duplikatu kodu, ale każda wartość logiczna ma sprawdzoną wartość w 20 różnych miejscach, przejdź do następnego kroku.

Po drugie, podziel kod na bloki. Do boolean przypisuje się tylko jeden raz (no może czasami dwa razy), aby skierować wykonanie do odpowiedniego bloku. Z dwoma booleanami masz cztery bloki. Każdy blok jest prawie identyczny. DRY zniknął. Nie rób z każdego bloku osobnej metody. Byłoby to bardziej eleganckie, ale umieszczenie całego kodu w jednej metodzie sprawia, że ​​każdy, kto zajmuje się konserwacją, widzi, że musi wprowadzić każdą zmianę w czterech miejscach. Przy dobrze zorganizowanym kodzie i wysokim monitorze różnice i błędy będą prawie oczywiste. Masz teraz możliwy do utrzymania kod i będzie działać szybciej niż oryginalny splątanej bałagan.

Po trzecie, spróbuj pobrać duplikaty linii kodu z każdego z bloków i przekształć je w ładne, proste metody. Czasami nic nie możesz zrobić. Czasami niewiele możesz zrobić. Ale z każdym krokiem cofasz się w kierunku DRY i sprawia, że ​​kod jest nieco łatwiejszy do śledzenia i bezpieczniejszy w utrzymaniu. Idealnie byłoby, gdyby Twoja oryginalna metoda nie zawierała duplikatu kodu. W tym momencie możesz podzielić go na kilka metod bez parametrów boolowskich lub nie. Wygoda kodu telefonicznego jest teraz głównym problemem.

Dodałem swoją odpowiedź do dużej liczby już tutaj ze względu na drugi krok. Nienawidzę powielania kodu, ale jeśli jest to jedyny rozsądny sposób rozwiązania problemu, zrób to w taki sposób, aby każdy na pierwszy rzut oka wiedział, co robisz. Użyj wielu bloków i tylko jednej metody. Uczyń bloki możliwie identycznymi pod względem nazw, odstępów, wyrównania, ... wszystkiego. Różnice powinny wyskoczyć na czytnik. To może sprawić, że oczywiste będzie, jak przepisać go w SUCHY sposób, a jeśli nie, utrzymanie go będzie dość proste.

RalphChapin
źródło
0

Alternatywnym podejściem jest zastąpienie parametrów boolowskich parametrami interfejsu, kodem do obsługi różnych wartości boolowskich refaktoryzowanych do implementacji interfejsu. Tak byś miał

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

gdzie masz implementacje IX i IY, które reprezentują różne wartości dla boolean. W treści funkcji, gdziekolwiek masz

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

zastępujesz go wywołaniem metody zdefiniowanej w IX, z implementacjami zawierającymi pominięte bloki kodu.

JGWeissman
źródło