SOLID - Open-Closed Principle
W oryginalnej wersji zasada otwartości-zamknięcia brzmi tak: Elementy oprogramowania (klasy, moduły, funkcje) powinny być otwarte na rozszerzanie ale zamknięte na zmiany.
Hm, zwykle gdy chcemy dodać jakąś funkcjonalność po prostu dopisujemy odpowiedni kod. Weźmy przykład:
Klasy proste, śliczne (oczywiście troszkę uproszczone :)). I nagle przychodzi wymaganie: użytkownik który ma uprawnienia administratora ma mieć flagę deleted która oznacza, że tak naprawdę nie ma być usuwany, tylko zaznaczony jako usunięty i nie prezentowany już potem w GUI. No to pierwsze podejście pewnie będzie takie:
No, gotowe! Tylko, czy to jest dobry design? Już if w metodzie usuwającej wskazuje, że dzieje się tam więcej niż jedna rzecz. Co więcej, zasada pojedynczej odpowiedzialności dużo nam tu nie pomoże, możemy co jakwyżej osiągnąć coś takiego:
Zwiększyliśmy trochę ilość kodu (twój manager już jest zadowolony - kolejna wskaźnik postępu zwiększony!), może trochę lepiej widać na czym polega różnica między usuwaniem administratora i nie-administratora, ale tak naprawdę kod wiele się nie poprawił. Jeśli będą dochodziły kolejne wymagania jak to, nasza metoda delete zmieni się wkrótce w wielki ciąg if-else. I tu właśnie jest miejsce do zastosowania zasady otwartości-zamknięcia.
Hm, zwykle gdy chcemy dodać jakąś funkcjonalność po prostu dopisujemy odpowiedni kod. Weźmy przykład:
public class User {
private String login;
private String password;
private UserRepository repo;
public void delete () {
repo.delete(this);
}
public User findByName(String name) {
return repo.findByName(name);
}
}
public class UserRepository {
private EntityManager em;
public void delete(User user) {
em.remove(user);
}
public User findByName(String name) {
return (User) em.createQuery("from User u where u.name=:name")
.setParameter("name", name)
.getSingleResult();
}
}
Klasy proste, śliczne (oczywiście troszkę uproszczone :)). I nagle przychodzi wymaganie: użytkownik który ma uprawnienia administratora ma mieć flagę deleted która oznacza, że tak naprawdę nie ma być usuwany, tylko zaznaczony jako usunięty i nie prezentowany już potem w GUI. No to pierwsze podejście pewnie będzie takie:
public class User {
private String login;
private String password;
private boolean admin;
private boolean deleted;
private UserRepository repo;
public void delete () {
repo.delete(this);
}
public User findByName(String name) {
return repo.findByName(name);
}
}
public class UserRepository {
private EntityManager em;
public User findByName(String name) {
return (User) em.createQuery("from User u where u.name=:name and deleted=false")
.setParameter("name", name)
.getSingleResult();
}
public void delete(User user) {
if (user.isAdmin()) {
user.setDeleted(true);
em.persist(user);
} else {
em.remove(user);
}
}
}
No, gotowe! Tylko, czy to jest dobry design? Już if w metodzie usuwającej wskazuje, że dzieje się tam więcej niż jedna rzecz. Co więcej, zasada pojedynczej odpowiedzialności dużo nam tu nie pomoże, możemy co jakwyżej osiągnąć coś takiego:
public class User {
private String login;
private String password;
private boolean admin;
private boolean deleted;
private UserRepository repo;
public void delete () {
if (! admin)
repo.deleteNonAdmin(this);
else {
repo.deleteAdmin(this);
}
}
public User findByName(String name) {
return repo.findByName(name);
}
}
public class UserRepository {
private EntityManager em;
public User findByName(String name) {
return (User) em.createQuery("from User u where u.name=:name and deleted=false")
.setParameter("name", name)
.getSingleResult();
}
public void deleteNonAdmin(User user) {
em.remove(user);
}
public void deleteAdmin(User user) {
user.setDeleted(true);
em.persist(user);
}
}
Zwiększyliśmy trochę ilość kodu (twój manager już jest zadowolony - kolejna wskaźnik postępu zwiększony!), może trochę lepiej widać na czym polega różnica między usuwaniem administratora i nie-administratora, ale tak naprawdę kod wiele się nie poprawił. Jeśli będą dochodziły kolejne wymagania jak to, nasza metoda delete zmieni się wkrótce w wielki ciąg if-else. I tu właśnie jest miejsce do zastosowania zasady otwartości-zamknięcia.
Kod powinien być otwarty na rozszerzanie a zamknięty na modyfikację.
Co to znaczy w praktyce?
Dziedziczenie.
Jeśli przypomnimy sobie, że w językach obiektowych możemy dziedziczyć, okaże się, że ten kod jest jeszcze do uratowania :)
Teraz if-else w ogóle nie istnieje. Kod wołający metody findByName i delete dalej nie musi wiedzieć, że dana instancja klasy User jest administratorem, ale nowa struktura kodu wyeliminowała potrzebę tworzenia warunków (zwykle było by ich więcej niż tylko w jednym miejscu, więc jeśli w kodzie masz gdzieś ten sam warunek sprawdzany parę razy w różnych metodach, to prawdopodobnie powinieneś zastanowić się nad dziedziczeniem). Kod jest teraz dużo bardziej przejrzysty (np. od razu wiadomo gdzie szukać jeśli coś jest nie tak z administratorem).
Ta zasada wydaje się zupełnie oczywista. Nie trzeba doktoratu z informatyki, żeby wpaść na takie rozwiązanie. Jednak jakże często dodajemy funkcjonalności do kodu bez większego zastanowienia. Po prostu dodajemy kod. Pilnowanie zasady otwartości-zamknięcia jest kolejnym sposobem na nie zwiększanie długu technicznego (nie pogorszanie jakości kodu) wraz z dodawaniem funkcjonalności.
Pisanie i projektowanie aplikacji poprzez stosowanie TDD powoduje, że czas na zastanowienie się nad najlepszym rozwiązaniem staje się nieodłączną częścią procesu rozwoju oprogramowania wykonywaną z resztą co parę minut, co oczywiście ma bezpośredni wpływ na jakość kodu który po sobie pozostawiamy. Istnienie testów powoduje, że nie mamy obaw związanych z wprowadzaniem większych zmian do oprogramowania - np. tak jak w powyższym przykładzie z wymianą warunków na dziedziczenie.
Co to znaczy w praktyce?
Dziedziczenie.
Jeśli przypomnimy sobie, że w językach obiektowych możemy dziedziczyć, okaże się, że ten kod jest jeszcze do uratowania :)
public class User {
protected String login;
protected String password;
protected UserRepository repo;
public User findByName(String name) {
return repo.findByName(name);
}
public void delete() {
repo.delete(this);
}
}
public class AdminUser extends User {
boolean admin;
private boolean deleted;
}
public class UserRepository {
protected EntityManager em;
public User findByName(String name) {
return (User) em.createQuery("from User u where u.name=:name")
.setParameter("name", name).getSingleResult();
}
public void delete(User user) {
em.remove(user);
}
}
public class AdminUserRepository extends UserRepository {
public User findByName(String name) {
return (User) em.createQuery("from User u where u.name=:name and deleted=false")
.setParameter("name", name)
.getSingleResult();
}
public void delete(AdminUser user) {
user.setDeleted(true);
em.persist(user);
}
}
Teraz if-else w ogóle nie istnieje. Kod wołający metody findByName i delete dalej nie musi wiedzieć, że dana instancja klasy User jest administratorem, ale nowa struktura kodu wyeliminowała potrzebę tworzenia warunków (zwykle było by ich więcej niż tylko w jednym miejscu, więc jeśli w kodzie masz gdzieś ten sam warunek sprawdzany parę razy w różnych metodach, to prawdopodobnie powinieneś zastanowić się nad dziedziczeniem). Kod jest teraz dużo bardziej przejrzysty (np. od razu wiadomo gdzie szukać jeśli coś jest nie tak z administratorem).
Ta zasada wydaje się zupełnie oczywista. Nie trzeba doktoratu z informatyki, żeby wpaść na takie rozwiązanie. Jednak jakże często dodajemy funkcjonalności do kodu bez większego zastanowienia. Po prostu dodajemy kod. Pilnowanie zasady otwartości-zamknięcia jest kolejnym sposobem na nie zwiększanie długu technicznego (nie pogorszanie jakości kodu) wraz z dodawaniem funkcjonalności.
Pisanie i projektowanie aplikacji poprzez stosowanie TDD powoduje, że czas na zastanowienie się nad najlepszym rozwiązaniem staje się nieodłączną częścią procesu rozwoju oprogramowania wykonywaną z resztą co parę minut, co oczywiście ma bezpośredni wpływ na jakość kodu który po sobie pozostawiamy. Istnienie testów powoduje, że nie mamy obaw związanych z wprowadzaniem większych zmian do oprogramowania - np. tak jak w powyższym przykładzie z wymianą warunków na dziedziczenie.
Od tego w jaki sposób wprowadzamy rozszerzenia do naszego projektu zależy bardzo dużo. Czas developmentu, ryzyko błędu, może nawet w końcu przepisywanie projektu. Wszystko to sprowadza się do marnowania dużych ilości pieniędzy naszego pracodawcy / klienta, może nawet osłabić jego konkurencyjność, a przecież nikt nie dał nam do tego prawa. I jeśli Twój project manager czy klient ciśnie Cię terminami, to może zamiast bezwolnie poddawać się tej presji lepiej dokładnie mu to wytłumaczyć. To nie jest Twoja rola, żeby podejmować decyzje o stałym obniżaniu jakości kodu przez byle jakie wrzucanie nowych linii do istniejącego kodu, więc zrzuć to na osoby które takie decyzje są władne podjąć. Niech twój PM podpisze się własną krwią pod zdaniem:
Tak, chcę doprowadzić do obniżenia jakości oprogramowania. Bardziej niż na jakości zależy mi na terminach. Jestem skłonny raczej w przyszłości mieć ciągłe problemy z ukańczaniem funkcjonalności i błędami niż zmienić termin tego releasu.
Jeśli to zrobi (co niestety się zdarza), przynajmniej będziesz miał czyste sumienie. I nikt nie zarzuci ci braku profesjonalizmu i nieodpowiedzialności.
autor: Paweł Lipiński
09:16
Komentarze (3)
