IT eSky.pl

RSS

Budowanie z kompozytu

Przemierzając bezkresne morze linii kodu jednej z naszych firmowych aplikacji, której jednym z zadań jest przygotowywanie faktur za sprzedane produkty,  natknąłem się na ciekawy fragment. Kawałek kodu miał za zadanie generować obiekty odpowiadające pozycjom na fakturze. Faktura może posiadać trzy pozycje: z ceną biletu, opłatą lotniskową i opłatą serwisową linii lotniczej. Kod wyglądał mniej więcej tak:

W sumie nudy. Nic tu się nie dzieje. Standardowa implementacja wzorca buildera jakich w internetach wiele. Zaciekawiło mnie jednak użycie funkcji array_filter. Postanowiłem więc zajrzeć do kilku builderów implementujących ItemsBuilder. Znalazłem tam coś co w uproszczeniu można zobrazować następująco:

Okazuje się że pozycje faktury mogą być generowane w różny sposób w zależności od prawa księgowego obowiązującego w kraju, w którym faktura została przygotowana.  Niektóre pozycje mogą zostać zsumowane do innej. W efekcie zdarzały się w builderach metody, które zwracały tylko i wyłącznie samotne i smutne null. Tym samym użycie funkcji array_filter zostało wyjaśnione i uwydatniło problem, którego wcześniej nie zauważyłem.

Czyli..

ten kod nie jest SOLIDny. Została złamana tutaj zasada (I) Interface Segregation. Wiele klas implementujących interfejs ItemsBuilder posiada metody, które nikomu, do niczego nie są potrzebne. Co więcej klient tych metod ItemsGenerator nie dość, że niepotrzebnie je wywołuje to jeszcze obsługuje zwracanego nulla filtrując tablicę pozycji faktury.

Po chwili zastanowienia nabrałem wątpliwości co do innych pryncypiów. Czy założenie, że odpowiedzialnością klasy buildera jest zbudowanie wszystkich pozycji faktury nie jest zbyt ogólne? Przecież każda z nich budowana jest w odrębny sposób (Single responsibility)? Hmm….

…a co stanie się w momencie gdy faktura zacznie wymagać budowania jakiejś nowej pozycji? Trzeba będzie zmienić interface i… dopisać implementacje nowej metody do wszystkich istniejących już builderów. Więc Open-Close też możemy skreślić z listy spełnionych pryncypiów.

…co z tym zrobić?

Najprościej jest pozbyć się problemu ze zbyt wielką odpowiedzialnością builderów. Wystarczy budowanie każdej z pozycji wyciągnąć do… mniejszych builderów ;) i delegować do nich budowanie pozycji o określonym typie.

Natomiast mając kod w obecnej formie ciężko będzie się pozbyć nadmiarowych metod z SeparatedItemsBuilder i SummedItemsBuilder.

Tylko czy te builder są nam jeszcze do czegoś potrzebne… Myślę, że nie! Skoro wydzieliliśmy budowanie każdej z pozycji faktury do osobnych klas, to co stoi na przeszkodzie, żeby zasilać nimi od razu ItemsGenerator? No właśnie, nic ;)

Zaproponowane rozwiązanie to implementacja wzorca kompozytu. Świetnie rozwiązuje obecne problemy. Do generatora dostarczamy tylko takie buildery jakie są w danym przypadku potrzebne więc nie ma żadnych nadmiarowych wywołań. W przypadku potrzeby stworzenia nowego typu pozycji wystarczy dopisać klasę buildera i przekazać ją do generatora wtedy, kiedy jest to potrzebne.  Natomiast główną zaletą kompozytu jest to,  że z pojedynczym obiektem możemy pracować tak samo jak z całą grupą obiektów tego samego typu. W naszym przypadku sprowadza się to do tego, że w fakturach posiadających jedną pozycję nie musimy nawet tworzyć obiektu generatora. Od razu możemy wykorzystać właściwy builder. Kod takiego rozwiązania mógłby wyglądać jak powyżej.

Użycie prezentuje się następująco:

Mały krok dalej.

Z generatorem pozycji w obecnej formie jest niestety jeden problem. Jak wiadomo PHP nie oferuje zadeklarowania jakiego typu elementy znajdują się w tablicy. Czyni to nasz generator zbyt łatwowiernym, w to że tablica zawsze będzie zawierała tylko i wyłącznie obiekty implementujące interfejs ItemsBuilder. Na szczęście można to łatwo wymusić pisząc prostą kolekcję przyjmującą tylko obiekty implementujące ItemsBuilder i zmienić w konstruktorze deklarację typu z arraya na kolekcję.

A Ty jak zrefaktoryzowałbyś taki kod? Podziel się swoimi doświadczeniami.

 

 


Zobacz również