sobota, 19 października 2013

SOLIDny kod cz. 4 - Liskov substitution principle

zacznijmy od podstaw

Zasada podstawienia Liskov brzmi:
Funkcje które używają wskaźników lub referencji do klas bazowych, muszą być w stanie używać również obiektów klas dziedziczących po klasach bazowych, bez dokładnej znajomości tych obiektów.
Sprowadza się to do tego, że w każdym miejscu gdzie wykorzystujemy klasę bazową (rozszerzaną) lub interfejs możemy wykorzystać każdą klasę potomną i nie wpłynie to na zmianę zachowania czy też wynik działania, nie wpłynie negatywnie na działania aplikacji.

Ok, definicja definicją, ale jak to ugryźć?

zapoznajmy się z przykładem

Załóżmy, że mamy do napisania aplikację do tworzenia i zarządzania przepływem faktur w firmie.
Mamy możliwość stworzenia faktury proforma, normalnej faktury oraz wystawienia faktury korygującej. Gdy już mamy gotowy dokument, to możemy go przesłać do wybanych osób. Dodatkowo każda faktura oraz faktura korygująca powinna zostać automatycznie przesłana do księgowej.
Jednym słowem nic skomplikowanego:)

Wszystkie te dokumenty są do siebie podobne zarówno pod względem sposobu tworzenia (wymagane informacje) jak i wyświetlania (jedyna istotna różnica zawiera się w nagłówku) dlatego też decydujemy się na stworzenie interfejsu, który będzie wykorzystany w obydwu modułach (kreacyjnym i wysyłkowym).
Dla potrzeb wpisu załóżmy, że poniższy interfejs zawiera wszystkie niezbędne metody oraz są one wykorzystywane w obu modułach (czyli jednym słowem jego rozbicie byłoby bezpodstawne):
package com.smalaca.liskovsubstitution.invoice;

import com.smalaca.liskovsubstitution.invoice.Item;
import com.smalaca.liskovsubstitution.invoice.Person;

public interface Invoice {
 
 public int getTotal();
 public void addItem(Item item);
 public Person getBuyer();
 public Person getSeller();
}

Interfejs jest implementowany przez trzy klasy:
package com.smalaca.liskovsubstitution.document;

import com.smalaca.liskovsubstitution.invoice.Invoice;
import com.smalaca.liskovsubstitution.invoice.Item;
import com.smalaca.liskovsubstitution.invoice.Person;

public class ProForma implements Invoice {

 //class implementation
}
package com.smalaca.liskovsubstitution.document;

import com.smalaca.liskovsubstitution.invoice.Invoice;
import com.smalaca.liskovsubstitution.invoice.Item;
import com.smalaca.liskovsubstitution.invoice.Person;

public class InvoiceCorrection implements Invoice {

 //class implementation
}
package com.smalaca.liskovsubstitution.document;

import com.smalaca.liskovsubstitution.invoice.Item;
import com.smalaca.liskovsubstitution.invoice.Person;

public class Invoice implements com.smalaca.liskovsubstitution.invoice.Invoice {

 //class implementation
}

Implementację metod pomijam, ponieważ jest zbędna dla potrzeb wpisu. Chciałbym jedynie dodać, że dla wszystkich powyższych klas interfejs określa wszystkie wystarczające metody i żadna z nich (klas) nie posiada ani jednej dodatkowej metody.

Kod wyszedł całkiem niezły, moduły stworzone i tylko trzeba było obsłużyć jakoś wysyłanie faktur zwykłych i korygujących do księgowej, ale w tym wypadku zadowalającym rozwiązaniem okazał się jeden warunek w kodzie i po krzyku.

i zaczynają się problemy

Pewnego dnia nasz klient dochodzi do wniosku, że chciałby aby praca księgowej również mogła być wykonywana przy użyciu aplikacji. Cóż można rzec, gdy klient ładnie prosi ;) Tym bardziej, gdy mamy dobrze napisany kod :)

Zaczynamy jednak myśleć nad całą funkcjonalnością i w pewnym momencie docieramy do scenariusza zaksięgowania faktury i napotykamy na problem, bo z jednej strony mamy świetny interfejs, który nam sporo ułatwia (Invoice), a z drugiej strony sprawa nie jest tak idealna jak by się mogło wydawać, ponieważ klasa ProForma również go implementuje, a dokumentów tego typu nie chcemy księgować.
Oczywiście można rozważyć usunięcie implementacji interfejsu z tej klasy, ale przecież całe tworzenie i wysyłanie opiera się na nim i zbyt wiele taka zmiana by nas kosztowała.

Decydujemy się w takim wypadku na dodanie warunku, który będzie wszystko sprawdzał i w przypadku przekazania instancji klasy ProForma wyrzuci deskryptywny wyjątek:
package com.smalaca.liskovsubstitution;

import com.smalaca.liskovsubstitution.invoice.Invoice;

public class Ledger {

 public void bookSales(Invoice invoice) throws NotAnInvoiceException
 {
  // throws exception when instance of ProForma class given
 }
}

Czy to rzeczywiście jest jednak takie dobre rozwiązanie? Ten wyjątek trzeba przecież gdzieś łapać i ewentualnie obsłużyć. I co gdy w trakcie analizy i projektowania odkryjemy więcej takich miejsc? Albo za jakiś czas nasz klient znowu przyjdzie z pomysłem na funkcjonalność i okaże się, że tam będzie trzeba ponownie wykorzystać warunkowe instrukcje?

i co?

I tak sobie myślę, że to już drugi raz, gdy trzeba było zastosować if'a (za pierwszym razem przy wysyłaniu maila do księgowej), więc chyba coś jest jednak nie tak.

Jeżeli też tak myślicie, to powiem Wam, że macie rację - powyższy kod nie przestrzega zasady LSP.
Oczywiście, gdybyśmy usunęli warunki, to kod działałby, ponieważ wszystkie trzy klasy implentują interfejs i dla każdej z nich poszczególna funkcjonalność w modułach powinna działać bezbłędnie, a jednak... dochodzimy do wniosku, że pomimo to, gdzieś logika ucieka i takie zachowanie nie miałoby sensu.

Na co w powyższym warto zwrócić uwagę?
Pierwszym sygnałem był warunek przy wysyłce maila do księgowej, który powinien zapalić u nas czerwoną lampkę, zasygnalizować nam coś. Drugą wskazówkę daje nam pożądana przez klienta funkcjonalność - księgowość w aplikacji.
Powyższe nie są problemem samym w sobie, ale pokazują nam, że gdzieś on jest, tym razem znalazł się w naszej logice biznesowej i po krótkim zastanowieniu myślę, że również stwierdzicie, że tak jest. Połączyliśmy fakturę, korektę i proformę, a od początku wiemy, że proforma to "odrobinę" inny typ dokumentu. I warto byłoby to w jakiś sposób zasygnalizować.

A skoro odrobinę inny i musimy stosować instrukcje warunkowe, żeby obsłużyć jego odmienność, to właśnie łamiemy LSP.

co z tym robimy

Po krótkim przemyśleniu problemu postanawiamy, że warto byłoby to jednak poprawić. Tylko jak?

I to jest pytanie do Was. Czy macie jakieś pomysły? Jakieś rozwiązania przychodzą Wam do głowy? Jak zmienić kod tak, aby był zgodny z LSP? A może nie trzeba tego robić i wyrzucanie wyjątków wcale tak złym pomysłem nie jest? Może LSP byłaby zwyczajnym przerostem fromy nad treścią?

Czekam na komentarze z propozycjami. Problem nie jest trywialny i z pewnością ma niejedno poprawne rozwiązanie.
Pragnę również zaznaczyć, że kod nie przeczy dwóm wcześniej opisanym przeze mnie zasadą, a jednak coś w nim mierzi.

PS. Jeżeli jednak wspólnie nie uda nam się wymyślić nic sensownego, to w niedługim czasie opublikuję swoją propozycję:)

1 komentarz:

  1. Bardzo ciekawie napisane. Jestem pod wielkim wrażaniem.

    OdpowiedzUsuń