Zasada jednej odpowiedzialności - czy ja ją nadużywam?

13

W celach informacyjnych - http://en.wikipedia.org/wiki/Single_responsibility_principle

Mam scenariusz testowy, w którym w jednym module aplikacji jest odpowiedzialny za tworzenie wpisów księgi. Istnieją trzy podstawowe zadania, które można wykonać -

  • Przeglądaj istniejące wpisy księgi w formacie tabeli.
  • Utwórz nowy wpis do księgi za pomocą przycisku Utwórz.
  • Kliknij pozycję księgi w tabeli (wspomnianą w pierwszym wskaźniku) i przejrzyj jej szczegóły na następnej stronie. Na tej stronie możesz anulować wpis księgi.

(Na każdej stronie jest jeszcze kilka operacji / sprawdzeń poprawności, ale dla zwięzłości ograniczę to do tych)

Postanowiłem więc stworzyć trzy różne klasy -

  • LedgerLandingPage
  • CreateNewLedgerEntryPage
  • ViewLedgerEntryPage

Klasy te oferują usługi, które można przeprowadzić na tych stronach, a testy Selenium wykorzystują te klasy, aby doprowadzić aplikację do stanu, w którym mógłbym potwierdzić.

Kiedy recenzowałem go u mojego kolegi, był już za bardzo nakręcony i poprosił mnie, abym zrobił jedną klasę dla wszystkich. Mimo że uważam, że mój projekt jest bardzo czysty, wątpię, czy nadużywam zasady pojedynczej odpowiedzialności

Tarun
źródło
2
IMHO nie ma sensu próbować ogólnie odpowiadać na twoje pytanie, nie wiedząc o dwóch rzeczach: jak duże są te klasy (według metody i LOC)? A o ile większe / bardziej złożone mają wzrosnąć w dającej się przewidzieć przyszłości?
Péter Török
2
Każda z 10 metod jest IMHO wyraźnym wskazaniem, aby nie umieszczać kodu w jednej klasie z 30 metodami.
Doc Brown
6
To terytorium graniczne: klasa 100 LOC i 10 metod nie jest zbyt mała, a jedna z 300 LOC nie jest zbyt duża. Jednak 30 metod w jednej klasie brzmi dla mnie za dużo. Ogólnie rzecz biorąc, zgadzam się z @Doc, ponieważ ryzyko posiadania wielu małych klas jest mniejsze niż jednej klasy z nadwagą. Zwłaszcza biorąc pod uwagę, że klasy naturalnie z czasem przybierają na wadze ...
Péter Török
1
@ PéterTörök - Zgadzam się, chyba że kod mógłby zostać przekształcony w jedną klasę, z której można korzystać intuicyjnie, która ponownie wykorzystuje kod zamiast duplikowania funkcjonalności, tak jak oczekuję, że OP ma.
SoylentGray
1
Być może zastosowanie MVC wszystko to wyjaśni: trzy widoki, jeden model (Ledger) i prawdopodobnie trzy kontrolery.
kevin cline

Odpowiedzi:

19

Powołując się na @YannisRizos tutaj :

Jest to podejście instynktowne i prawdopodobnie poprawne, ponieważ bardziej prawdopodobne jest, że zmiana będzie polegać na logice biznesowej zarządzania księgami. Gdyby tak się stało, o wiele łatwiej byłoby utrzymać jedną klasę niż trzy.

Nie zgadzam się, przynajmniej teraz, po około 30 latach doświadczenia w programowaniu. Mniejsze klasy IMHO lepiej utrzymać w prawie każdym przypadku, o którym myślę w takich przypadkach. Im więcej funkcji umieścisz w jednej klasie, tym mniej struktury będziesz mieć, a jakość kodu obniży się - każdego dnia nieco. Kiedy poprawnie zrozumiałem Tarun, każda z tych 3 operacji

LedgerLandingPage

CreateNewLedgerEntryPage

ViewLedgerEntryPage

jest przypadkiem użycia, każda z tych klas składa się z więcej niż jednej funkcji do wykonania powiązanego zadania, a każda z nich może być opracowana i przetestowana osobno. Tak więc tworzenie klas dla każdej z nich grupuje rzeczy, które należą do siebie, co znacznie ułatwia identyfikację części kodu, która ma zostać zmieniona, jeśli coś ma się zmienić.

Jeśli masz wspólne funkcje między tymi 3 klasami, należą one albo do wspólnej klasy bazowej, do Ledgersamej klasy (zakładam, że masz przynajmniej jedną dla operacji CRUD), albo do oddzielnej klasy pomocniczej.

Jeśli potrzebujesz więcej argumentów do stworzenia wielu mniejszych klas, polecam zajrzeć do książki Roberta Martina „Clean Code” .

Doktor Brown
źródło
to jest cały powód, dla którego wymyśliłem trzy klasy różnic zamiast przesuwać wszystko w jednej klasie. Ponieważ żadna z nich nie ma wspólnej funkcjonalności, nie mam żadnej wspólnej klasy. Dziękuję za link.
Tarun
2
„Mniejsze klasy lepiej utrzymać w prawie każdym przypadku, jaki mogę wymyślić”. Nie sądzę, że nawet Clean Code posunąłby się tak daleko. Czy naprawdę argumentowałbyś, że we wzorcu MVC powinien być na przykład jeden kontroler na stronę (lub, jak to ująłeś, przypadek użycia)? W Refactoring Fowler ma dwa zapachy kodowe: jeden odnosi się do wielu powodów zmiany klasy (SRP), a drugi do konieczności edytowania wielu klas dla jednej zmiany.
pdr
@pdr, OP stwierdza, że ​​nie ma wspólnej funkcjonalności między tymi klasami, więc (dla mnie) trudno jest wyobrazić sobie sytuację, w której trzeba dotknąć więcej niż jednej, aby dokonać jednej zmiany.
Péter Török
@pdr: Jeśli masz zbyt wiele klas do edycji dla jednej zmiany, jest to głównie znak, że nie zmieniłeś wspólnej funkcjonalności w oddzielne miejsce. Ale w powyższym przykładzie prawdopodobnie doprowadziłoby to do czwartej klasy, a nie tylko jednej.
Doc Brown
2
@pdr: BTW, czy rzeczywiście przeczytałeś „Clean Code”? Bob Martin posuwa się bardzo daleko, dzieląc kod na mniejsze jednostki.
Doc Brown
11

Nie ma ostatecznego wdrożenia zasady pojedynczej odpowiedzialności ani żadnej innej zasady. Powinieneś zdecydować na podstawie tego, co bardziej prawdopodobne jest zmiana.

Jak pisze @Karpie we wcześniejszej odpowiedzi :

Potrzebujesz tylko jednej klasy, a jedynym obowiązkiem tej klasy jest zarządzanie księgami.

Jest to podejście instynktowne i prawdopodobnie poprawne, ponieważ bardziej prawdopodobne jest, że zmiana będzie polegać na logice biznesowej zarządzania księgami. Gdyby tak się stało, o wiele łatwiej byłoby utrzymać jedną klasę niż trzy.

Ale należy to zbadać i podjąć decyzję w zależności od przypadku. Nie powinieneś naprawdę przejmować się obawami dotyczącymi drobnych możliwości zmiany i skoncentrować się na stosowaniu zasady na tym, co może się zmienić. Jeśli logika zarządzania księgami jest procesem wielowarstwowym, a warstwy są podatne na niezależne zmiany lub są obawy, które powinny być z zasady odrębne (logika i prezentacja), należy użyć oddzielnych klas. To gra prawdopodobieństwa.

Podobne pytanie na temat nadużywania zasad projektowania, które moim zdaniem może Cię zainteresować: Wzory projektowe: kiedy używać, a kiedy przestać robić wszystko przy użyciu wzorów

Yannis
źródło
1
Według mnie SRP nie jest tak naprawdę wzorcem projektowym.
Doc Brown
@DocBrown Oczywiście nie jest to wzorzec projektowy, ale dyskusja na ten temat jest niezwykle istotna ... Dobry chwyt, zaktualizowałem odpowiedź
yannis
4

Przeanalizujmy te klasy:

  • LedgerLandingPage: Rolą tej klasy jest pokazanie listy ksiąg. W miarę rozwoju aplikacji prawdopodobnie będziesz chciał dodać metody sortowania, filtrowania, wyróżniania i przezroczystego łączenia danych z innych źródeł.

  • ViewLedgerEntryPage: Ta strona pokazuje szczegółowo księgę. Wydaje się całkiem proste. Tak długo, jak dane są proste. Tutaj możesz anulować księgę. Możesz także chcieć to poprawić. Lub skomentuj, załącz plik, paragon lub zewnętrzny numer rezerwacji lub cokolwiek innego. A kiedy zezwolisz na edycję, zdecydowanie chcesz pokazać historię.

  • CreateLedgerEntryPage: Jeśli ViewLedgerEntryPagema pełną możliwość edycji, odpowiedzialność tej klasy można spełnić, edytując „pusty wpis”, co może, ale nie musi mieć sensu.

Te przykłady mogą wydawać się trochę przesadne, ale chodzi o to, że z UX mówimy tutaj o funkcjach. Pojedyncza cecha jest zdecydowanie odrębną, pojedynczą odpowiedzialnością. Ponieważ wymagania dotyczące tej funkcji mogą się zmieniać, a wymagania dwóch różnych funkcji mogą zmieniać się w dwóch przeciwnych kierunkach, a wtedy żałujesz, że nie trzymałeś się SRP od samego początku.
Ale odwrotnie, zawsze możesz przykleić fasadę do trzech klas, jeśli posiadanie jednego interfejsu jest wygodniejsze z dowolnego powodu, o którym możesz pomyśleć.


Istnieje nadużycie SRP : jeśli potrzebujesz kilkunastu klas do odczytania zawartości pliku, raczej bezpiecznie jest założyć, że właśnie to robisz.

Jeśli spojrzysz na SRP z drugiej strony, to w rzeczywistości mówi, że jedną klasę powinna przejąć jedna odpowiedzialność. Należy jednak pamiętać, że obowiązki istnieją tylko w ramach poziomów abstrakcji. Dlatego ma sens, aby klasa z wyższego poziomu abstrakcji wypełniała swoją odpowiedzialność, delegując pracę do komponentu niższego poziomu, co najlepiej osiągnąć poprzez odwrócenie zależności .

back2dos
źródło
Myślę, że nawet ja nie byłbym w stanie lepiej opisać odpowiedzialności tych klas niż ty.
Tarun
2

Czy te klasy mają tylko jeden powód do zmiany?

Jeśli jeszcze tego nie wiesz, być może wkroczyłeś w pułapkę projektowania spekulatywnego / ponad inżynierską (zastosowano zbyt wiele klas, zastosowano zbyt złożone wzorce projektowe). Nie jesteś zadowolony z YAGNI . Na wczesnych etapach cyklu życia oprogramowania (niektóre) wymagania mogą być niejasne. Dlatego kierunek zmian nie jest obecnie dla ciebie widoczny. Jeśli zdajesz sobie z tego sprawę, przechowuj go w jednej lub minimalnej liczbie klas. Jednak wiąże się to z odpowiedzialnością: gdy tylko wymagania i kierunek zmiany są jaśniejsze, należy ponownie przemyśleć obecny projekt i dokonać refaktoryzacji, aby zaspokoić przyczynę zmiany.

Jeśli już wiesz, że istnieją trzy różne powody zmiany i że są one przypisane do tych trzech klas, to właśnie zastosowałeś odpowiednią dawkę SRP. Ponownie wiąże się to z pewną odpowiedzialnością: jeśli się mylisz lub przyszłe wymagania zmieniają się nieoczekiwanie, musisz przemyśleć obecny projekt i dokonać refaktoryzacji, aby zaspokoić przyczynę zmiany.

Chodzi o to:

  • Miej oko na sterowniki do zmian.
  • Wybrał elastyczny projekt, który można refaktoryzować.
  • Przygotuj się na zmianę kodu.

Jeśli masz taki sposób myślenia, będziesz kształtować kod bez większego strachu przed spekulacyjnym projektowaniem / nad inżynierią.

Jednak zawsze jest czynnik czasowy: dla wielu zmian nie będziesz miał czasu, którego potrzebujesz. Refaktoryzacja kosztuje czas i wysiłek. Często napotykałem sytuacje, w których wiedziałem, że muszę zmienić projekt, ale z powodu ograniczeń czasowych musiałem go odłożyć. Stanie się tak i nie można tego uniknąć. Odkryłem, że łatwiej jest refaktoryzować kod inżynierii niż kod inżynierii. YAGNI daje mi dobrą wskazówkę: w razie wątpliwości należy ograniczyć liczbę klas i stosowanie wzorców projektowych do minimum.

Theo Lenndorff
źródło
1

Istnieją trzy powody, aby wywoływać SRP jako powód do podziału klasy:

  • dzięki czemu przyszłe zmiany wymagań mogą pozostawić niektóre klasy bez zmian
  • aby błąd można było szybciej usunąć
  • aby zmniejszyć klasę

Istnieją trzy powody, aby odmówić podziału klasy:

  • dzięki czemu przyszłe zmiany wymagań nie spowodują wprowadzenia tej samej zmiany w wielu klasach
  • tak, aby wyszukiwanie błędów nie wymagało śledzenia długich ścieżek pośrednich
  • opierać się technikom przełamywania enkapsulacji (właściwości, znajomi itp.), które ujawniają wszystkim informacje lub metody, gdy są one potrzebne tylko jednej pokrewnej klasie

Kiedy patrzę na twoją sprawę i wyobrażam sobie zmiany, niektóre z nich spowodują zmiany w wielu klasach (np. Dodaj pole do księgi, będziesz musiał zmienić wszystkie trzy), ale wiele nie - na przykład dodając sortowanie do listy ksiąg lub zmieniając reguły sprawdzania poprawności podczas dodawania nowego wpisu do księgi. Reakcja twojego kolegi jest prawdopodobnie spowodowana tym, że trudno jest wiedzieć, gdzie szukać błędu. Jeśli coś wygląda źle na liście ksiąg, czy to dlatego, że zostało źle dodane, czy coś jest nie tak z listą? Jeśli istnieje rozbieżność między podsumowaniem a szczegółami, co jest nie tak?

Możliwe jest również, że twój kolega uważa, że ​​zmiany wymagań, które oznaczają, że będziesz musiał zmienić wszystkie trzy klasy, są znacznie bardziej prawdopodobne niż zmiany, w których można uniknąć zmiany tylko jednej. To może być naprawdę przydatna rozmowa.

Kate Gregory
źródło
dobrze widzieć inną perspektywę
Tarun,
0

Myślę, że jeśli księga była tabelą w db, sugeruję przypisanie tych zadań do jednej klasy (np. DAO). Jeśli księga ma więcej logiki i nie była tabelą w db, sugeruję, że powinniśmy stworzyć więcej klas, aby wykonać te zadania (może to być klasa dwu lub trzyosobowa) i zapewnić klasę elewacji, którą można wykonać tylko dla klienta.

Mark xie
źródło
Cóż, księga jest funkcją interfejsu użytkownika i istnieje wiele powiązanych z nią operacji, które można wykonać z różnych stron.
Tarun,
0

IMHO nie powinieneś w ogóle korzystać z zajęć. Nie ma tu żadnych abstrakcji danych. Księgi rachunkowe są konkretnym typem danych. Metodami manipulacji i wyświetlania są funkcje i procedury. Z pewnością będzie wiele powszechnie używanych funkcji, takich jak formatowanie daty. Niewiele jest miejsca na wyodrębnianie i ukrywanie danych w klasie w procedurach wyświetlania i wyboru.

Istnieje pewien przypadek ukrycia stanu w klasie do edycji księgi.

Niezależnie od tego, czy nadużywasz SRP, nadużywasz czegoś bardziej fundamentalnego: nadużywasz abstrakcji. Jest to typowy problem wynikający z mitycznego pojęcia, że ​​OO jest rozsądnym paradygmatem rozwoju.

Programowanie jest w 90% konkretne: wykorzystanie abstrakcji danych powinno być rzadkie i starannie zaprojektowane. Z drugiej strony abstrakcja funkcjonalna powinna być bardziej powszechna, ponieważ szczegóły języka i wybór algorytmów należy oddzielić od semantyki operacji.

Yttrill
źródło
-1

Potrzebujesz tylko jednej klasy, a jedynym obowiązkiem tej klasy jest zarządzanie księgami .

sevenseacat
źródło
2
Dla mnie klasa „... menedżera” zwykle pokazuje, że nie było problemu z dalszym podziałem. Czym zarządza klasa? Prezentacja, upór?
sebastiangeiger
-1. Umieszczenie 3 lub więcej przypadków użycia w 1 klasie jest dokładnie tym, czego SRP próbuje uniknąć - i nie bez powodu.
Doc Brown
@sebastiangeiger Co robią trzy klasy PO? Prezentacja czy wytrwałość?
sevenseacat
@Karpie Mam szczerą nadzieję na prezentację. To prawda, że ​​przykład nie jest zbyt dobry, ale spodziewałbym się, że „... Strona” na stronie robi coś podobnego do widoku.
sebastiangeiger
2
W rzeczywistości potrzebujesz tylko jednej klasy dla całej aplikacji, z pojedynczą odpowiedzialnością za zadowolenie użytkowników ;-)
Péter Török