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:

interface ItemsBuilder{
    public function createTicketPriceInvoiceItem();
    public function createAirportFeeInvoiceItem();
    public function createAirlineServiceFeeInvoiceItem();
}

class ItemsGenerator {
    private $itemsBuilder;

    public function __construct(ItemsBuilder $itemsBuilder) {
        $this->itemsBuilder = $itemsBuilder;
    }

    public function generateItems() {
        $items = [
            $this->itemsBuilder->createTicketPriceInvoiceItem(),
            $this->itemsBuilder->createAirportFeeInvoiceItem(),
            $this->itemsBuilder->createAirlineServiceFeeInvoiceItem(),
        ];

        $items = array_filter($items);

        return $items;
    }
}

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:

class SeparatedItemsBuilder implements ItemsBuilder {

    public function createTicketPriceInvoiceItem() {
        /**
         * zbuduj pozycję z ceną biletu
         */
    }

    public function createAirportFeeInvoiceItem() {
        /**
         * zbuduj pozycję z opłatą lotniskową
         */
    }

    public function createAirlineServiceFeeInvoiceItem() {
        /**
         * zbuduj pozycję z opłatą serwisową linii lotniczej
         */
    }
}

class SummedItemsBuilder implements ItemsBuilder {

    public function createTicketPriceInvoiceItem() {
        /**
         * zbuduj pozycję z ceną biletu 
         * i sumuj z opłatą lotniskową oraz opłatą serwisową linii lotniczej
         */
    }

    public function createAirportFeeInvoiceItem() {
        return null;
    }

    public function createAirlineServiceFeeInvoiceItem() {
        return null;
    }
}

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 😉

interface ItemsBuilder {
    /**
     * @return array
     */
    public function build();
}

class TicketPriceInvoiceItemBuilder implements ItemsBuilder {
    /**
     * @return array
     */
    public function build() {
    }
}

class TicketPriceWithFeesInvoiceItemBuilder implements ItemsBuilder {
    /**
     * @return array
     */
    public function build() {
    }
}

class AirportFeeInvoiceItemBuilder implements ItemsBuilder {
    /**
     * @return array
     */
    public function build() {
    }
}

class AirlineServiceFeeInvoiceItemBuilder implements ItemsBuilder {
    /**
     * @return array
     */
    public function build() {
    }
}

class ItemsGenerator implements ItemsBuilder {
    protected $itemBuilders;

    public function __construct(array $itemBuilders) {
        $this->itemBuilders = $itemBuilders;
    }

    /**
     * @return array
     */
    public function build() {
        $items = [];

        foreach ($this->itemBuilders as $itemBuilder) {
            $items = array_merge($items, $itemBuilder->build());
        }

        return $items;
    }
}

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:

$itemsBuilder = new ItemsGenerator([
    new TicketPriceInvoiceItemBuilder(),
    new AirportFeeInvoiceItemBuilder(),
    new AirlineServiceFeeInvoiceItemBuilder(),
]);
$items = $itemsBuilder->build();

$itemsBuilder = new TicketPriceWithFeesInvoiceItemBuilder();
$items = $itemsBuilder->build();

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.