19 lut 2009

SOLID - SRP część 2 - klasy

Ostatnio pisałem o Regule Pojedynczej Odpowiedzialności skupiając się na metodach. Ale ta reguła dotyczy w równym stopniu strukturyzowania klas. Tym razem będzie mniej kodu, bo skupię się tylko na sygnaturach pokazując o co chodzi.

Reguła Pojedynczej Odpowiedzialności brzmi w tym przypadku tak: powinien być dokładnie jeden powód do dokonywania zmian w klasie. Oznacza to, że kod powinien mieć taką strukturę, żeby każda klasa miała dokładnie jedną odpowiedzialność: np. wyświetlanie komponentu, komunikacja z bazą danych, logika biznesowa.

Najbardziej klasycznym przykładem zastosowania tej reguły jest wzorzec MVC. Oddzielenie od siebie danych, mechanizmu wyświetlania i obsługa operacji UI.

To przecież jasne! Każdy to wie, nie?

No to zobaczmy czy napewno. Mamy sobie taką klasę:

@Stateless
@Local
public class UserManagerBean {

@PersistenceContext(type = PersistenceContextType.TRANSACTION)
private EntityManager em;
private LoginManager loginManager;

public User findUserByName(String name) {
return (User) em.createQuery("from User u where u.name=:name")
.setParameter("name", name).getSingleResult();
}

public boolean isUserValid(User user) {
if (user.getNumWrongLogins() > 3
&& (System.currentTimeMillis() - user.getLastLoginTrial() < 5000))
user.block();

if (user.isBlocked() || user.isAccountExpired())
return false;
return true;
}

public String login(User user) {
boolean result = loginManager.login(user.getName(), user.getPassword());
if (result)
return "You logged in successfully!";
return "Logging in failed";
}
}

Ten kod na pierwszy rzut oka wygląda ok. Gdyby to był poprzedni wpis, przyczepiłbym się tylko metody isUserValid, bo robi dwie rzeczy zamiast jednej (weryfikuje czy blokować Usera i ew. blokuje, oraz sprawdza czy User może się logować). Ale z punktu widzenia Reguły Pojedynczej Odpowiedzialności klas ten kod jest zupełnie do niczego... a dokładniej to do wszystkiego :)  (jak z resztą większość EJB na tym świecie, ponieważ ten model promuje takie programowanie). Ta klasa w jednej metodzie komunikuje się z bazą danych, w drugiej zawiera logikę, w trzeciej zwraca tekst do wyświetlenia w UI. Brrrrr. Straszne.

W przykładzie klasy, która ma 35 linii może jeszcze to tak nie kłuje w oczy, ale taka klasa mogła by mieć 30 różnych metod i wtedy przestaje być jasne za co taka klasa jest odpowiedzialna, co gdzie należy zmienić jeśli chce się wprowadzić nową funkcjonalność lub zmodyfikować istniejącą. Przy dużej liczbie takich komponentów aplikacja przestaje być utrzymywalna. Odpowiedzialności mieszają się ze sobą, obiekty wołają nawzajem różne swoje metody tworząc sieć zależności i oczekiwań. W w dużych aplikacjach ze skomplikowanymi procesami brak poprawnego i dokładnego rozdzielenia odpowiedzialności powoduje, że zmiany w jednej metodzie powodują często powstawanie błędów w wielu innych miejscach aplikacji.

Jak więc należało by ten problem rozwiązać? Ja proponowałbym kod bazodanowy wyprowadzić do odpowiedniego modułu/warstwy (DAO, repozytorium, etc.) a dostęp do niego zrealizować np. jako metodę samego obiektu User (np. a'la Grails'owy GORM: User.findByName(name)). Weryfikacja poprawności użytkownika albo do obiektu walidacji, albo do klasy User (jako User.isValid()). Kod zwracający treść do wyświetlenia przerzucić na stronę wyświetlającą (albo jako tag czy fragment) a samo wywołanie logowania (linia wołająca obiekt loginManager) jest już odpowiednie dla kontrolera UI. 
I w ten sposób... nie ma już więcej UserManagerBean'a :) (tak, to taki sprytny sposób podprogowego przekazania myśli, że EJB nie powinny być wykorzystywane do metod które nie korzystają bezpośrednio z transakcyjności, bezpieczeństwa czy remotingu)
Jeśli będzie zmiana w strukturze danych w bazie albo zmieni się wymaganie co do zapytania zwracającego użytkownika, jest jasne gdzie tego szukać - w klasie reprezentującej repozytorium obiektów User. Jeśli będziemy chcieli dodać i18n, będzie to w miejscu odpowiedzialnym za wyświetlanie. Jeśli będzie trzeba zwiększyć dostępną liczbę nieudanych prób logowania Użytkownika, to też łatwo będzie ustalić gdzie jest odpowiedni kod.

Przy definiowaniu przynależności metod do klas można wykorzystać heurystykę częstotliwości odwołań do różnych klas w ramach metody. Jeśli w jakiejś metodzie większość wywołań metod dotyczy jednej klasy, to z dużym prawdopodobieństwem ta metoda powinna w tej klasie się znaleźć (zmniejszając ilość kodu o odwołania do obiektu i poprawiając jego logiczne rozłożenie).
Jeśli dbamy o to, by metody były krótkie, tych odwołań nie będzie dużo i łatwiej o dobry (sprawny) design, stąd jeszcze jeden powód do pilnowania pojedynczej odpowiedzialności metod. W ten sposób dużo łatwiej jest osiągnąć Święty Graal projektowania obiektowego: High Cohesion with Loose Coupling

Choć w sumie Zasada Pojedynczej Odpowiedzialności dotyczy zupełnie czego innego w przypadku metod niż w przypadku klas, wykorzystywanie jej w obu (rozdzielone małe metody o jednej odpowiedzialności umieszczone w odpowiednich klasach) ma niebagatelny wpływ na jakość naszego kodu a więc na łatwość jego zrozumienia przez innych oraz jego utrzymywalność. 

3 komentarze:

Jacek Laskowski pisze...

To ma faktycznie sens. Jakkolwiek kilkakrotnie zmierzałem w kierunku rozłożenia odpowiedzialności właściwie, to dokładnie jak napisałeś "ten model [EJB] promuje takie programowanie". Zbyt często daję się ponieść nowościom bez dokładniejszej analizy. Dodaję wpis do naszego lutowego biuletynu Warszawa JUG. Gratulacje!

Jacek

pedro pisze...

Spolszczanie nie służy ogólnie stosowanym skrótą SRP jest przez jakiś czas Twego wpisu Regułą (RPO), później Zasadą (ZPO), Myślę że warto stosować orginalną nazwe a napewno orginalny skrót.

Te skróty powstaja po to byśmy my programiści mogli się łatwo komunikować i to komunikować globalnie. Jeżeli w Code Review (to jakoś naturalnie wiem o co chodzi w Przeglądzie kodu już jakoś mniej :)) powiem że ta klasa narusza SRP to większość wie o co chodzi. (ci co nie wiedzą są edukowani by wiedzieć)
Nawet sam przyznaje pamiętam SRP a dopiero staram sie to rozwinąć.

Paweł Lipiński pisze...

Może masz i rację. Ja w czasie Code Review mówię raczej: ta metoda robi to, to i to, nie sądzisz, że zamiast tego lepiej było by podzielić to na 3 oddzielne?

Mam wątpliwości czy faktycznie SRP jakimś szeroko rozpoznawanym skrótem. Gdyby tak było, nie miałbym o czym pisać :)

Ale nie twierdzę, że nie masz racji (co więcej, z tego powodu zostawiłem oryginalną wersję w tytule posta). Po prostu nie spotkałem się z użyciem SRP w rozmowach programistów.