Czy ten projekt klasy narusza zasadę pojedynczej odpowiedzialności?

63

Dzisiaj pokłóciłem się z kimś.

Wyjaśniałem zalety posiadania bogatego modelu domeny w przeciwieństwie do anemicznego modelu domeny. I pokazałem swój punkt w prostej klasie wyglądającej tak:

public class Employee
{
    public Employee(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastname;
    }

    public string FirstName { get private set; }
    public string LastName { get; private set;}
    public int CountPaidDaysOffGranted { get; private set;}

    public void AddPaidDaysOffGranted(int numberOfdays)
    {
        // Do stuff
    }
}

Kiedy bronił swojego modelu anemicznego, jednym z jego argumentów było: „Jestem zwolennikiem SOLID . Naruszasz zasadę pojedynczej odpowiedzialności (SRP), ponieważ reprezentujesz dane i logikę w tej samej klasie”.

Uznałem to twierdzenie za naprawdę zaskakujące, ponieważ zgodnie z tym rozumowaniem każda klasa posiadająca jedną właściwość i jedną metodę narusza SRP, a zatem OOP ogólnie nie jest SOLIDNY, a programowanie funkcjonalne jest jedyną drogą do nieba.

Postanowiłem nie odpowiadać na jego liczne argumenty, ale jestem ciekaw, co społeczność sądzi na to pytanie.

Gdybym odpowiedział, zacznę od wskazania wyżej wspomnianego paradoksu, a następnie wskazania, że ​​SRP jest wysoce zależny od poziomu szczegółowości, który chcesz wziąć pod uwagę, i jeśli posuniesz się wystarczająco daleko, każda klasa zawierająca więcej niż jeden właściwość lub jedna metoda narusza ją.

Co byś powiedział

Aktualizacja: przykład został hojnie zaktualizowany przez Guntberta, aby uczynić metodę bardziej realistyczną i pomóc nam skoncentrować się na podstawowej dyskusji.

tobiak777
źródło
19
klasa ta narusza SRP nie dlatego, że miesza dane z logiką, ale dlatego, że ma niską spójność - potencjalny obiekt Boga
gnat
1
Jaki jest cel dodawania urlopu pracownikowi? Powinna być może lekcja kalendarza lub coś, co ma święta. Myślę, że twój przyjaciel ma rację.
James Black
9
Nigdy nie słuchaj kogoś, kto mówi: „Wierzę w X”.
Stig Hemmer
29
Nie chodzi o to, czy jest to naruszenie SRP, ale o to, czy w ogóle jest to dobre modelowanie. Załóżmy, że jestem pracownikiem. Kiedy pytam swojego kierownika, czy jest z nim w porządku, jeśli jeżdżę na narty przez długi weekend, nie dodaje mi wakacji . Kod tutaj nie pasuje do rzeczywistości, którą zamierza modelować, i dlatego jest podejrzany.
Eric Lippert,
2
+1 za programowanie funkcjonalne jest jedyną drogą do nieba.
erip

Odpowiedzi:

68

Pojedyncza odpowiedzialność powinna być rozumiana jako abstrakcja logicznych zadań w twoim systemie. Klasa powinna ponosić wyłączną odpowiedzialność (wykonać wszystko, co konieczne, aby) wykonać jedno, konkretne zadanie. To może w rzeczywistości wiele przynieść do dobrze zaprojektowanej klasy, w zależności od odpowiedzialności. Na przykład klasa, która uruchamia silnik skryptów, może mieć wiele metod i danych związanych z przetwarzaniem skryptów.

Twój współpracownik koncentruje się na złej rzeczy. Pytanie nie brzmi „jacy członkowie ma ta klasa?” ale „jakie użyteczne operacje wykonuje ta klasa w programie?” Po zrozumieniu model domeny wygląda dobrze.

Mason Wheeler
źródło
Jeśli programowanie obiektowe zostało stworzone i ma na celu modelowanie rzeczywistych obiektów i klas (akcje + atrybuty), to dlaczego nie napisać klasy kodu mającej wiele obowiązków (akcji)? Obiekt świata rzeczywistego może mieć wiele obowiązków. Na przykład dziennikarz pisze artykuły w gazetach i udziela wywiadów politykom w programie telewizyjnym. Dwie obowiązki za obiekt z prawdziwego życia! Co jeśli zamierzam napisać dziennikarza klasowego?
user1451111
41

Zasada pojedynczej odpowiedzialności dotyczy tylko tego, czy fragment kodu (w OOP, zwykle mówimy o klasach) odpowiada za jeden element funkcjonalności . Myślę, że twój przyjaciel mówiąc, że funkcje i dane nie mogą się łączyć, tak naprawdę nie zrozumiał tego pomysłu. Gdyby Employeezawierały także informacje o jego miejscu pracy, prędkości samochodu i jakie pokarmy je pies, mielibyśmy problem.

Ponieważ ta klasa zajmuje się tylko i Employee, myślę, że można uczciwie powiedzieć, że nie narusza rażąco SRP, ale ludzie zawsze będą mieli własne opinie.

Jednym z miejsc, w których możemy poprawić jest oddzielenie informacji o pracowniku (takich jak imię i nazwisko, numer telefonu, adres e-mail) od jego wakacji. Moim zdaniem nie oznacza to, że metody i dane nie mogą się ze sobą łączyć , to po prostu oznacza, że ​​być może funkcjonalność wakacyjna może znajdować się w osobnym miejscu.

Frank Bryce
źródło
20

Moim zdaniem ta klasa mogłaby potencjalnie naruszyć SRP, gdyby nadal reprezentowała zarówno an, jak Employeei EmployeeHolidays.

W tej chwili i jeśli przyszedłby do mnie do recenzji, prawdopodobnie przepuściłbym to. Gdyby dodano więcej właściwości i metod specyficznych dla Pracownika oraz dodano więcej właściwości specyficznych dla świąt, prawdopodobnie zaleciłbym podział, powołując się zarówno na SRP, jak i ISP.

NikolaiDante
źródło
Zgadzam się. Jeśli kod jest tak prosty, jak tutaj podany, prawdopodobnie pozwoliłbym mu się przesuwać. Ale moim zdaniem nie powinienem być obowiązkiem pracownika do obsługi własnego urlopu. To może nie wydawać się wielkim problemem z tym, gdzie spoczywa odpowiedzialność, ale spójrz na to w ten sposób: Jeśli byłeś nowy w bazie kodu i musiałeś pracować nad funkcjami związanymi z wakacjami - gdzie byś pierwszy wyglądał? Jeśli chodzi o logikę urlopową, osobiście NIE patrzyłbym na podmiot pracownika na początek.
Niklas H
1
@NiklasH Zgoda. Choć osobiście nie wyglądałbym przypadkowo i nie odgadłbym klasy, szukałem w studiu słowa „Holiday” i sprawdziłem, w jakich klasach się pojawił. :)
NikolaiDante
4
Prawdziwe. Ale co, jeśli w tym nowym systemie nie będzie się nazywać „Wakacje”, ale „Wakacje” lub „Czas wolny”. Ale zgadzam się z tym, że zazwyczaj masz możliwość po prostu go wyszukać lub możesz poprosić współpracownika. Moim komentarzem było przede wszystkim nakłonienie PO do mentalnego modelowania odpowiedzialności i gdzie najbardziej oczywistym miejscem dla logiki byłoby :-)
Niklas H
Zgadzam się z twoim pierwszym oświadczeniem. Jeśli jednak przyjdzie do przeglądu, nie sądzę, żebym tak zrobił, ponieważ naruszenie SRP jest śliskie nachylenie i może to być pierwsze z wielu uszkodzonych okien. Twoje zdrowie.
Jim Speaker
20

Istnieją już świetne odpowiedzi wskazujące, że SRP to abstrakcyjna koncepcja logicznej funkcjonalności, ale są subtelniejsze kwestie, które moim zdaniem są warte dodania.

Pierwsze dwie litery w SOLID, SRP i OCP dotyczą zmiany kodu w odpowiedzi na zmianę wymagań. Moja ulubiona definicja SRP to: „moduł / klasa / funkcja powinna mieć tylko jeden powód do zmiany”. Argumentowanie o prawdopodobnych przyczynach zmiany kodu jest znacznie bardziej produktywne niż argumentowanie, czy kod jest SOLIDNY, czy nie.

Ile powodów musi zmienić Twoja klasa pracownika? Nie wiem, ponieważ nie znam kontekstu, w którym go używasz, a także nie widzę przyszłości. Mogę zrobić burzę mózgów, możliwe zmiany w oparciu o to, co widziałem w przeszłości, i możesz subiektywnie ocenić prawdopodobieństwo ich wystąpienia. Jeśli więcej niż jeden wynik między „uzasadnionym prawdopodobieństwem” a „moim kodem już się zmienił z tego właśnie powodu”, to łamiesz SRP przeciwko takiej zmianie. Oto jeden: ktoś o więcej niż dwóch nazwiskach dołącza do Twojej firmy (lub architekt czyta ten doskonały artykuł W3C ). Oto kolejna: Twoja firma zmienia sposób przydzielania dni świątecznych.

Zauważ, że te powody są jednakowo ważne, nawet jeśli usuniesz metodę AddHolidays. Wiele anemicznych modeli domen narusza SRP. Wiele z nich to po prostu tabele bazy danych w kodzie i bardzo często tabele bazy danych mają ponad 20 powodów do zmiany.

Oto coś do przeżuwania: czy klasa Pracowników zmieniłaby się, gdyby Twój system musiał śledzić pensje pracowników? Adresy Dane kontaktowe w nagłych wypadkach? Jeśli powiesz „tak” (i „prawdopodobnie się zdarzy”) dwóm z nich, wtedy twoja klasa naruszy SRP, nawet jeśli nie będzie w nim jeszcze kodu! SOLID dotyczy zarówno procesów i architektury, jak i kodu.

Carl Leth
źródło
9

To, że klasa reprezentuje dane, nie jest obowiązkiem klasy, jest to prywatny szczegół implementacji.

Klasa ma jeden obowiązek reprezentowania pracownika. W tym kontekście oznacza to, że przedstawia niektóre publiczne interfejsy API, które zapewniają funkcjonalność, z którą musisz radzić sobie w kontaktach z pracownikami (czy dobrym przykładem jest AddHolidays, jest dyskusyjne).

Wdrożenie jest wewnętrzne; zdarza się, że wymaga to pewnych zmiennych prywatnych i logiki. To nie znaczy, że klasa ma teraz wiele obowiązków.

RemcoGerlich
źródło
Ciekawa myślenie, wielkie dzięki za udostępnienie
tobiak777
Nicea - dobry sposób na wyrażenie zamierzonych celów OOP.
user949300,
5

Pomysł, że mieszanie logiki i danych w jakikolwiek sposób jest zawsze błędny, jest tak absurdalny, że nawet nie zasługuje na dyskusję. Jednak w tym przykładzie rzeczywiście występuje wyraźne naruszenie pojedynczej odpowiedzialności, ale nie dlatego, że istnieje właściwość DaysOfHolidaysi funkcja AddHolidays(int).

To dlatego, że tożsamość pracownika jest wmieszana w zarządzanie urlopem, co jest złe. Tożsamość pracownika to złożona rzecz, która jest wymagana do śledzenia urlopu, wynagrodzenia, nadgodzin, reprezentowania tego, kto jest w zespole, łączenia się z raportami wydajności itp. Pracownik może również zmienić imię, nazwisko lub jedno i drugie, i pozostać niezmienionym pracownik. Pracownicy mogą mieć nawet wiele pisowni swoich nazwisk, takich jak ASCII i pisownia Unicode. Ludzie mogą mieć od 0 do n imion i / lub nazwisk. Mogą mieć różne nazwy w różnych jurysdykcjach. Śledzenie tożsamości pracownika jest wystarczającą odpowiedzialnością, że zarządzanie urlopami lub urlopami nie może być dodane bez nazywania go drugą odpowiedzialnością.

Piotr
źródło
„Śledzenie tożsamości pracownika jest wystarczającą odpowiedzialnością, że zarządzanie urlopem lub urlopem nie może być dodane bez nazywania go drugą odpowiedzialnością”. + Pracownik może mieć kilka nazwisk itp. Celem modelu jest skupienie się na istotnych faktach z rzeczywistego świata dla danego problemu. Istnieją wymagania, dla których ten model jest optymalny. W tych wymaganiach pracownicy są interesujący tylko w takim stopniu, w jakim możemy modyfikować ich wakacje, a my nie jesteśmy zbytnio zainteresowani zarządzaniem innymi aspektami ich rzeczywistych warunków.
tobiak777
@reddy „Pracownicy są interesujący tylko w zakresie, w jakim możemy modyfikować ich święta” - co oznacza, że ​​musisz je poprawnie zidentyfikować. Gdy tylko masz pracownika, może on zmienić nazwisko w dowolnym momencie z powodu małżeństwa lub rozwodu. Mogą również zmienić swoje imię i płeć. Czy zwolnisz pracowników, jeśli ich nazwisko zmieni się tak, że ich nazwisko będzie zgodne z nazwiskiem innego pracownika? Nie dodasz teraz wszystkich tych funkcji. Zamiast tego dodasz go, gdy będziesz go potrzebować, co jest dobre. Niezależnie od tego, ile jest wdrożonych, odpowiedzialność za identyfikację pozostaje taka sama.
Peter
3

„Jestem zwolennikiem SOLID. Naruszasz zasadę pojedynczej odpowiedzialności (SRP), ponieważ reprezentujesz dane i logikę w tej samej klasie”.

Podobnie jak inni, nie zgadzam się z tym.

Powiedziałbym, że SRP jest naruszone, jeśli wykonujesz więcej niż jedną logikę w klasie. Nie ma znaczenia, ile danych należy przechowywać w klasie, aby osiągnąć ten pojedynczy element logiki.

Lekkość Wyścigi na orbicie
źródło
Nie! SRP nie jest naruszane przez wiele elementów logicznych, wiele elementów danych ani żadną ich kombinację. Jedynym wymaganiem jest to, że obiekt powinien trzymać się swojego celu. Jego celem może potencjalnie być wiele operacji.
Martin Maat,
@MartinMaat: Tak, wiele operacji. W rezultacie jedna logika. Myślę, że mówimy to samo, ale z innymi terminami (i cieszę się, że twoje są poprawne, ponieważ nie studiuję tego często)
Lightness Races in Orbit
2

Nie uważam za użyteczne debatowanie na temat tego, co stanowi i nie stanowi ani jednej odpowiedzialności, ani jednego powodu do zmiany. Zamiast tego zaproponowałbym zasadę minimalnego smutku:

Zasada minimalnego żalu: kod powinien dążyć do zminimalizowania prawdopodobieństwa wymaganej zmiany lub zmaksymalizować łatwość zmiany.

Jak to? Nie powinien zająć się naukowcem zajmującym się rakietami, aby dowiedzieć się, dlaczego może to pomóc w obniżeniu kosztów utrzymania, i mam nadzieję, że nie powinno to być punktem niekończącej się debaty, ale jak w przypadku SOLID ogólnie, nie jest to coś, co można stosować na ślepo. Jest to coś do rozważenia podczas równoważenia kompromisów.

Jeśli chodzi o prawdopodobieństwo wymagania zmian, spada to:

  1. Dobre testy (poprawiona niezawodność).
  2. Zaangażowanie tylko minimalnego kodu wymaganego do zrobienia czegoś konkretnego (może to obejmować zmniejszenie sprzężeń aferentnych).
  3. Po prostu uczynienie kodu niepoprawnym w tym, co robi (zobacz Make Badass Principle).

Jeśli chodzi o trudność wprowadzania zmian, idzie w górę z połączeniami odprowadzającymi. Testowanie wprowadza sprzężenie odprowadzające, ale poprawia niezawodność. Zrobione dobrze, ogólnie przynosi więcej korzyści niż szkody i jest całkowicie akceptowalne i promowane przez zasadę minimalnego smutku.

Zrób zasadę Badassa: klasy używane w wielu miejscach powinny być niesamowite. Powinny być niezawodne, wydajne, jeśli wiąże się to z ich jakością itp.

A zasada Make Badass jest powiązana z zasadą minimalnego smutku, ponieważ rzeczy badass będą miały mniejsze prawdopodobieństwo potrzeby zmian niż rzeczy, które są do kitu.

Zacząłbym od wskazania wyżej wspomnianego paradoksu, a następnie wskazania, że ​​SRP jest wysoce zależny od poziomu szczegółowości, który chcesz wziąć pod uwagę, i jeśli posuniesz się wystarczająco daleko, każda klasa zawierająca więcej niż jedną właściwość lub jedną metodę narusza to.

Z punktu widzenia SRP klasa, która prawie nic nie robi, z pewnością miałaby tylko jeden (czasem zerowy) powód do zmiany:

class Float
{
public:
    explicit Float(float val);
    float get() const;
    void set(float new_val);
};

To praktycznie nie ma powodów do zmiany! To lepsze niż SRP. To ZRP!

Poza tym sugerowałbym, że jest to rażące naruszenie zasady Make Badass. Jest to również absolutnie bezwartościowe. Coś, co robi tak niewiele, nie może mieć nadziei na bycie poważnym. Ma za mało informacji (TLI). I oczywiście, kiedy masz coś, co jest TLI, nie może zrobić nic naprawdę znaczącego, nawet z zawartymi w nim informacjami, więc musi wyciekać to do świata zewnętrznego w nadziei, że ktoś inny zrobi coś znaczącego i złego. I ta nieszczelność jest dobra w przypadku czegoś, co ma na celu jedynie agregację danych i nic więcej, ale ten próg jest różnicą, jak widzę, między „danymi” a „obiektami”.

Oczywiście coś, co jest TMI, jest również złe. Możemy umieścić całe nasze oprogramowanie w jednej klasie. Może nawet mieć tylko jedną runmetodę. I ktoś może nawet argumentować, że ma teraz jeden bardzo szeroki powód do zmiany: „Ta klasa będzie musiała zostać zmieniona tylko wtedy, gdy oprogramowanie wymaga poprawy”. Jestem głupi, ale oczywiście możemy sobie wyobrazić wszystkie problemy z konserwacją.

Istnieje więc równowaga co do ziarnistości lub szorstkości projektowanych obiektów. Często oceniam to na podstawie ilości informacji, które musisz wyciec do świata zewnętrznego i tego, jak wiele znaczących funkcji może wykonać. Często uznaję zasadę Bad Badass za pomocną w znalezieniu równowagi, łącząc ją z zasadą minimalnego smutku.


źródło
1

Przeciwnie, dla mnie Anemiczny model domeny łamie niektóre główne koncepcje OOP (które łączą atrybuty i zachowanie), ale może być nieunikniony w oparciu o wybory architektoniczne. Domeny anemiczne są łatwiejsze do myślenia, mniej organiczne i bardziej sekwencyjne.

Wiele systemów robi to zwykle wtedy, gdy wiele warstw musi grać z tymi samymi danymi (warstwa usługi, warstwa internetowa, warstwa klienta, agenci ...).

Łatwiej jest zdefiniować strukturę danych w jednym miejscu i zachowanie w innych klasach usług. Jeśli ta sama klasa została użyta na wielu warstwach, ta może się urosnąć i zadaje pytanie, która warstwa jest odpowiedzialna za określenie zachowania, którego potrzebuje, i kto może wywoływać metody.

Na przykład może nie być dobrym pomysłem niż proces agenta, który oblicza statystyki dotyczące wszystkich pracowników i może wywoływać obliczenia dla dni płatnych. A GUI listy pracowników z pewnością nie potrzebuje wcale nowego obliczenia zagregowanego identyfikatora stosowanego w tym agencie statystycznym (i danych technicznych z nim związanych). Kiedy segregujesz metody w ten sposób, zazwyczaj kończysz się klasą zawierającą tylko struktury danych.

Możesz zbyt łatwo serializować / deserializować dane „obiektowe”, lub tylko niektóre z nich, lub do innego formatu (json) ... bez obawy o jakąkolwiek koncepcję / odpowiedzialność za obiekt. To tylko przekazywanie danych. Zawsze możesz wykonać mapowanie danych między dwiema klasami (Pracownik, PracownikV, PracownikStatystyczny…), ale co tak naprawdę oznacza Pracownik?

Tak, tak, całkowicie oddziela dane w klasach domen i przetwarzanie danych w klasach usług, ale jest tutaj potrzebne. Taki system jest jednocześnie funkcjonalny, aby wnosić wartość biznesową, a także techniczny, aby propagować dane wszędzie tam, gdzie są potrzebne, zachowując jednocześnie odpowiedni zakres odpowiedzialności (a rozproszony obiekt również tego nie rozwiązuje).

Jeśli nie musisz rozdzielać zakresów zachowań, możesz swobodnie umieszczać metody w klasach usług lub klasach domen, w zależności od tego, jak widzisz swój obiekt. Zwykle postrzegam przedmiot jako „prawdziwą” koncepcję, która w naturalny sposób pomaga utrzymać SRP. Więc w twoim przykładzie jest to bardziej realistyczne niż szef pracownika dodający wypłatę przyznaną na jego konto PayDay. Pracownik jest zatrudniony przez firmę, działa, może być chory lub zostać poproszony o poradę, a on ma konto Payday (szef może je odzyskać bezpośrednio od niego lub z rejestru PayDayAccount ...) Ale możesz utworzyć skrót zbiorczy tutaj dla uproszczenia, jeśli nie chcesz zbyt dużej złożoności prostego oprogramowania.

Vince
źródło
Dziękuję za Twój wkład, Vince. Chodzi o to, że nie potrzebujesz warstwy usługi, gdy masz bogatą domenę. Jest tylko jedna klasa odpowiedzialna za zachowanie i jest to twoja jednostka domeny. Pozostałe warstwy (warstwa internetowa, interfejs użytkownika itp.) Zazwyczaj zajmują się DTO i ViewModels i jest w porządku. Model domeny służy do modelowania domeny, a nie do działania interfejsu użytkownika lub wysyłania wiadomości przez Internet. Twoje przesłanie odzwierciedla to powszechne nieporozumienie, które wynika z faktu, że ludzie po prostu nie wiedzą, jak dopasować OOP do swojego projektu. I myślę, że to bardzo smutne - dla nich.
tobiak777
Nie sądzę, że mam błędne wyobrażenie o bogatej domenie OOP, w ten sposób zaprojektowałem wiele sotwares i to prawda, że ​​jest naprawdę dobra do konserwacji / ewolucji. Ale przykro mi powiedzieć, że to nie jest srebrna kula.
Vince
Nie mówię, że tak. W przypadku pisania kompilatora prawdopodobnie tak nie jest, ale myślę, że w przypadku większości aplikacji biznesowych / SaaS jest to znacznie mniej sztuki / nauki niż to, co sugerujesz. Potrzeba modelu domeny może być udowodniona matematycznie, twoje przykłady sprawiają, że myślę raczej o dyskusyjnych projektach niż wadach w OOP
tobiak777
0

Naruszasz zasadę pojedynczej odpowiedzialności (SRP), ponieważ reprezentujesz dane i logikę w tej samej klasie.

Brzmi dla mnie bardzo rozsądnie. Model może nie mieć właściwości publicznych, jeśli udostępnia działania. Zasadniczo jest to idea separacji poleceń i zapytań. Pamiętaj, że Command na pewno będzie miał stan prywatny.

Dmitry Nogin
źródło
0

Nie możesz naruszać zasady pojedynczej odpowiedzialności, ponieważ jest to jedynie kryterium estetyczne, a nie reguła natury. Nie daj się zwieść naukowo brzmiącej nazwie i dużym literom.

ZunTzu
źródło
1
To nie jest prawdziwa odpowiedź, ale byłby świetny jako komentarz do pytania.
Jay Elston,