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

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.

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.

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.

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.

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.

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.

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

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

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.

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.

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.

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.