Kontekst pierwotny, a reużywalność obiektu

By oszczędzić sobie czasu i korzystać z przychylności programowania obiektowego – kod który napiszemy raz, możemy ponownie wykorzystać w nowych przypadkach. Myślę że zasada DRY, zawsze siedzi nam z tyłu głowy i zna ją każdy zawodowy programista. Niestety w programowaniu nic nie jest czarno-białe i po latach pracy nad różnymi projektami pojawiło się kilka wątpliwości co do ślepego stosowania DRY.

Co robi metoda i dlaczego?

Rola obiektu jest właściwie kontekstem jego wywołania. Przeanalizujmy przypadek w którym posiadamy Encje/WriteModel Product z jeszcze nienazwaną metodą:

namespace App\Offer\Domain\Entity;

final class Product 
{
	/* ... */
	
	public function ____________(): array 
	{
		return [
			'name' => $this->name,
			'description' => $this->description,
			'price' => $this->price,
			'currency' => $this->currency,
			'created_at' => $this->createdAt->format('Y-m-d H:i:s')	
		];
	}
} 

Co możemy odczytać z tego kodu?

    • metoda zwraca array’a który jest strukturą danych. Dodanie do niej @docBlock - /** @return array<string, mixed> */ nie wiele wniesie do klientów bo i tak by nie iterowali po zwróconej przez metodę tablicy,
    • array będący strukturą danych budowany jest na podstawie wewnętrznego stanu obiektu. Na przykładzie tego nie widać, ale załużmy, że serializacji podlega każde pole klasy,
    • Jest to metody typu Query (CQS) – niemodyfikująca wewnętrznego stanu obiektu czyli pozbawiona efektów ubocznych.

Kolejne wnioski możemy wyciągnąć patrząc na (jak dotąd) jedynego klienta korzystającego z tej metody:

namespace App\Offer\Infrastructure\Repository;

use App\Offer\Domain\Repository\ProductRepository;
use App\Offer\Domain\Entity\Product;
use Doctrine\DBAL\Connection;

final class ProductMySQLRepository implements ProductRepository
{
	public function __construct(private Connection $connection) {}
	
	public function save(Product $product): void 
	{
		$this->connection->executeStatement(
			'INSERT INTO product ...',
			$product->____________()
		);
	}
}

    • nienazwana metoda wywoływana jest w Warstwie Infrastruktury,
    • zmapowany stan Write Modelu reprezentował będzie jeden wiersz w tabeli product,

Biorąc pod uwagę fakt, że klasa ProductMySQLRepository jest jedynym klientem metody Product::__________ - możemy założyć, że powstała ona specjalnie by być wykorzystana do utworzenia wiersza w tabeli. Programista stanął przed problemem – „jak mogę utrwalić Encję Product – nie tworząc dla niej całej armi getterów?”. Załóżmy, że zdecydował się utworzyć metodę mapującą stan na dane oczekiwane przez drugi parametr Doctrine’owego Connection::executeStatement. Czy nie przewidując dla niej innego wykorzystania (w tym punkcie czasu) w przyszłości, wywołanie jej w innym kontekście – całkowicie innym niż INSERT bazodanowy – będzie akceptowalne?

Jaka nazwa jest najodpowiedniejsza?

Do wyboru mamy:
    1. toArray
    2. asArray
    3. serialize
    4. jsonSerialize
    5. map

Takie podejście sprawia, że w nazwie metody nie ma śladu po kontekście jej użycia, a który jest w całej historii powstania tej metody bardzo istotny. Bezkontekstowa nazwa metody sprawia złudne wrażenie, że wykorzystanie jej ponownie w zupełnie innym kontekście, w zupełnie innej roli jest całkowicie usprawiedliwione.

Powinniśmy więc odpowiedzieć sobie na jedno pytanie: „Czy nazwa metody powinna określać intencje jej pierwotnego użycia?” - moim zdaniem nie. Niezmienia to jednak wspomnianej już wielokrotnie istotności kontekstu dla którego została wprowadzona, na którego musimy zwracać uwagę podczas ponownego użycia metody.
 
Na potrzeby omawianego przykładu możemy ustalić, że asArray będzie prawdziwą nazwą Product::__________ metody. Dlaczego akturat ta nazwa została wybrana spośród innych? - można powiedzieć, że chodzi o konwencje w projekcie. Często musimy serializować obiekty do tabel, powody tego są różne. Tak jak w opisanym przykładzie, raz potrzebujemy przesłać dane do tabeli bazodanowej, innym razem wysłać je poza aplikację. Musimy przyjąć jedną konwencję nazewniczą dla tego typu metod – na co kolwiek się ostatecznie zdecydujemy, nie będzie miało ostatecznego wpływu na jakość projektu.   

Jeden obiekt – dwa konteksty użycia

Zapisywanie produktu do bazy danych zostało wdrożone na produkcję miesiące temu. Biznes jednak nie pozostawił tematu na zawsze i postanowił wzbogacić moduł oferty o nowy feature. Na każde dodanie nowego produktu miał być wysyłany email do zewnętrznego systemu z szablonem umożliwiającym dynamiczne dodanie danych produktu. Podczas tworzenia templatki, można by korzystać z zestawu kluczy, których wartości zawsze byłyby dostępne wraz z wysyłanym emailem. By jeszcze skomplikować sprawę, za te czynności (wysyłanie emaili i zarządzanie templatkami) odpowiedzialny by był zewnętrzny system.   

Spoglądajć na kod odpowiedzialny za zapisywanie produktu sprawa wygląda dość prosto bo... 

namespace App\Offer\Application\Handler;

use App\Offer\Domain\Entity\Product;
use App\Offer\Domain\Repository\ProductRepository;

final class CreateProductHandler 
{
	public function __construct(
		private ProductRepository $productRepository;
	) {}

	public function __invoke(/*...*/): void 
	{
		/*...*/
		$this->productRepository->save($product);
	}
}

...jedyne co musimy zrobić to wstrzyknąć serwis odpowiedzialny za komunikację z zewnętrznym serwisem wysyłającym emaile. Jako że mamy obiekt $product pod ręką, wraz z możliwością jego łatwej serializacji do struktury tablicowej – możemy pokusić się o implementację z jego wykorzystaniem. Metoda __invoke potrafiąca zlecać wysyłkę email’a wyglada nastepująco:

public function __invoke(/*...*/): void 
{
	/*...*/
	$this->productRepository->save($product);
	$this->externalEmailService->send($adminEmail, $product->asArray());
}

W tym momencie struktura tablicowa zwracana przez metodę Product::asArray jest wykorzystywana dwuktornie w różnych  kontekstach:
    1. zapisu wiersza tabeli bazy MySQL,
    2. templatce emaila znajdującego się w zewnętrznym systemie  

Jak pokazuje poniższy kod w zewnętrznym systemie znalazły się „pojęcia” związane z wewnątrz-systemową reprezentacją zapisu encji z innego systemu.

	
Utworzono produkt {{name}}
	{{created_at}}
	

	
{{description}}

Rozważmy plusy i minusy

✅ nie musimy robić nic więcej niż wywołanie metody która została już wcześniej napisana. Nazwa metody serializującej nie zawiera w sobie żadnego kontekstu w którym została utworzona dlatego korzystamy z niej z czystym sumieniem,

❌ powiązaliśmy ze sobą niejawnie dwa konteksty: zmiana klucza w zserializowanym produkcie wprowadzona na potrzeby kontekstu bazodanowego, będzie miała też swoje konsekwencje w kontekście zewnętrznego serwisu do wysyłki emaili. Ewentualne zmiany, które muszą być wprowadzone tylko w jednym z kontekstów będą wymagały wprowadzenia małych hacków,
❌ do zewnętrznego serwisu wysyłającego emaile mogą trafić nadmiarowe/poufne dane, być może o nieodpowiednich nazwach kluczy. Możemy tego uniknąć stosując drobne hacki – przemapowanie tablicy, unsetowanie,
❌ korzystamy z metody utworzonej w celu utrwalaniu Write Modelu – w sposób jakiego nie zakładał jej twórca. Traktując zwracany array jako View Model,
❌ Gdybyśmy zdecydowali się na wysyłkę emaila w Event Subscriberze nie mielibyśmy takiego łatwego dostępu do encji $product jak wcześniej. Musielibyśmy albo pozyskać ją z repozytorium  i w dalszym ciągu, z uporem maniaka korzystać z niej jak z Read Modelu.
 

Z pozoru prostrze rozwiązanie może powodować więcej problemów w przyszłości niż rozwiązuje teraz.

View Model w nowym kontekście

By uchronić się przed silnym couplingiem pomiędzy dwoma wspomnianymi kontekstami, nie możemy cały czas polegać na jednej tablicowej strukturze danych. Musimy wprowadzić nowy byt, a z racji tego, że zserializowana encja Product była początkowo wykorzystywana w kontekście zapisu danych do tabeli bazodanowej – tą część pozostawimy w pierotnej postaci. W kontekście wysyłki maili zostanie utworzona nowa klasa typu View Model.

Obiekty tego typu:
    • stanowią warstwę buforową pomiędzy rdzeniem naszej aplikacji, a światem zewnętrznym,
    •  są niemutowalne, służą jedynie jako struktura danych bez żadnych zachowań,
    • powinny posiadać odpowiednio sformatowane dane przeznaczone dla klienta.

Przykładowa implementacja mogłaby prezentować się następująco:

namespace App\Offer\Application\ViewModel;

final class ProductEmail 
{
	public function __construct(
		private string $name,
		private string $description,
		private float $price,
		private string $currency,
		private DateTimeImmutable $createdAt
	) {}
	
	public function asArray(): array 
	{
		return [
			'name' => $this->name,
			'description' => $this->description,
			'price' => number_format($this->price) . ' ' . strtoupper($this->currency),
			'createdAt' => $this->createdAt->format('Y-m-d H:i:s')	
		];
	}
}

Jak widać klucze jak i formatowanie udostepnianych danych są specjalnie dostosowane pod wymagania bytów ze świata zewnętrznego. Nie jesteśmy teraz od niczego zależni, dlatego na nowe wymagania biznesowe możemy dowolnie zmieniać serializację View Modelu – dodawać nowe klucze, modyfikować nazwy już istniejących jak i zmieniać foramtowanie samych wartości. Problemem może okazać się samo instancjonowanie tego obiektu, najpewniej w metodzie repozytorium powiązanym z Encją Product, niemniej jednak jest to niewielki koszt za cenę dobrego designe’u.

Koszt jaki musimy ponieść to:

💰 Utworzenie nowej klasy View Model’u
💰 Instancjonowanie VM, najpewniej w nowej metodzie repozytorium
💰 dodając do Encji Product nowe pole, nie doda się ono automatycznie do View

Modelu co miałoby miejsce w pierwszej inkarnacji „Wysyłki emaila na utworzenie produktu”. Niewątpliwie rozpatrujemy to jako zaletę, ale również jako koszt – rzeczy o której należy pamiętać i potęcjalnie wykonać.

Złożoność

Możemy odnieść wrażenie, że wraz z dodawaniem kolejnych klas/metod rośnie złożoność projektu. W tym przypadku jest to złudne wrażenie. Pomyślmy, korzystając z jednej struktury tworzymy silny coupling pomiędzy modułami, które nie powinny być ze sobą powiązane na tym poziomie:

Dodajemy nowe pole do Encji Product ➡️ pojawia się nowy klucz możliwy do wykorzystania w templatce emaila.

lub:

Usuwamy pole z Encji Product ➡️ Zewnętrzny System przestaje wysyłać emaile o utworzeniu produktu.

I z szerszej perspektywy:

Modyfikujemy Domenę w Module Offer ➡️ Zewnętrzny System do emaili przestaje działać.

Nowy programista w projekcie, mógłby przeoczyć ten fakt i nie być nawet świadom takiego efektu ubocznego podczas dodawania nowego atrybutu Encji. Tego typu nieprzewidziane efekty uboczne są przyczyną powstania trudnych do wytropienia błędów, sprawiając, że złożoność oprogramowania rośnie.

Czas zweryfikował: rola jest jedna

Osobne byty dla innych kontekstów dają niesamowitą swobodę, ale jeżeli początkowo są identyczne i za każdym razem trzeba nanosić takie same zmiany do dwóch klas – znaczy, że nie powinniśmy w ogóle wprowadzać takiego rozdzielenia, bo pierwotny obiekt odgrywa taką samą rolę w dwóch kontekstach. 

Podsumowanie

Tak jak zostało wykazane w opisywanym przypadku, by usunąć złożoność i ukryty coupling, musiały być utworzone dodatkowe byty. W konsekwencji tego, mogą pojawiać się głosy innych członków team’u:

Czasami trudno jest przewidzieć konsekwencje niektórych decyzji podczas implementacji. Dlatego warto czasami zatrzymać się i przemyśleć niektóre kwestie, jakie będą ich konsekwencję. Z pozoru „skomplikowanie” projektu dodaniem kilki nowych klas może okazać się zabawienne w czasie jego dalszego życia. Zachęcam więc do przemyśleń w tej kwestji.

 

Weryfikacja stanu obiektu gdy stan nie jest ujawniony

 

Spotkałem się ostatnio z ciekawym przypadkiem weryfikowania wyniku testu jednostkowego. Testowaniu została poddana void'owska metoda Agregatu i nijak nie można było wyciągnąć na zewnątrz stanu obiektu, który powinien ulec zmianie w wyniku wywołania metody under test, by można było go zweryfikować z oczekiwaniami. Agregat nie posiadał powiązanych z tym stanem getter'ów, wygenerowane event'y nie mogły zostać jawnie zwrócone poza obiektu (\Prooph\EventSourcing\AggregateRoot tego nie udostępniał) i żadna inna metoda jakkolwiek nie wyrzucała stanu na zewnątrz. Dodanie takich funkcji na siłę, tylko na potrzeby wykonania testu jednostkowego wydawało się nadużyciem więc też takie rozwiązanie nie wchodziło w grę.

Przez powyższe problemy, trzeba było wykazać się pomysłowością by napisać test - kolega z zespołu wpadła na takie oto rozwiązanie:

public function test(): void
{
    $objectUnderTest = new Person('Jan', 'Kowalski');

    // When
    // ...

    Closure::fromCallable(function () {
        TestCase::assertSame('Jan', $this->firstName);
        TestCase::assertSame('Kowalski', $this->lastName);
    })->bindTo($objectUnderTest, Person::class)();
}

Gdy pierwszy raz zobaczyłem takie podejście byłem zaskoczony że tak się da, zaraz potem uznałem że tak nie powinno się robić. Tyle raz powtarzano, że testowaniu powinno podlegać jedynie publiczne API obiektu, a jego wewnętrzny stan powinien zostać w ukryciu... i jest to mądre podejście. Dzięki temu mamy wolną rękę w przeprowadzaniu refaktoru - raz napisany test, daje nam możliwość weryfikowania czy ulepszanie wewnętrznej struktury obiektu po zmianach działa dalej tak jak tego oczekujemy.   

To rozwiązanie łamię zasadę, wiążąc test z ukrytymi przed resztą kodu produkcyjnego wnętrznościami Agregatu. Potem jednak zacząłem się zastanawiać na ile realnie problematyczny będzie on w utrzymaniu i czy czasem od dawna nie mierzymy się z tego typu problemami.

🔒 Kod produkcyjny dalej nie wie o stanie Agregatu

Niewątpliwą zaletą zastosowania metody Closure::bindTo jest fakt, że nie wpływa ona w żadnym stopniu na kod produkcyjny. Nie utworzyliśmy specjalnych getter'ów wykorzystanych tylko na potrzeby testów co w przyszłości mogłoby skłonić innych developerów do skorzystania z nich. W pewnym stopniu getter jest formą udostępniania wartości 1:1 z tym co jest wewnętrznym stanem obiektu. Dodanie getter'a wygląda jak rozszerzenie publicznego API, ale tylko na pozór. Bo jeżeli zmienimy wewnętrzny stan to będziemy musieli zmienić owego getter'a. Można by zastanowić się nad innymi realnymi problemami z tym związanymi, ale jest to temat na inny wpis.   

W każdym razie wydaje mi się, że problem tworzenia takich getter'ów istnieje w światku PHP, jest stosunkowo często stosowany i tylko sprawia wrażenie niegroźnego - dlatego powinniśmy zwracać na to uwagę. 

Stosowanie Closure::bindTo zamias dedykowanych getterów chroni kod produkcyjny.

🇬🇧 Szkoła Londyńska

Pisząc test jednostkowy klasy typu serwis (np. Event Subscriber'a, Command Handler'a) według paradygmatów tz. Szkoły Londyńskiej musimy najpierw zamockować jego zależności. Można to zrobić za pomocą customowych implementacji InMemoryRepository bądź przy użyciu narzędzi takich jak Prophecy/MockObject.

I tutaj właśnie zacząłem dostrzegać podobieństwo względem opisywanego przykładu. Dokładnie wiemy jak w teście jednostkowym należy zbudować zależność wymaganą przez object under test oraz jak będzie wyglądała interakcja pomiędzy zależnością, a testowanym obiektem.

Musimy jawnie oprogramować to związanie by test mógł przejść na zielono. Test oczywiście weryfikuje publiczne API serwisu, ale wie też coś o jego wewnętrznej pracy. Zmieniając zależności serwisu będziemy musieli zaktualizować testy - dokładnie tak samo jak w opisywanym przypadku wykorzystania Closure::bindTo.

Jest jednak subtelna różnica, w przypadku Closure::bindTo funkcja anonimowa musi dokładnie wiedzieć w jakim polu znajduje się wartość czyli pożądana przez nas zmiana stanu. W przypadku weryfikowania mock'ów zaś, ta informacja w dalszym ciągu jest przed nami ukryta, ale niewątpliwie w jednym i drugim przypadku odwołujemy się podczas wykonywania asercji do wewnętrznego stanu obiektu.   

Jak widać ten problem towarzyszył w projekcie w którym pracuje praktycznie od samego początku jego powstania, a mimo to dało się z nim żyć - co więcej - nikt nie uznawał tego za problematyczne. W opisywanym przypadku pojawił się jedynie w nieco innej formie (inny typ testowanego obiektu), ale to dalej nic nowego z czym wcześniej się nie borykaliśmy.  

🗃 Dla jakich typów obiektów?

Na samym początku muszę zauważyć, że chodzi o zmianę stanu obiektu dlatego z tego rozwiazania należałoby korzystać tylko w przypadku testowania metod void'owskich. Takich metod nie posiadają obiekty typu:

❌ Event

❌ DTO

❌ Value Object

Serwisy z metodami void'owskimi też weryfikujemy w inny sposób - sprawdzając ich z'mock'owane zależności, dlatego kolejno odpadają nam:

❌ Event Subscriber

❌ Command Handler

❌ Serwis Aplikacyjny/Domenowy

To co właściwie pozostało to:

✅ Encja

✅ Agregat 

Lecz tylko w przypadku gdy takowe już nie udostępniają swojego stanu dla innego kodu produkcyjnego!

🩻 Inny sposób udostępniania stanu

Taki sam efekt możnaby osiągnąć stosując refleksje:

public function test(): void
{
    $objectUnderTest = new Person('Jan', 'Kowalski');

    // When
    // ...

    $reflection = new ReflectionClass(Person::class);
    $firstNameProperty = $reflection->getProperty('firstName');
    $firstNameProperty->setAccessible(true);
    $lastNameProperty = $reflection->getProperty('lastName');
    $lastNameProperty->setAccessible(true);

    self::assertSame('Jan', $firstNameProperty->getValue($objectUnderTest));
    self::assertSame('Kowalski', $lastNameProperty->getValue($objectUnderTest));
}

W porównaniu z zastosowaniem Closure::bindTo wypada podobnie jeżeli chodzi o podejście czy konsekwencje dla potencjalnego refaktoru. Więcej linii kodu negatywnie wpływa na czytelność i w wątpliwość możemy poddawać szybkość działania, co nie powinno stanowić problemu gdy ten kod tak czy inaczej nie będzie działał produkcyjnie. 

♻️ Warstwa abstrakcji na asercję

Można by dodatkowo schować taką implementację przed testem w customowej klasie Assert. Jeżeli z biegiem czasu pojawi się jakiś sposób na lepsze udostępnianie stanu niż za pomocą Closure::bindTo lub refleksji to sama klasa testowa nie będzie wymagała modyfikacji. Dodatkowo wprowadzenie specjalnej klasy asercji wpłynie na poprawę czytelności samego testu.

public function test(): void
{
    $objectUnderTest = new Person('Jan', 'Kowalski');
    
    // When
    // ...
    
    PersonAssert::sameName('Jan', 'Kowalski', $objectUnderTest);
} 

📝 Podsumowanie

Koniec końców uważam, że zastosowanie Closure::bindTo było dobrym pomysłem. Pomimo tego, że udostępniamy prywatny stan na zewnątrz, napisany test jednostkowy przynosi wartość. Co prawda jesteśmy mniej odporni na zmiany klasy poddanej testom, ale z drugiej strony nie mieliśmy zbytniego wyboru. Moglibyśmy użyć testów integracyjnych lecz nie są one tak precyzyjne jak testy jednostkowe. Niekiedy nie dysponujemy takim rozwiązaniem w projekcie co uniemożliwiłoby całkowicie otestowanie takiej klasy. Korzystanie z Closure::bindTo powinno być ograniczone tylko do absolutnej konieczności gdy nie mamy innego wyboru, jest ono obarczone pewnym obciążeniem, ale na tyle małym by w dalszym ciągu utrzymać wszystko w ryzach. 

Jedyne nad czym trzeba się zastanowić to czy weryfikacja działania obiektu jest więcej warta niż poniższe efekty uboczne:

⚠️ zmiana wewnętrznego stanu wymaga naprawienia testu

⚠️ zmiana nazwy pola sprawi że komunikat nieprzechodzącego testu nie będzie do końca jasny (UndefinedProperty)

⚠️ narzędzia do analizy statycznej mogą zgłaszać błąd w związku z nieznanym polem