Czy konstruktor, który sprawdza poprawność swoich argumentów, narusza SRP?

66

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 Streamwejś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ć CheckInputjak

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?

Paul Kertscher
źródło
26
Mógłbym argumentować, że CheckInputwciąż robi wiele rzeczy: zarówno iteruje tablicę, jak i wywołuje funkcję predykatu i wywołuje funkcję akcji. Czy to nie jest naruszenie SRP?
Bart van Ingen Schenau
8
Tak, o to właśnie starałem się powiedzieć.
Bart van Ingen Schenau
135
ważne jest, aby pamiętać, że jest to zasada pojedynczej odpowiedzialności ; nie zasada pojedynczego działania . Ma jedną odpowiedzialność: zweryfikować, czy strumień jest zdefiniowany i zapisywalny.
David Arno,
40
Należy pamiętać, że celem tych zasad oprogramowania jest uczynienie kodu bardziej czytelnym i łatwym w utrzymaniu. Twój oryginalny CheckInput jest o wiele łatwiejszy do odczytania i konserwacji niż Twoja wersja refaktoryzowana. W rzeczywistości, gdybym kiedykolwiek natknął się na twoją ostatnią metodę CheckInput w bazie kodu, zeskrobałbym wszystko i przepisałbym ją tak, aby pasowała do tego, co pierwotnie posiadałeś.
17 z 26
17
Te „zasady” są praktycznie bezużyteczne, ponieważ możesz po prostu zdefiniować „pojedynczą odpowiedzialność” w dowolny sposób, niezależnie od tego, jaki był twój pierwotny pomysł. Ale jeśli spróbujesz je sztywno zastosować, to zapewne skończysz z tego rodzaju kodem, który, szczerze mówiąc, jest trudny do zrozumienia.
Casey

Odpowiedzi:

151

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, CheckInputbę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 :

  • promuje enkapsulację (unikanie Boskich Przedmiotów),
  • sprzyja rozdzielaniu obaw (unikanie falowania zmian w całej bazie kodu),
  • pomaga przetestować, zawężając zakres obowiązków.

Fakt, że CheckInputjest realizowany z dwoma czekami i podnosi dwa różne wyjątki, jest do pewnego stopnia nieistotny .

CheckInputma 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:

Constructor(Stream stream) {
    CheckInput(stream);
    // ...
}

przeciw:

Constructor(Stream stream) {
    CheckInput(stream,
        (this.StreamIsNull, this.Throw<ArgumentNullException>),
        (this.StreamIsReadonly, this.Throw<ArgumentException>));
    // ...
}

Teraz CheckInputrobi mniej ... ale dzwoniący robi więcej!

Przesunąłeś listę wymagań z miejsca CheckInput, w którym są zawarte, do Constructormiejsca , w którym są widoczne.

Czy to dobra zmiana? To zależy:

  • Jeśli CheckInputjest tam tylko wywoływany: jest dyskusyjny, z jednej strony uwidacznia wymagania, z drugiej - zaśmieca kod;
  • Jeśli CheckInputjest 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ść:

Prowadzenie samochodu do miejsca docelowego.

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, ...

Matthieu M.
źródło
2
Myślę, że sposób, w jaki używasz słowa „enkapsulacja” i jego pochodnych, jest mylący. Poza tym świetna odpowiedź!
Fabio Turati,
4
Zgadzam się z twoją odpowiedzią, ale argument samokierujący mózgiem samochodu często kusi ludzi do przełamania SRP. Jak powiedziałeś, to moduły wykonane z modułów wykonanych z modułów. Możesz określić cel tego całego systemu, ale ten system powinien zostać rozbity sam. Możesz rozwiązać prawie każdy problem.
Sava B.,
13
@SavaB .: Jasne, ale zasada pozostaje ta sama. Moduł powinien mieć pojedynczą odpowiedzialność, choć o większym zakresie niż jego składniki.
Matthieu M.,
3
@ user949300 Dobra, a może po prostu „jazda”. Naprawdę, „kierowanie pojazdem” jest odpowiedzialnością, a „bezpiecznie” i „legalnie” to wymagania dotyczące sposobu, w jaki spełnia on tę odpowiedzialność. Często określamy wymagania przy określaniu odpowiedzialności.
Brian McCutchon,
1
„SRP jest prawdopodobnie najbardziej niezrozumianą zasadą oprogramowania”. Dowodzi tego odpowiedź :)
Michael,
41

Cytując wujka Boba na temat SRP ( https://8thlight.com/blog/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html ):

Zasada pojedynczej odpowiedzialności (SRP) stanowi, że każdy moduł oprogramowania powinien mieć tylko jeden powód do zmiany.

... Ta zasada dotyczy ludzi.

... Kiedy piszesz moduł oprogramowania, chcesz się upewnić, że kiedy zmiany są wymagane, zmiany mogą pochodzić tylko od jednej osoby, a raczej od jednej ściśle powiązanej grupy osób reprezentujących jedną wąsko zdefiniowaną funkcję biznesową.

... To jest powód, dla którego nie umieszczamy SQL na stronach JSP. Dlatego nie generujemy HTML w modułach obliczających wyniki. Z tego powodu reguły biznesowe nie powinny znać schematu bazy danych. To jest powód, dla którego rozdzielamy obawy.

Wyjaśnia, że ​​moduły oprogramowania muszą uwzględniać obawy konkretnych interesariuszy. Dlatego odpowiadając na twoje pytanie:

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?

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.

Emerson Cardoso
źródło
4
Najlepsza odpowiedź. Aby rozwinąć kwestię PO, jeśli kontrole straży pochodzą od dwóch różnych interesariuszy / działów, checkInputs()należy je podzielić, powiedzmy na checkMarketingInputs()i checkRegulatoryInputs(). W przeciwnym razie dobrze jest połączyć je wszystkie w jedną metodę.
user949300,
36

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.

Eric Lippert
źródło
1
Ponadto pozostawienie utworzonego obiektu w prawidłowym stanie jest jedyną odpowiedzialnością konstruktora zasadniczo zawsze. Jeśli, jak wspomniałeś, wymaga dodatkowych kontroli, których środowisko wykonawcze i / lub kompilator nie są w stanie zapewnić, to tak naprawdę nie można tego obejść.
SBI,
23

Nie wszystkie obowiązki są sobie równe.

wprowadź opis zdjęcia tutaj

wprowadź opis zdjęcia tutaj

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.

candied_orange
źródło
21

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.

Ed Plunkett
źródło
14

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 metodami DoSomething, 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 IStreamValidatorjedną metodą, isValid(Stream)jeśli tego właśnie chcesz. Każda implementacja klasy IStreamValidatormoże używać predykatów takich jak StreamIsNulllub StreamIsReadonlyjeś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.

Neil
źródło
4

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:

var message = "Your package will arrive before " + DateTime.Now.AddDays(14);

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.

JacquesB
źródło
Odmienna klasa dla praktycznie każdego wymagania brzmi jak bezbożny koszmar.
whatsisname
@whatsisname: Być może SRP nie jest dla ciebie. Żadna zasada projektowania nie ma zastosowania do wszystkich rodzajów i rozmiarów projektów. (Należy jednak pamiętać, że mówimy tylko o niezależnych wymaganiach (tj. Możemy zmieniać niezależnie), a nie tylko o każdym wymaganiu, ponieważ od tego czasu będzie to zależeć tylko od tego, jak szczegółowe są określone.)
JacquesB
Myślę, że bardziej chodzi o to, że SRP wymaga elementu oceny sytuacyjnej, który trudno jest opisać w jednym chwytliwym zdaniu.
whatsisname
@whatsisname: Całkowicie się zgadzam.
JacquesB
+1 dla SRP zostaje naruszone, 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ć, podczas gdy drugie pozostanie na miejscu
Juzer Ali
3

Podoba mi się punkt z odpowiedzi @ EricLippert :

Zadaj sobie pytanie, dlaczego w twoim narzędziu kontrolnym nie ma sprawdzania, czy przekazany obiekt to strumień . 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.

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 nullw czasie kompilacji, a następnie złapać literały nullw 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óbmy NonNullable<T>:

public struct NonNullable<T> where T : class
{
    public T Value { get; private set; }
    public NonNullable(T value)
    {
        if (value == null) { throw new NullArgumentException(); }
        this.Value = value;
    }
    //  Ease-of-use:
    public static implicit operator T(NonNullable<T> value) { return value.Value; }
    public static implicit operator NonNullable<T>(T value) { return new NonNullable<T>(value); }

    //  Hack-ish overloads that prevent null-literals from being implicitly converted into NonNullable<T>'s.
    public static implicit operator NonNullable<T>(Tuple<T> value) { return new NonNullable<T>(value.Item1); }
    public static implicit operator NonNullable<T>(Tuple<T, T> value) { return new NonNullable<T>(value.Item1); }
}

Teraz zamiast pisać

public void Foo(Stream stream)
{
  if (stream == null) { throw new NullArgumentException(); }

  // ...method code...
}

, tylko napisz:

public void Foo(NonNullable<Stream> stream)
{
  // ...method code...
}

Następnie są trzy przypadki użycia:

  1. Połączenia użytkownika Foo()z wartością inną niż null Stream:

    Stream stream = new Stream();
    Foo(stream);
    

    Jest to pożądany przypadek użycia i działa z lub bez NonNullable<>.

  2. Połączenia użytkownika Foo()z zerowym Stream:

    Stream stream = null;
    Foo(stream);
    

    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łania NullArgumentException.

  3. Połączenia użytkownika Foo()z null:

    Foo(null);

    nullnie zostanie niejawnie przekonwertowany na a NonNullable<>, 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:Streamsprawdzanie zarówno dla konstruktora, jak nulli dla stream.CanWriteniego. Nadal byłby to sprawdzanie typu w czasie wykonywania, ale:

  1. Ozdabia typ WriteableStreamkwalifikatorem, sygnalizując potrzebę dzwoniącym.

  2. Sprawdza w jednym miejscu w kodzie, więc nie trzeba go powtarzać za throw InvalidArgumentExceptionkażdym razem.

  3. Lepiej odpowiada SRP, przesuwając obowiązki sprawdzania typu na system typów (rozszerzany przez ogólne dekoratory).

Nat
źródło
3

Twoje podejście jest obecnie proceduralne. Rozbijasz Streamprzedmiot i sprawdzasz go z zewnątrz. Nie rób tego - to niszczy enkapsulację. Niech Streambędzie odpowiedzialny za własną walidację. Nie możemy starać się zastosować SRP, dopóki nie będziemy mieli kilku klas do zastosowania.

Oto, Streamktóry wykonuje akcję tylko wtedy, gdy przejdzie walidację:

class Stream
{
    public void someAction()
    {
        if(!stream.canWrite)
        {
            throw new ArgumentException();
        }

        System.out.println("My action");
    }
}

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 Streaminterfejs na interfejs i zaimplementować go jako konkretną klasę.

interface Stream
{
    void someAction();
}

class DefaultStream implements Stream
{
    @Override
    public void someAction()
    {
        System.out.println("My action");
    }
}

Możemy teraz napisać dekorator, który otacza a Stream, wykonuje walidację i odkłada na podaną Streamrzeczywistą logikę akcji.

class WritableStream implements Stream
{
    private final Stream stream;

    public WritableStream(final Stream stream)
    {
        this.stream = stream;
    }

    @Override
    public void someAction()
    {
        if(!stream.canWrite)
        {
            throw new ArgumentException();
        }
        stream.someAction();
    }
}

Teraz możemy je komponować w dowolny sposób:

final Stream myStream = new WritableStream(
    new DefaultStream()
);

Chcesz dodatkowej weryfikacji? Dodaj kolejny dekorator.

Michael
źródło
1

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.

Wayne Conrad
źródło
0

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.

class Math
    private int a;
    private int b;
    def constructor(int x, int y) 
        if(x != null)
          a = x
        else if(x < 0)
          a = abs(x)
        else if (x == -1)
          throw "Some Silly Error"
        else
          a = 0
        end
        if(y != null)
           b = y
        else if(y < 0)
           b = abs(y)
        else if(y == -1)
           throw "Some Silly Error"
        else
         b = 0
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

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ę

class Math
    private int a;
    private int b;
    def constructor(int x, int y)
        cleanX(x)
        cleanY(y)
    end
    def cleanX(int x)
        if(x != null)
          a = x
        else if(x < 0)
          a = abs(x)
        else if (x == -1)
          throw "Some Silly Error"
        else
          a = 0
        end
   end
   def cleanY(int y)
        if(y != null)
           b = y
        else if(y < 0)
           b = abs(y)
        else if(y == -1)
           throw "Some Silly Error"
        else
         b = 0
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

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

class Math
    private int a;
    private int b;
    def constructor(int x, int y)
        a = clean(x)
        b = clean(y)
    end
    def clean(int i)
        if(i != null)
          return i
        else if(i < 0)
          return abs(i)
        else if (i == -1)
          throw "Some Silly Error"
        else
          return 0
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

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.

class Math
    private int a;
    private int b;
    def constructor(int x, int y)
        a = clean(x)
        b = clean(y)
    end
    def clean(int i)
        if(i == null)
          return 0
        else if (i == -1)
          throw "Some Silly Error"
        else
          return abs(i)
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

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”.

Coteyr
źródło
-3

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.Assertlub coś podobnego w celu zmniejszenia bałaganu:

public AClassThatDefinitelyNeedsAWritableStream(Stream stream)
{
   Assert.That(stream.CanWrite, "Put crucial information here, and not inane bloat.");

   // Go on normal operation.
}
abuzittin gillifirca
źródło