Refaktoring modelu, czyli upraszczanie przez komplikowanie

W poprzednim wpisie pokazywałem jak wygląda mechaniczny refaktoring. Dziś chciałbym skupić się na tym z szerszej perspektywy. Zastanówmy się co naprawdę robi nasz biznes, gdzie jest wartość biznesowa i spróbujmy podzielić nasz problem na mniejsze składowe.

Prezentacja modelu

Przypomnę jaki zestaw klas brał udział w naszych rozważaniach.

class User:
    # ...

class Wholesaler(User):
    # ...

class Retailer(User):
    # ...

class Item:
    # ...

class Basket:
    # ...

Od razu widać, że są to osobne koncepty, ale w naszym poprzednim rozwiązaniu były ze sobą mocno związane. A nasz przykład był prosty i nie brał pod uwagę, że taki użytkownik w rzeczywistości, to prawdopodobnie pobierany byłby z sesji, bo najczęściej jest zalogowany do swojego konta i dopiero wtedy robi zakupy (albo potwierdza cały proces), bo inaczej nie miałby możliwości otrzymać rabatu.

Potencjalne problemy

Poprzedni refactoring był faktycznie dobrze zrobiony, natomiast to były tylko proste transformaty i jeśli nie zajmiemy się poprawkami na poziomie całego modelu i procesu biznesowego, to możemy zapędzić się w kozi róg.

Jakie mogą być potencjalne problemy obecnego rozwiązania?

Spuchnięty model Usera

Jeśli wrzucamy dodatkowe pola i logikę do użytkownika, to z czasem będzie On przerośnięty rzeczami, które do niego nie należą i stanie się takim workiem, na wszystko czego nie potrafimy skatalogować. Bo w zasadzie wszystko wymaga w swoim procesie użytkownika, więc to chyba dobry kierunek? No niekoniecznie i to wspólnie wypracujemy

Zbyt szeroka odpowiedzialność

Czy na prawdę odpowiedzialnością koszyka, albo użytkownika jest liczenie promocji? Pamiętajmy, że jesteśmy tylko ludźmi i mamy prawo popełniać błędy. I dlaczego chcemy obarczać błędami cały koszyk, albo całego użytkownika, jeśli popełnimy błąd w samym wyliczaniu promocji. Dodatkowo zauważmy, że cały czas zmienialiśmy klasę koszyka i użytkownika mimo iż nie zmienialiśmy niczego co bezpośrednio jest powiązane z żadnych z nich

Silne powiązania

Żeby wyliczyć cenę produktów musieliśmy uzależnić koszyk od użytkownika, a takie silne powiązania powodują, że nie będziemy mogli łatwo zastąpić żadnego z tych modułów jeśli przyjdzie taka potrzeba. Dodatkowo nie da się przetestować tego w odosobnieniu i testy prostego działania koszyka wymagają zaciągnięcia wszystkich modułów. Tak właśnie powstaje wielka kula błota i kod legacy, którego nikt nie chce utrzymywać ;)

Jaki problem rozwiązujemy

Zastanówmy się jaka jest prawdziwa potrzeba biznesowa w podanych wcześniej przykładach i wymaganiach. Wiemy, że jedyną stałą rzeczą jest zmiana. Wiemy też, że nie mamy możliwości przygotować się na wszystkie możliwe zmiany, ale naszą odpowiedzialnością jest przygotowanie się na te zmiany, które są najbardziej prawdopodobne. Jakie zmiany tutaj następowały? Zmiana promocji w zależności od ilości zakupionych produktów. A jakie zmiany mogą wystąpić w przyszłości? Prawdopodobnie inne promocje, być może jakieś tymczasowe na black friday? Skoro tak, to spróbujmy zachować się jak prawdziwy partner biznesowy i zasugerujmy coś, co pozwoli efektywnie zarządzać promocjami. Przy okazji odplączemy od siebie wspomniane wcześniej koncepty.

Propozycja rozwiązania wyliczania promocji

Rozwiązując problemy na wyższym poziomie warto dopytywać o przyszłe wymagania, które mogą zburzyć nasz model. Oczywiście ktoś może zarzucić, że nie powinniśmy się przejmować tak wcześnie wszystkimi wymaganiami, bo przecież tak mówią dobre praktyki, jak KISS (Keep It Simple Stupid), czy YAGNI (You aren't gonna need it). Ale właśnie celem takich teoretycznych komplikacji jest wyłonienie generycznego problemu, a generyczne problemy rozwiązuje się po prostu łatwiej. Odnosząc to do naszego problemu, to w zasadzie naliczanie promocji to nic innego jak przeniesienie funkcji matematycznej, lub prostej tabelki na kod, a to umiemy robić bardzo dobrze. Jak to może wyglądać?

from dataclasses import dataclass


@dataclass
class Threshold:
    amount: int
    value: int


@dataclass
class Discount:
    thresholds: list[Threshold]


class Promotions:
    @staticmethod
    def calculate(amount: int, discount: Discount) -> int:
        for threshold in sorted(discount.thresholds, key=lambda x: x.amount, reverse=True):
            if threshold.amount <= amount:
                return threshold.value
        return 0

Takie wyliczanie promocji jest proste i spełnia wszystkie wymagania, przed którymi wcześniej byliśmy stawiani. Dodatkowo jest gotowe na przyszłe promocje. A skąd bierze się taka promocja? No i to jest właśnie najlepsze! Nie musimy się tym przejmować. Czy będzie to importowane z excela, czy zrobimy prosty CRUDOwy interfejs, albo nawet jakiś zaawansowany javascriptowy edytor z możliwością rysowania wykresów. To jest nieistotne, bo mamy to ładnie zamknięte, a samo wyliczanie promocji jest banalnie proste, bo jest to prosta funkcja z wejściem i wyjściem. Nawet zamykanie tego w klasie nie jest koniecznie, ale tutaj klasy nie traktowałem pod względem obiektowym, a bardziej jako formę organizacji kodu.

Połączenie modułów, czyli relacja z koszykiem

A jak połączyć to z koszykiem? A no nijak :) To nie jest odpowiedzialność koszyka, żeby wyliczać promocje. Koszyk powinien otrzymać wartość rabatu w parametrze. Dzięki takiemu podejściu całość będzie luźno powiązania, co ułatwi testowanie! Przy okazji pozbądźmy się relacji do użytkownika.

@dataclass
class Item:
    name: str
    price: int
    id: uuid.UUID = field(default_factory=uuid.uuid4)


class Basket:

    def add(self, item: Item):
        self._items[item.id] = item

    def total_cost(self, promotion: int):
        price = sum([item.price for item in self._items.values()])
        return price * (1 - float(promotion)/100)

Połączenie modułów, czyli relacja do użytkownika

Uważny czytelnik szybko może zauważyć, że jest to nieco wyidealizowany przykład, bo nie mam żadnego powiązania z użytkownikiem, więc to nie ma prawa działać! Ale tu również się zastanówmy, czy na prawdę potrzebujemy zapraszać naszego użytkownika do modułu promocji, który jest taki prosty? Czy na pewno użytkownik w module do promocji, to ten sam użytkownik, który loguje się do systemu? Zaryzykowałbym stwierdzenie, że szybciej możemy to określać przez relację jakichś roli, bądź grup. Czyli jedyne co musimy zrobić, to dodać nowy moduł, który odpowiednio pobierze użytkownika pasującego do danej roli. Najprostsza możliwa implementacja może wyglądać następująco:

from dataclasses import dataclass

from shopping import User


@dataclass
class Client:
    role: str


class Clients:
    @staticmethod
    def get_client(user: User):
        return Client(role=user.status)

Połączenie modułów, czyli realizacja procesu

Wygląda, że mamy już gotowe prawie wszystkie klocki. Musimy teraz na ich podstawie zbudować gotowe rozwiązanie. A więc całość mogłaby przebiegać następująco. Nie chcę implementować całego mechanizmu requestów, więc dla uproszczenia przyjmijmy, że user jest po prostu zapisany jako atrybut obiektu. I jeszcze jedna mała uwaga. Żeby całość była maksymalnie zbliżona do faktycznego rozwiązania z request dodałem również pseudo repozytorium, żeby z niego pobierać rabaty.

A więc podsumujmy po kolei jakie mamy moduły i za co odpowiadają

Client, czyli reprezentacja kupującego

Bardzo prosty moduł, którego zadaniem jest tłumaczenie użytkownika z sesji na klienta, który jest zrozumiały w obrębie procesu zakupowego. Jest to o tyle istotne, że klient (osoba robiąca zakupy) może posiadać szereg różnych atrybutów i niekoniecznie chcemy reprezentować to przez dziedziczenie jak wcześniej. Może okazać się, że na przykład na twoją rolę wpływa to czy polubiłeś stronę na social mediach. Może twoja rola jest płynna w zależności od twojej historii, a może po prostu biznes zmieni zdanie co do sposobu wybierania roli. Dlatego zamknięcie tego w osobnym komponencie wydaje się sensownym posunięciem. Na razie implementacja wygląda następująco

from dataclasses import dataclass

from user import User


@dataclass
class Client:
    role: str


class Clients:
    @staticmethod
    def get_client(user: User):
        return Client(role=user.status)

A co jeśli pojawią się wymagania, które zepsują nasz model? Co jeśli na przykład wartość rabatu będzie uzależniona od poprzednich zakupów, na przykład każdy klient, który zakupił u nas 100 produktów staje się klientem VIP i dla niego z automatu mamy 3% rabatu, nawet jeśli kupuje tylko jedną sztukę. Czy wtedy poprzedni model nie byłby lepszy? A no nie! Zdecydowanie nie!

Przypomnę, że ten użytkownik to nieco inny byt niż użytkownik, który się loguję do systemu. Charakteryzuje się innymi atrybutami i robi inne operacje w systemie. To, że jest to ta sama osoba nie znaczy, że musimy robić jedną wielką klasę, która pokryje wszystkie możliwe przypadki. Tutaj potencjalnym rozwiązaniem byłoby przechowywanie w kliencie kopii zakupionych przedmiotów, czyli na przykład coś takiego

@dataclass
class Item:
    price: float 
    name: str


@dataclass
class Client:
    role: str
    items: list[Item]

A do promocji przekazywalibyśmy dwie wartości. Pierwsza to zakupione wcześniej pozycje, a druga to produkty obecnie w koszyku. Jest to decyzja lokalna i o ile wymaganie biznesowe nie wpływa faktycznie na oba moduły, to nie powinniśmy zmieniać ich wzajemnie przez przypadek.

Promocja, czyli matematyka w pudełku

Dzięki temu, że zamknęliśmy promocję w pudełku mamy bardzo prosty sposób testowania całości. Dodatkowo proste repozytorium, otwiera zupełnie nowe możliwości dla biznesu. Już nie muszą męczyć programistów przy każdej zmianie wymagań, tylko sami mogą eksperymentować z nowymi wartościami. I dodatkowym bonusem jest to, że sami mogą sobie definiować promocje tymczasowe, jak np. black friday, bo mogą sami sobie włączyć taką promocję i wyłączyć po kilku dniach.

Teraz wyobraźmy sobie, że przyszłoby wymaganie na utrzymanie 10 progów dla każdego klienta, albo wprowadzenie nowego rodzaju klienta. Obecnie mechanizm promocji jest od tego agnostyczny i nawet nie będzie musiał być zmieniany! W miarę wystarczająca implementacja dostępna jest poniżej

from dataclasses import dataclass
from datetime import datetime


@dataclass
class Threshold:
    amount: int
    value: int


@dataclass
class Discount:
    thresholds: list[Threshold]
    role: str
    date_from: datetime
    date_to: datetime


class EmptyDiscount(Discount):
    def __init__(self):
        self.thresholds = []


class DiscountRepository:
    def __init__(self):
        self._discounts = []

    def add(self, discount: Discount) -> None:
        self._discounts.append(discount)

    def get_current(self, role) -> Discount:
        for discount in self._discounts:
            if discount.date_from < datetime.now() < discount.date_to and role == discount.role:
                return discount
        return EmptyDiscount()


class Promotions:
    @staticmethod
    def calculate(amount: int, discount: Discount) -> int:
        for threshold in sorted(discount.thresholds, key=lambda x: x.amount, reverse=True):
            if threshold.amount <= amount:
                return threshold.value
        return 0

Dodatkowo zastosowałem tu tak zwany wzorzec "pustego" obiektu, czyli zamiast zwracać None i typować to jako Optional podczas pobierania zniżki, zwracamy obiekt o identycznym interfejsie, ale z pustą listą progów zniżkowych. Taki trik pozwoli mi uprościć interfejsy oraz uniknąć defensywnego sprawdzania "if not None" na każdym poziomie. Polecam :)

A czy tutaj mogą pojawić się jakieś wymagania, ktore wywrócą nasz model do góry nogami? Na pewno :) Jedno z takich wymagań wspomniałem przy okazji omawiania klasy klienta, czyli promocja naliczana na podstawie wcześniej kupionych pozycji. Tutaj trzeba zmienić przekazywanie parametru z ilość, na faktyczną listę przedmiotów. Innym takim wymaganiem może być na przykład wprowadzenie logiki zniżki pod warunkiem, że użytkownik kupi w jednym procesie zakupowym 2 książki tego samego autora. Zmiana jest zlokalizowana w jednym miejscu. Możemy ją prosto przetestować testami jednostkowymi, więc ryzyko wprowadzenia błędu jest znacznie mniejsze niż w przypadku przerośniętego "Usera" i "Basketa"

Użytkownik, jako zaślepka

Całość pokazywałem jako uproszczony przykład, więc nie umieściłem użytkownika w request, ale taki przykładowy użytkownik może mieć zdecydowanie inne pola. I to widać już na naszym przykładzie, gdzie jawnie zmieniłem atrybut "status", a "rolę"

import uuid
from dataclasses import dataclass, field


@dataclass
class User:
    name: str
    status: str
    id: uuid.UUID = field(default_factory=uuid.uuid4)

Koszyk, czyli schowek na przedmioty do kupienia

Takie odseparowanie od siebie promocji, użytkowników i koszyka spowodowało, że koszyk znowu jest prostym CRUDzikiem, który łatwo można testować. Nie musimy go dotykać przy jakiejkolwiek z proponowanych wcześniej modyfikacji. Wszystkie potrzebne informacje dostaje z zewnątrz, dzięki temu zdecydowanie zwiększamy kohezję tego koszyka, co przekłada się na jego utrzymanie i czytelność.

import uuid
from dataclasses import dataclass, field


@dataclass
class Item:
    name: str
    price: int
    id: uuid.UUID = field(default_factory=uuid.uuid4)


class Basket:
    def __init__(self):
        self._items = {}

    def items_count(self) -> int:
        return len(self._items)

    def add(self, item: Item):
        self._items[item.id] = item

    def total_cost(self, promotion: int):
        price = sum([item.price for item in self._items.values()])
        return price * (1 - float(promotion)/100)

Trudno mi znaleźć jakiś przykład, który wywróci koncept koszyka. Może kody rabatowe? Użytkownik może wprowadzić kod rabatowy z innej strony, żeby naliczyć promocję? No właśnie ... promocję, czyli to raczej pasuje do omówionego wcześniej konceptu. No to może promocja 1% dla nowych klientów? Ale to też pasuje bardziej do Clienta. Takie szukanie antywymagań, to bardzo ciekawe zadanie, które konfrontuje pomysł z rzeczywistością. Zachęcam do podonych eksperymentów na prawdziwych projektach. Samo testowanie modelu "na kartce" jest stosunkowo tanie, a pozwoli szybko wycofać się z błędnych decyzji.

Proces zakupowy spajający całość

Tu mamy coś czego wcześniej nie było. Ten nowy element to niejako koordynator całego procesu biznesowego. I to jest jedyny punkt, który ma informację o wszystkich obiektach. Wcześniej tego nie było, bo prezentowałem sam interfejs koszyka. Tu można traktować to po prostu jak kontroler, czy serwis aplikacyjny, czyli w zasadzie punkt wejścia do naszej aplikacji. Jego kod również jest bardzo prosty

from typing import Optional

from basket import Basket, Item
from user import User
from client import Clients
from promotion import Promotions, DiscountRepository


class Shopping:
    def __init__(self, user: User, discount_repository: Optional[DiscountRepository] = None):
        self._user = user
        self._basket = Basket()
        self._discount_repository = discount_repository or DiscountRepository()

    def add_to_basket(self, item: Item):
        self._basket.add(item)

    def buy(self):
        client = Clients.get_client(self._user)  # symulujemy request.user
        discount = self._discount_repository.get_current(client.role)
        promotion = Promotions.calculate(amount=self._basket.items_count(), discount=discount)
        return self._basket.total_cost(promotion)

Plusy refaktoryzacji procesu

Ktoś może zarzucić, że teraz jest więcej klas niż wcześniej. I będzie miał rację! Ktoś może zarzucić, że teraz powstały nowe byty, których wcześniej w kodzie nie było. I znowu będzie miał rację! Teraz nasz kod odzwierciedla to co dzieje się w naszym biznesie. Obecne rozwiązanie jest łatwiejsze w testowaniu, bo każdy moduł może być testowanu w odseparowaniu, więc testów będzie mniej. Mamy szereg testów jednostkowych i kilka testów integracyjnych naszego Shopping jako koordynatora, żeby sprawdzić czy działość współgra ze sobą. Taki test może wyglądać następująco

from datetime import datetime, timedelta
import unittest

from basket import Item
from user import User
from promotion import DiscountRepository, Discount, Threshold
from shopping import Shopping


class TestShopping(unittest.TestCase):
    def setUp(self):
        user = User(name="ddeby", status="wholesaler")
        self.discount_repository = DiscountRepository()
        self.shopping_service = Shopping(user=user, discount_repository=self.discount_repository)

    def test_buy_10_products_without_promotion(self):
        for _ in range(10):
            self.shopping_service.add_to_basket(Item(name="name", price=10))
        self.assertEqual(self.shopping_service.buy(), 100)

    def test_buy_10_products_with_promotion(self):
        self.discount_repository.add(Discount(
            role="wholesaler",
            date_from=datetime.now() - timedelta(days=1),
            date_to=datetime.now() + timedelta(days=1),
            thresholds=[Threshold(10, 10)]
        ))
        for _ in range(10):
            self.shopping_service.add_to_basket(Item(name="name", price=10))
        self.assertEqual(self.shopping_service.buy(), 90)


if __name__ == '__main__':
    unittest.main()

Kolejnym plusem jest utrzymywalność i rozszerzalność. Obecny design pozwala na rozbudowę w wielu kierunkach i całość będzie zamknięta w jednym miejscu, więc wycofanie się z błędnej decyzji będzie relatywnie łatwiejsze.

Dodatkowym przyjemnym efektem ubocznym może być potencjalnie rozproszenie systemu. Zauważ, że teraz nic nie stoi na przeszkodzie, żeby taki moduł promocji, stał jako osobna usługa, do której łączymy się przez klienta HTTP, albo trzymamy kopię u siebie i synchronizujemy całość asynchronicznie.

Ale przecież nie pobieramy płatności!

Na koniec chciałbym jeszcze zauważyć, że nie pobieramy płatności od użytkownika po wykonaniu zakupów, a jedynie zwracamy wartość koszyka. Może to być trochę mylące. Ale to miał być tylko przykład. Jak dodać takie pobieranie z portfela? Na przykład w następujący sposób

class Shopping:
    # ...
    def buy(self):
        # ...
        payer = Payments.get_payer(self._user). 
        wallet = Wallet.pay(payer, amount)
        # ...

Jak widzisz, nie dotykamy innych usług.

A co jeśli i w portfelu pojawi się zmiana wymagań? Na przykład wprowadzimy mechanizm w którym niektórzy klienci będą mogli wejść na debet, bo im ufami. Znowu zmiana jest zlokalizowana w jednym miejscu.

Możemy dowolnie komponować nasz proces. Możemy przestawiać kolejność wykonanych akcji. Możemy w każdym module mieć inną lokalną architekturę i tak długo jak ich API jest niezmienne, to całość powinna działać bez zarzutu.

Refaktoring modelu

I to jest właśnie refaktoring na poziomie całego procesu biznesowego. Nie zmieniliśmy naszego kodu tylko mechanicznie, ale podeszliśmy bardziej kompleksowo. Odwzorowaliśmy nasz biznes w kodzie. Uniknęliśmy złożoności przypadkowej, której niestety jest zbyt dużo w naszych projektach. Niestety taki proces, przez który tu przeszliśmy wymaga bardzo dobrego zrozumienia domeny biznesowej w której się poruszamy, bo dopiero wtedy będziemy mogli sensownie zaproponować niezależne komponenty, które mogą ze sobą fajnie współpracować. Nie mniej takie podejście czyni z nas bardziej partnerów aniżeli prostych rzemieślników, do klepania kolejnych tasków. Tego życzę Wam, żebyście dostarczali jak największą wartość do Waszych projektów, bo to po jest po prostu ciekawe ;)

Podsumowanie

Ktoś może zarzucić, że przykład jest bardzo akademicki, bo przy pierwszej zmianie wymagań powinno się robić taki interfejs do budowania promocji. Rzeczywistość jest niestety bardziej brutalna. Pracując pod presją czasu, często gubimy takie proste i oczywiste kwestie. Wymagania często napisane są w taki sposób, że nie widać tego na pierwszy rzut oka i naiwne modelowanie powoduje, że powstają potworki typu ClientVipWithoutPromotion, który dziedziczy po kliencie VIP, ale nie ma promocji, bo zbyt często zalegał z płatnościami, ale musimy zostawić mu status VIP, bo w module rozliczeniowym dalej zostają mu możliwe debety do zaciągnięcia. Sam miałem przyjemność w systemie właśnie z koszykiem i tam całość była przerośnięta kolejnymi wymaganiami. Funkcja dodawania do koszyka miała kilka tysięcy linijek, do środka przekazywane były dziesiątki parametrów, niektóre przez referencje, przez co modyfikacja stanu była nieprzewidywalna w innej części systemu. I sam pomysł na wpis zrodził się kiedy prawie 2 dni szukałem błędu, który polegał na tym, że jak ktoś doda produkt do koszyka, następnie doda drugi, a ten pierwszy usunie i będzie powtarzać cały proceś, to po 5 przejściach okazywało się, że jeden z tych produktów jest za darmo. To było nie do przyjęcia, natomiast na błąd złożyło się kilka czynników

• lazy loading mówił, że mam więcej obiektów niż w rzeczywistości było w koszyku (nie przeliczał się ponownie i count zwracał starą wartość)
• mutowalne słowniki nadpisywały stan wewnętrznych kolekcji danych, co wprowadzało nieporządane efekty uboczne
• odtworzenie błędu w testach jednostkowych było niemal niemożliwe, bo wymagało postawienia niemal całego projektu, bo się okazało, że Użytkownik miał relację nie tylko do Koszyka i Przedmiotów, ale też na przykład do Płatności, Historii i wielu innych. Test wymagał dodania większej ilości mocków niż faktycznie linijek kodu sprawdzającego
• kod był przerośnięty i cała logika siedziała w jednej funkcji ... oczywiście w koszyku

Żałuję, że wtedy nie byłem na tyle świadomy co teraz, bo uniknąłbym wielu problemów, które sam wtedy stworzyłem. Dlatego zachęcam do przemyśleć po tym wpisie, bo być może w waszych projektach kryją się podobne potwory?

Jeśli zaciekawił Was ten wpis, to zapraszam do odcinku naszego podcastu, w którym z wspólnie z Marcinem szerzej podchodzimy do tego zagadnienia