Staram się w jak największym stopniu przestrzegać zasady jednolitej odpowiedzialności (SRP) i przyzwyczaiłem się do pewnego wzorca (dla SRP dotyczącego metod), w dużej mierze polegającego na delegatach. Chciałbym wiedzieć, czy to podejście jest dobre, czy też są z nim jakieś poważne problemy.
Na przykład, aby sprawdzić dane wejściowe do konstruktora, mógłbym wprowadzić następującą metodę (dane Stream
wejściowe są losowe, mogą być dowolne)
private void CheckInput(Stream stream)
{
if(stream == null)
{
throw new ArgumentNullException();
}
if(!stream.CanWrite)
{
throw new ArgumentException();
}
}
Ta metoda (prawdopodobnie) robi więcej niż jedną rzecz
- Sprawdź wejścia
- Rzuć różne wyjątki
Dlatego, aby zastosować się do SRP, zmieniłem logikę na
private void CheckInput(Stream stream,
params (Predicate<Stream> predicate, Action action)[] inputCheckers)
{
foreach(var inputChecker in inputCheckers)
{
if(inputChecker.predicate(stream))
{
inputChecker.action();
}
}
}
Co podobno robi tylko jedną rzecz (prawda?): Sprawdź dane wejściowe. Do faktycznego sprawdzania danych wejściowych i zgłaszania wyjątków wprowadziłem metody takie jak
bool StreamIsNull(Stream s)
{
return s == null;
}
bool StreamIsReadonly(Stream s)
{
return !s.CanWrite;
}
void Throw<TException>() where TException : Exception, new()
{
throw new TException();
}
i może zadzwonić CheckInput
jak
CheckInput(stream,
(this.StreamIsNull, this.Throw<ArgumentNullException>),
(this.StreamIsReadonly, this.Throw<ArgumentException>))
Czy jest to w ogóle lepsze niż pierwsza opcja, czy też wprowadzam niepotrzebną złożoność? Czy jest jakiś sposób, aby poprawić ten wzór, jeśli jest w ogóle wykonalny?
źródło
CheckInput
wciąż robi wiele rzeczy: zarówno iteruje tablicę, jak i wywołuje funkcję predykatu i wywołuje funkcję akcji. Czy to nie jest naruszenie SRP?Odpowiedzi:
SRP jest prawdopodobnie najbardziej niezrozumianą zasadą oprogramowania.
Aplikacja zbudowana jest z modułów, które są zbudowane z modułów, które są zbudowane z ...
Na dole jedna funkcja, taka jak,
CheckInput
będzie zawierała tylko odrobinę logiki, ale w miarę wzrostu liczby kolejnych modułów hermetyzuje się coraz więcej logiki i jest to normalne .SRP nie polega na wykonaniu pojedynczej akcji atomowej . Chodzi o jedną odpowiedzialność, nawet jeśli ta odpowiedzialność wymaga wielu działań ... i ostatecznie dotyczy konserwacji i testowania :
Fakt, że
CheckInput
jest realizowany z dwoma czekami i podnosi dwa różne wyjątki, jest do pewnego stopnia nieistotny .CheckInput
ma wąską odpowiedzialność: zapewnienie, że dane wejściowe są zgodne z wymogami. Tak, istnieje wiele wymagań, ale nie oznacza to, że istnieje wiele obowiązków. Tak, możesz podzielić czeki, ale jak to by pomogło? W pewnym momencie kontrole muszą być w jakiś sposób wymienione.Porównajmy:
przeciw:
Teraz
CheckInput
robi mniej ... ale dzwoniący robi więcej!Przesunąłeś listę wymagań z miejsca
CheckInput
, w którym są zawarte, doConstructor
miejsca , w którym są widoczne.Czy to dobra zmiana? To zależy:
CheckInput
jest tam tylko wywoływany: jest dyskusyjny, z jednej strony uwidacznia wymagania, z drugiej - zaśmieca kod;CheckInput
jest wywoływany wiele razy z tymi samymi wymaganiami , oznacza to naruszenie DRY i masz problem z enkapsulacją.Ważne jest, aby zdawać sobie sprawę, że jedna odpowiedzialność może wiązać się z dużym nakładem pracy. „Mózg” samochodu samojezdnego ma jedną odpowiedzialność:
Jest to jedna odpowiedzialność, ale wymaga koordynacji wielu czujników i aktorów, podejmowania wielu decyzji, a nawet ma potencjalnie sprzeczne wymagania 1 ...
... jednak wszystko jest zamknięte. Więc klienta to nie obchodzi.
1 bezpieczeństwo pasażerów, bezpieczeństwo innych, przestrzeganie przepisów, ...
źródło
Cytując wujka Boba na temat SRP ( https://8thlight.com/blog/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html ):
Wyjaśnia, że moduły oprogramowania muszą uwzględniać obawy konkretnych interesariuszy. Dlatego odpowiadając na twoje pytanie:
IMO, patrzysz tylko na jedną metodę, kiedy powinieneś spojrzeć na wyższy poziom (w tym przypadku poziom klasy). Być może powinniśmy rzucić okiem na to, co obecnie robi twoja klasa (a to wymaga więcej wyjaśnień na temat twojego scenariusza). Na razie twoja klasa wciąż robi to samo. Na przykład, jeśli jutro pojawi się prośba o zmianę dotyczącą pewnej walidacji (np. „Teraz strumień może mieć wartość NULL”), nadal musisz przejść do tej klasy i zmienić w niej rzeczy.
źródło
checkInputs()
należy je podzielić, powiedzmy nacheckMarketingInputs()
icheckRegulatoryInputs()
. W przeciwnym razie dobrze jest połączyć je wszystkie w jedną metodę.Nie, SRP nie informuje o tej zmianie.
Zadaj sobie pytanie, dlaczego w twoim narzędziu kontrolnym nie ma sprawdzania, czy „przekazany obiekt jest strumieniem” . Odpowiedź jest oczywista: język uniemożliwia programowi wywołującemu kompilację programu, który przechodzi w trybie innym niż strumień.
System typów C # jest niewystarczający do spełnienia twoich potrzeb; twoje kontrole wdrażają wymuszanie niezmienników, których dziś nie można wyrazić w systemie typów . Gdyby istniał sposób, aby powiedzieć, że metoda pobiera strumień, który nie jest dopuszczalny dla wartości zerowych, napisałbyś to, ale tak nie jest, więc zrobiłeś następną najlepszą rzecz: wymusiłeś ograniczenie typu w czasie wykonywania. Mamy nadzieję, że również to udokumentowałeś, aby programiści używający twojej metody nie musieli jej naruszać, nie zdawali testów na testach, a następnie naprawiali problem.
Umieszczanie typów w metodzie nie stanowi naruszenia zasady pojedynczej odpowiedzialności; metoda ta nie wymusza również spełnienia warunków wstępnych ani zapewnienia spełnienia warunków dodatkowych.
źródło
Nie wszystkie obowiązki są sobie równe.
Oto dwie szuflady. Oboje mają jedną odpowiedzialność. Każdy z nich ma nazwy, które informują o tym, co do nich należy. Jednym z nich jest szuflada na sztućce. Druga to szuflada na śmieci.
Jaka jest różnica? Szuflada na sztućce wyjaśnia, co do niej nie należy. Szuflada na śmieci przyjmuje jednak wszystko, co pasuje. Wyjmowanie łyżek z szuflady ze srebrnymi naczyniami wydaje się bardzo złe. Trudno mi jednak wymyślić cokolwiek, co zostałoby pominięte, gdyby zostało usunięte z szuflady śmieci. Prawda jest taka, że możesz twierdzić, że cokolwiek ma jedną odpowiedzialność, ale która według ciebie ma bardziej skoncentrowaną jedną odpowiedzialność?
Obiekt mający jedną odpowiedzialność nie oznacza, że może się tu zdarzyć tylko jedna rzecz. Obowiązki mogą się zagnieżdżać. Ale te obowiązki związane z zagnieżdżaniem powinny mieć sens, nie powinny cię zaskakiwać, gdy znajdziesz je tutaj, i powinieneś je przegapić, jeśli odejdą.
Więc kiedy oferujesz
CheckInput(Stream stream);
Nie martwię się, że to zarówno sprawdzanie danych wejściowych, jak i zgłaszanie wyjątków. Byłbym zaniepokojony, gdyby jednocześnie sprawdzał dane wejściowe i zapisywał dane wejściowe. To paskudna niespodzianka. Tego, którego nie umknęłoby, gdyby go nie było.
źródło
Kiedy zawiązujesz węzły i piszesz dziwny kod, aby zachować zgodność z Ważną Zasadą Oprogramowania, zwykle źle zrozumiałeś zasadę (choć czasami zasada jest błędna). Jak wskazuje doskonała odpowiedź Matthieu, całe znaczenie SRP zależy od definicji „odpowiedzialności”.
Doświadczeni programiści widzą te zasady i odnoszą je do wspomnień o spreparowanym przez nas kodzie; mniej doświadczeni programiści je widzą i mogą nie mieć z nimi nic wspólnego. To abstrakcja unosząca się w przestrzeni, z szerokim uśmiechem i bez kota. Sądzą, i zwykle idzie źle. Zanim rozwiniesz programowanie wyczucia konia, różnica między dziwnym, skomplikowanym kodem a normalnym kodem wcale nie jest oczywista.
To nie jest przykazanie religijne, którego należy przestrzegać bez względu na osobiste konsekwencje. Jest to raczej ogólna zasada mająca na celu sformalizowanie jednego elementu programowania wyczuwania konia i pomoc w utrzymaniu kodu tak prostym i przejrzystym, jak to możliwe. Jeśli ma odwrotny skutek, masz prawo poszukać informacji z zewnątrz.
W programowaniu nie można przejść o wiele bardziej złośliwie, niż próbować wydedukować znaczenie identyfikatora z pierwszych zasad, po prostu wpatrując się w niego, i dotyczy to pisania identyfikatorów w pisaniu o programowaniu tak samo, jak identyfikatorów w rzeczywistym kodzie.
źródło
Rola CheckInput
Po pierwsze, pozwól mi położyć oczywiste tam,
CheckInput
to robi jedno, nawet jeśli to jest sprawdzanie różnych aspektów. Ostatecznie sprawdza dane wejściowe . Można argumentować, że nie jest to jedno, jeśli masz do czynienia z wywoływanymi metodamiDoSomething
, ale myślę, że można bezpiecznie założyć, że sprawdzanie danych wejściowych nie jest zbyt niejasne.Dodanie tego wzorca dla predykatów może być przydatne, jeśli nie chcesz, aby logika sprawdzania danych wejściowych została umieszczona w twojej klasie, ale ten wzorzec wydaje się dość szczegółowy dla tego, co próbujesz osiągnąć. Bardziej bezpośrednie może być przekazanie interfejsu
IStreamValidator
jedną metodą,isValid(Stream)
jeśli tego właśnie chcesz. Każda implementacja klasyIStreamValidator
może używać predykatów takich jakStreamIsNull
lubStreamIsReadonly
jeśli sobie tego życzy, ale wracając do punktu centralnego, jest to raczej niedorzeczna zmiana, aby wprowadzić zasadę pojedynczej odpowiedzialności.Kontrola poczytalności
Moim pomysłem jest, abyśmy wszyscy mieli „kontrolę poczytalności”, aby upewnić się, że masz przynajmniej do czynienia ze Strumieniem, który nie ma wartości zerowej i jest zapisywalny, a ta podstawowa kontrola w jakiś sposób nie czyni z twojej klasy walidatora strumieni. Pamiętaj, że bardziej wyrafinowane czeki najlepiej pozostawić poza klasą, ale to tam jest granica. Gdy musisz zacząć zmieniać stan swojego strumienia, czytając z niego lub przeznaczając zasoby na walidację, zacząłeś formalną walidację swojego strumienia i właśnie to należy wciągnąć do jego własnej klasy.
Wniosek
Myślę, że jeśli zastosujesz wzór, aby lepiej zorganizować aspekt swojej klasy, warto być w jej własnej klasie. Ponieważ wzór nie pasuje, powinieneś również zapytać, czy tak naprawdę należy do swojej klasy. Uważam, że jeśli nie uważasz, że walidacja strumienia prawdopodobnie zostanie zmieniona w przyszłości, a zwłaszcza jeśli uważasz, że ta walidacja może nawet mieć charakter dynamiczny, to opisany przez Ciebie wzorzec jest dobrym pomysłem, nawet jeśli może być początkowo trywialne. W przeciwnym razie nie ma potrzeby arbitralnego komplikowania programu. Nazwijmy szpadel łopatą. Sprawdzanie poprawności to jedno, ale sprawdzanie, czy dane wejściowe są zerowe, nie jest sprawdzaniem poprawności, dlatego uważam, że możesz być bezpieczny, zachowując je w swojej klasie bez naruszania zasady pojedynczej odpowiedzialności.
źródło
Zasada wyraźnie nie określa, że fragment kodu powinien „robić tylko jedną rzecz”.
„Odpowiedzialność” w SRP należy rozumieć na poziomie wymagań. Kod odpowiada za spełnienie wymagań biznesowych. SRP jest naruszane, jeśli obiekt spełnia więcej niż jedno niezależne wymagania biznesowe . Przez niezależne oznacza to, że jedno wymaganie może się zmienić, a drugie pozostaje na swoim miejscu.
Można sobie wyobrazić wprowadzenie nowego wymogu biznesowego, co oznacza, że ten konkretny obiekt nie powinien sprawdzać, czy jest czytelny, podczas gdy inne wymaganie biznesowe wciąż wymaga, aby obiekt sprawdzał, czy jest czytelny? Nie, ponieważ wymagania biznesowe nie określają szczegółów implementacji na tym poziomie.
Rzeczywistym przykładem naruszenia SRP może być kod taki jak ten:
Ten kod jest bardzo prosty, ale nadal można sobie wyobrazić, że tekst zmieni się niezależnie od oczekiwanej daty dostawy, ponieważ są one podejmowane przez różne części firmy.
źródło
Podoba mi się punkt z odpowiedzi @ EricLippert :
EricLippert ma rację, że jest to problem dla systemu typów. A ponieważ chcesz zastosować zasadę pojedynczej odpowiedzialności (SRP), w zasadzie potrzebujesz systemu typów, który będzie odpowiedzialny za to zadanie.
W rzeczywistości można to zrobić w języku C #. Możemy złapać literały
null
w czasie kompilacji, a następnie złapać literałynull
w czasie wykonywania. To nie jest tak dobre, jak sprawdzanie pełnego czasu kompilacji, ale jest to ścisła poprawa w porównaniu z nigdy nie łapaniem w czasie kompilacji.Więc wiesz, jak ma C #
Nullable<T>
? Odwróćmy to i zróbmyNonNullable<T>
:Teraz zamiast pisać
, tylko napisz:
Następnie są trzy przypadki użycia:
Połączenia użytkownika
Foo()
z wartością inną niż nullStream
:Jest to pożądany przypadek użycia i działa z lub bez
NonNullable<>
.Połączenia użytkownika
Foo()
z zerowymStream
:To błąd wywołania. Tutaj
NonNullable<>
pomaga poinformować użytkownika, że nie powinien tego robić, ale tak naprawdę nie zatrzymuje go. Tak czy inaczej, skutkuje to czasem działaniaNullArgumentException
.Połączenia użytkownika
Foo()
znull
:null
nie zostanie niejawnie przekonwertowany na aNonNullable<>
, więc użytkownik otrzyma błąd w IDE przed uruchomieniem. To delegowanie sprawdzania wartości zerowej do systemu typów, tak jak zaleciłby SRP.Możesz rozszerzyć tę metodę również o inne argumenty dotyczące argumentów. Na przykład, ponieważ chcesz zapisywać strumień, możesz zdefiniować
struct WriteableStream<T> where T:Stream
sprawdzanie zarówno dla konstruktora, jaknull
i dlastream.CanWrite
niego. Nadal byłby to sprawdzanie typu w czasie wykonywania, ale:Ozdabia typ
WriteableStream
kwalifikatorem, sygnalizując potrzebę dzwoniącym.Sprawdza w jednym miejscu w kodzie, więc nie trzeba go powtarzać za
throw InvalidArgumentException
każdym razem.Lepiej odpowiada SRP, przesuwając obowiązki sprawdzania typu na system typów (rozszerzany przez ogólne dekoratory).
źródło
Twoje podejście jest obecnie proceduralne. Rozbijasz
Stream
przedmiot i sprawdzasz go z zewnątrz. Nie rób tego - to niszczy enkapsulację. NiechStream
będzie odpowiedzialny za własną walidację. Nie możemy starać się zastosować SRP, dopóki nie będziemy mieli kilku klas do zastosowania.Oto,
Stream
który wykonuje akcję tylko wtedy, gdy przejdzie walidację:Ale teraz naruszamy SRP! „Klasa powinna mieć tylko jeden powód do zmiany”. Mamy połączenie 1) walidacji i 2) rzeczywistej logiki. Mamy dwa powody, które mogą wymagać zmiany.
Możemy to rozwiązać za pomocą walidacji dekoratorów . Najpierw musimy przekonwertować nasz
Stream
interfejs na interfejs i zaimplementować go jako konkretną klasę.Możemy teraz napisać dekorator, który otacza a
Stream
, wykonuje walidację i odkłada na podanąStream
rzeczywistą logikę akcji.Teraz możemy je komponować w dowolny sposób:
Chcesz dodatkowej weryfikacji? Dodaj kolejny dekorator.
źródło
Zadaniem klasy jest świadczenie usługi zgodnej z umową . Klasa ma zawsze umowę: zestaw wymagań dotyczących jej używania i obiecuje, że przedstawi swój stan i wyniki, pod warunkiem, że wymagania zostaną spełnione. Umowa ta może być wyraźna, poprzez dokumentację i / lub stwierdzenia, lub dorozumiana, ale zawsze istnieje.
Częścią umowy twojej klasy jest to, że program wywołujący podaje konstruktorowi kilka argumentów, które nie mogą być zerowe. Wykonanie umowy jest obowiązkiem klasy, więc sprawdzenie, czy osoba dzwoniąca spełniła swoją część umowy, można łatwo uznać za wchodzące w zakres odpowiedzialności klasy.
Pomysł, że klasa realizuje kontrakt wynika z Bertranda Meyera , projektanta języka programowania Eiffel i pomysłu projektowania na podstawie umowy . Język Eiffla sprawia, że specyfikacja i kontrola umowy są częścią tego języka.
źródło
Jak wskazano w innych odpowiedziach SRP jest często źle rozumiany. Nie chodzi o kod atomowy, który pełni tylko jedną funkcję. Chodzi o to, aby upewnić się, że twoje obiekty i metody wykonują tylko jedną rzecz, a jedna rzecz jest wykonywana tylko w jednym miejscu.
Spójrzmy na kiepski przykład pseudo kodu.
W naszym raczej absurdalnym przykładzie „odpowiedzialnością” konstruktora matematyki # jest uczynienie obiektu matematyki użytecznym. Robi to najpierw odkażając dane wejściowe, a następnie upewniając się, że wartości nie są -1.
Jest to poprawne SRP, ponieważ konstruktor robi tylko jedną rzecz. Przygotowuje obiekt Math. Jednak nie jest to łatwe do utrzymania. Narusza SUCHO.
Weźmy więc kolejną przepustkę
W tej przepustce trochę lepiej zrozumieliśmy DRY, ale nadal mamy sposoby na DRY. Z drugiej strony SRP wydaje się trochę nie w porządku. Teraz mamy dwie funkcje z tą samą pracą. Zarówno cleanX, jak i cleanY dezynfekują dane wejściowe.
Spróbujmy jeszcze raz
Teraz w końcu były lepsze na temat DRY, a SRP wydaje się zgadzać. Mamy tylko jedno miejsce, które wykonuje „odkażającą” pracę.
Kod jest teoretycznie łatwiejszy w utrzymaniu i lepszy, ale kiedy idziemy naprawić błąd i go zaostrzyć, musimy to zrobić tylko w jednym miejscu.
W większości rzeczywistych przypadków obiekty byłyby bardziej złożone, a SRP zostałby zastosowany na wielu obiektach. Na przykład wiek może należeć do Ojca, Matki, Syna, Córki, więc zamiast 4 klas, które określają wiek od daty urodzenia, masz klasę Osoby, która to robi, i 4 klasy dziedziczą po tym. Ale mam nadzieję, że ten przykład pomoże wyjaśnić. SRP nie polega na akcjach atomowych, ale na wykonywaniu „zadania”.
źródło
Mówiąc o SRP, wujek Bob nie lubi wszędzie zerowanych czeków zerowych. Ogólnie rzecz biorąc, jako zespół powinieneś unikać używania parametrów zerowych dla konstruktorów, gdy tylko jest to możliwe. Gdy opublikujesz swój kod poza zespołem, rzeczy mogą się zmienić.
Wymuszanie braku zerowania parametrów konstruktora bez uprzedniego zapewnienia spójności danej klasy powoduje rozdęcie kodu wywołującego, zwłaszcza testów.
Jeśli naprawdę chcesz egzekwować takie umowy, rozważ użycie
Debug.Assert
lub coś podobnego w celu zmniejszenia bałaganu:źródło