Jak programować obiektowo? cz. 4 – metod ciąg dalszy

Sporo czasu minęło od ostatniego wpisu z serii i chciałbym Was za to serdecznie przeprosić. Zdaję sobie sprawę z tego, że wielu z Was pewnie już przyjęło za pewnik, że kolejny się nie pojawi i będzie to jeszcze jedna seria, której autor nie zaczął tego, co rozpoczął. Chciałem Was jednak zapewnić, że pomimo sporych opóźnień i problemów organizacyjnych na przestrzeni kilku ostatnich miesięcy, moje plany pozostają nie zmienione. I mam nadzieję, że Wasza chęć zgłębienia aspektów programowania zorientowanego obiektowo również 🙂

Żeby nie przedłużać przejdę do tego, na co tak długo kazałem Wam czekać. Dzisiaj obiecany kod, który prezentuje to, co omówiłem w poprzednim wpisie. Tak, więc najwyższa pora aby zobaczyć jak to wszystko sprawdza się w praktyce.

Przed przeczytaniem dalszej części radzę odświeżyć sobie pamięć i spojrzeć na część drugą, ponieważ dzisiejszy przykład będzie bezpośrednią kontynuacją tego, co tam zaczęliśmy tworzyć.

Kod daj mnie tu…

Jak już pamiętacie tworzymy system dla firmy transportowej. Jednym z głównych części składowych ma być baza kontrahentów. Przy dodawaniu kontrahenta użytkownik musi określić jego NIP oraz unikalną nazwę. Musi istnieć możliwość wystawiania i przesyłania drogą mailową faktur za usługi firmy oraz musi być możliwość wykonania przelewu na konto kontrahenta z którego usług korzystała firma. Firma jest międzynarodowa, więc musi istnieć możliwość obsługi różnych walut. Niektórzy kontrahenci mają kilka osób, do których chcą aby były wysyłane faktury. Dodatkowo powinna istnieć możliwość określenia adresu siedziby kontrahenta.

Po przeanalizowaniu powyższego można dojść do wniosku, że „kontrahent bez adresu” oraz funkcjonalność wystawiania faktur nie jest czymś co da się zrealizować. Decydujemy się na rozmowę z klientem i wspólnie decydujemy się wprowadzić zmianę w wymaganiach – zdanie „powinna istnieć możliwość określenia adresu siedziby kontrahenta” zmieniamy na „adres siedziby kontrahenta jest niezbędny do jego utworzenia”.

Implementacja wymagań

Wiemy, że każdy kontrahent musi posiadać NIP, unikalną nazwę oraz adres siedziby czyli istnienie kontrahenta bez tych danych nie ma żadnego sensu. Dzięki temu jesteśmy w stanie określić jakie parametry powinien przyjmować konstruktor:

Kolejną funkcjonalnością, którą mamy zaimplementować jest możliwość wystawiania faktur. Aby to zrobić potrzebujemy pobrać dane, które są niezbędne do wystawienia faktury, czyli:

  • Nazwa firmy
  • ulica
  • kod pocztowy, miasto
  • kraj
  • NIP firmy

Jak łatwo zauważyć, oprócz informacji nt. wartości atrybutów, które są typów prostych obiektu klasy Contractor potrzebujemy również informacji z obiektu agregowanego klasy Address. W wyniku tej potrzeby może powstać nam taki kod:

Kiedy już posiadamy tę metodę, możemy utworzyć analogiczną metodę w klasie Contractor:

I na koniec tego paragrafu pytanie do Was – jakie są minusy takiego rozwiązania? Czy w swojej aplikacji zdecydowalibyście się na inne rozwiązanie? Jakie?

Wysyłanie maili

Ok, dane już mamy, więc do wygenerowania faktury już niczego więcej nam nie trzeba. Fakturę jednak trzeba będzie jeszcze przesłać. Tylko gdzie? Na jaki e-mail? W pierwszej iteracji decydujemy się na wysyłanie faktury do każdego zdefiniowanego kontaktu, więc po pierwsze musimy dobrać się w jakiś sposób do ich adresów mailowych:

I na koniec musimy zwrócić adresy mailowe wszystkich kontaktów kontrahenta:

A gdzie numer konta?

Ostatnią rzeczą, którą mamy zaimplementować jest umożliwienie wykonywania przelewu. Żeby taka operacja była w ogóle możliwa niezbędny jest nam numer konta bankowego kontrahenta, z tym, że to konto musi obsługiwać określoną w przelewie walutę.

Zacznijmy od metod klasy BankAccount. Po pierwsze, musimy mieć możliwość sprawdzenia, czy dane konto bankowe obsługuje pożądaną walutę:

Po drugie, musimy mieć możliwość pobrania numeru tego konta bankowego:

I po trzecie, należy zaimplementować możliwość wyciągnięcia tych danych z obiektu klasy Contractor:

Dla uproszczenia logiki zdecydowałem się na zwracanie pierwszego numeru konta wspierającego daną walutę. Oczywiście w przypadku realizacji takiej aplikacji trzeba byłoby zastanowić się, czy dopuszczamy, aby kontrahent posiadał tylko jedno konto dla określonej waluty czy też zwracamy kolekcję z numerami kont. W przypadku dopuszczenia istnienia większej ilości kont dla danej waluty niezbędne byłoby również przemyślenie tego, kto lub co może decydować o wykorzystaniu konkretnego numeru konta. Jakieś globalne ustawienia? Jakiś algorytm z odpowiednimi regułami? Albo pozwalamy użytkownikowi wybrać jedno konto z listy poprawnych Takie rzeczy bezwzględnie należałoby przedyskutować z klientem.

Drugą rzeczą, nad którą należy się zastanowić jest sposób obsługi braku danego konta. Wyrzucenie wyjątku czy null? Czy może jeszcze coś innego. Ja zdecydowałem się na wyrzucenie wyjątku, ale może null byłby odpowiedniejszy? A może wszystko zależy od wymagań? Jaka jest Wasza opinia?

Operacje na obiektach

Oczywiście musimy jeszcze zaimplementować kilka metod, które pozwolą nam na edytowanie obiektów klasy Contractor.

Zacznijmy od klasy Address. Z wymagań wiemy, że kontrahent musi posiadać jeden adres, a co za tym idzie nie możemy go usunąć lub dodać kolejnego. Nic jednak nie stoi na przeszkodzie, aby taki adres kontrahenta zmienić:

Następnie musi istnieć możliwość wykonywania operacji na kontach bankowych klienta. Numeru konta bankowego raczej nie zmienimy, a dla uproszczenia zakładam, że waluty, którą obsługuje dane konto bankowe, również nie jesteśmy w stanie zmienić. Tak więc pozostaje nam dodawanie i usuwanie:

Działanie pierwszej metody jest chyba jasne. Niemniej jednak, czy nie wydaje się Wam, że czegoś w niej brakuje?

Druga metoda szuka istniejącego konta i jeżeli je znajdzie, to usuwa, a następnie zwraca wartość true. W innym przypadku wyrzuca wyjątek informujący o tym, że nie istnieje konto bankowe obsługujące daną walutę. Dlaczego wyjątek? Bo tak naprawdę staramy się wykonać niedozwoloną operację, czyli usunięcie czegoś, czego tak naprawdę nie ma. A dlaczego nie false? Bo jeżeli chcecie sprawdzić istnienie danego konta to, w myśl Single responsibility principle powinna powstać jeszcze jedna metoda, która umożliwia takie sprawdzenie.

W przykładzie zdecydowałem się na określanie konta do usunięcia za pomocą obsługiwanej waluty (zakładając, że może istnieć jedno konto per waluta). Oczywiście działanie takiej operacji w rzeczywistej aplikacji mogłoby opierać się na numerze konta bankowego (a może wręcz powinno?). Przy założeniu jednego konta dla danej waluty efekt końcowy jest ten sam, ale zastanówcie się jak to ma się do zasady Open/Closed?

Zarządzanie osobami kontaktowymi

Ponieważ w naszym przykładzie osoba kontaktowa to nazwa (imię i nazwisko) wraz z powiązanym z nią adresem email, również zdecyduję się na uproszczenie i uniemożliwienie edycji wartości tych atrybutów dla istniejących osób kontaktowych. W związku z tym mamy również do zaimplementowania dodawanie i usuwanie osób kontaktowych:

Dodawanie osoby kontaktowej jest proste.

Natomiast implementując metodę usuwającą osobę kontaktową można zauważyć, że niezbędna do jej wykonania jest metoda getName(). Tak więc mamy:

Po utworzeniu tych wszystkich metod warto zastanowić się również nad edycją wartości atrybutów typów podstawowych obiektów klasy Contractor. Mamy dwa takie atrybuty: name i NIP. W tym momencie musimy się dowiedzieć w jaki sposób taka edycja będzie się odbywała. Czy będą to dwie osobne akcje tzn. użytkownik systemu edytuje albo nazwę kontrahenta albo NIP. Czy może będzie to edycja wszystkich danych. Zazwyczaj zezwala się na pełną edycję i zakładam, że w tym przypadku tak jest. Dlatego też wystarczy nam tylko jedna metoda:

Kilka uwag na koniec

Gdyby ten kod był implementowany w rzeczywistej aplikacji to należałoby pamiętać jeszcze o kilku rzeczach:

  • warto sprawdzać typ parametrów podstawowych (pozostałe można w PHP typować) i wyrzucać wyjątek InvalidArgumentException w przypadku, gdy są nieprawidłowe
  • metody addBankAccount() i addContactPerson() są niewystarczające, ponieważ należałoby jeszcze uwzględnić, co w przypadku dodawania takiego samego obiektu, bądź obiektu, który posiada takie same wartości dla pól unikalnych (np. numer konta bankowego)
  • wszelkie wątpliwości należy rozwiewać poprzez rozmowę z klientem, a nie na podstawie intuicji, ponieważ ta niestety może nas zawieść.

I jeszcze na koniec zapraszam do odpowiedzi na zadane przeze mnie pytania, zapraszam do dyskusji nad stworzonym kodem, ciekaw jestem Waszych uwag i przemyśleń.

Pozostałe artykuły z cyklu

Jestem fanatykiem obiektowego programowania i nieustannie pogłębiam swoją wiedzę we wszelkich tematach z nim związanych. Wszystko czego się dowiem konfrontuję z rzeczywistością, ponieważ teoria, która nie ma odzwierciedlenia w praktyce, traci swój sens tam, gdzie zaczyna się praca programisty :)

  • poh

    art całkiem spoko, ale razi funkcjonalność != funkcja – http://wittamina.pl/wp-content/uploads/2013/06/funkcjonalnosc_znaczenie.jpg

  • Potfur

    Czy mogę pokrytykować? 🙂

    • nrm

      Myślę, że w ramach wspólnej nauki każda konstruktywna krytyka jest mile widziana i Sebastian nie walnie focha 😉

      • Potfur

        Wszystko poniżej to jedno wielkie IMHO.

        $name, $nip, $currency powinne być reprezentowane jako ValueObject, tak jak to jest z adresem.

        ContactPerson, BankAccount powinny mieć zdefiniowane interfejsy. ContactPerson aż się prosi o to.

        Address nie powinien mieć metody getDataToInvoice() – to powinn być w innym obiekcie odpowiadającym za faktury, który umie czytać addressy.

        Domyślam się że na potrzeby przykładu uproszczono kilka rzeczy… ale trochę za bardzo.

        • Sebastian Malaca

          Oczywiście, że się (nie) obrażę 🙂
          Tak naprawdę to liczyłem na te komentarze 🙂

          >> $name, $nip, $currency powinne być reprezentowane jako
          >> ValueObject, tak jak to jest z adresem.
          Zgadzam się, z każdym, ale czy nie wydaje Ci się, że na danym etapie byłby to taki „lekki” overengineering. Oczywiście w pełnie masz rację co do Currency. To powinna być osobna klasa i tutaj nie ma usprawiedliwienia.
          Co do Name, to często w aplikacjach programiści/projektanci decydują się na uproszczenie i sprowadzają to do stringa. Zwróć uwagę, że osobna klasa domenowa powinna zawierać szereg reguł stwierdzających kiedy jest poprawna. Powinna o to dbać. Czy nie sądzisz, że takie uproszczenie (string zamiast obiektu) jest akceptowalne?
          Co do NIPu to tutaj sprawa ma się odrobinę inaczej i z racji dość specyficznych reguł warto byłoby rozważyć wydelegowanie tego do VO już na tym etapie. Taki zabieg podkreślałby złożoność pola.
          Punkt dla Ciebie 🙂

          >> ContactPerson, BankAccount powinny mieć zdefiniowane
          >> interfejsy. ContactPerson aż się prosi o to.
          Po co? A gdzie tu jakieś logiczne powiązanie? Czekam na rozwinięcie.

          >> Address nie powinien mieć metody getDataToInvoice() –
          >> to powinn być w innym obiekcie odpowiadającym za
          >> faktury, który umie czytać addressy

          Kolejny punkt dla Ciebie. Oczywiście ani tutaj, ani w klasie Contractor taka metoda nie ma prawa bytu i jest dokładnie tak jak napisałeś. Dość popularnym sposobem jest stworzenie metody, która zwraca Immutable DTO i po sprawie. Wtedy obiekt udostępnie możliwe do przerobienia w dowolną formę informację na swój temat.

          >> Domyślam się że na potrzeby przykładu uproszczono kilka rzeczy… ale trochę za bardzo.

          Patrząc na (wysoką) jakość komentarzy jednak jestem skłonny się z Tobą nie zgodzić. Taki przykład udowodnił jedynie, że jest mnóstwo osób, które są chętne do dyskusji z zaprezentowanym kodem, wyciągają odpowiednie wnioski i dobrze to uzasadniają. A nie zakładają, że ktośtam sobie cośtam napisał i na pewno ma rację.
          Jakby na to nie patrzeć, te komentarze świadczą o bardzo dobrym zrozumieniu tematu jakim jest OOP.
          A że mi się dostanie przy okazji? Jak się taki kod publikuje, to z taką reakcją trzeba się liczyć 🙂

          • Potfur

            Name: VO gdyż – nazwa firmy, imię, nazwisko w osobnych polach, w razie potrzeby rzutowane na string wg ustalonej kolejności.
            Sprowadzenie tego do pojedynczego stringu można zrobić kiedy nazwa pełni rolę wyłącznie informacyjną – np. nick na forum.

            ContactPerson: gdyż osobą kontaktową może być prosty adres e-mail, inny kontrahent czy inna encja systemu.

            BankAccount: podobnież, różne rodzaje kont, różne formaty numerów – różna walidacja.

          • Sebastian Malaca

            Name: Ok, masz rację. Jeżeli na nazwę patrzymy jako na zbiór takich atrybutów, to z pewnością jest to dobry kierunek. Ja to rozumiałem jako nazwę firmy (a że przykład jest mój… :P). Niemniej jednak, jestem przekonany, że jest spora szansa, że gdyby to był rzeczywisty kod, to w trakcie dalszych rozmów z klientem wyszłoby, że Twoje rozwiązanie jest tym, które my chcemy, a które klient od początku miał na myśli 🙂

            Co do ContactPerson i BankAccount to widzę, w którą stronę idzie Twoje myślenie, jednak osobiście pozostawiłbym to w obecnej formie i zdecydował się na refaktoryzację, jeżeli rzeczywiście zaszłaby taka potrzeba. Jednak na etapie analizy warto by już to przedyskutować z klientem skoro pojawia się taka myśl. Rozwiałoby to wszelkie wątpliwości 🙂

  • Morales

    Tylko po co ten prefix przed nazwami atrybutów?
    Nie lepiej samo name zamiast _name?

    • aPoCoMiLogin

      Wydaje mi się że to przyzwyczajenie do zenda albo starych wersji php w których nie było kwalifikatorów widoczności. Ale to było tak dawno, że bardziej obstawiam zenda.

    • Rafał Łużyński

      w Pythonie używamy _ aby zaznaczyć, że dana metoda czy atrybut jest jedynie do użytku wewnętrznego, czyli masz ich nie tykać z zewnątrz, tyle, że w Pythonie nie ma „private”. Jednak ja nie widzę w tym nic złego również w PHP i jest to kwestia gustu imo.

      • nrm

        No właśnie, problem w tym, że teraz w PHP jest wojna w tego typu sprawach: panuje przekonanie, że NIE WOLNO „se” robić wg. gustu tylko wszystko musi być PRAWILNIE jak PSR/itp. przykazał 😉

        • Rafał Łużyński

          Ja jestem za ujednoliceniem zasad pisania kodu, ale zasady wewnątrz teamu mają większe priority niż zewnętrzne zasady. Także, jeśli team ustalił, że oznaczamy zmienne prywatne „_”, to powinno się to konsekwentnie stosować. Jednak jeśli mamy możliwość stosować PSR w nowym projekcie, to lepiej się do niego podporządkować.

        • satyr

          Nie nazwałbym tego problemem. Myślę, że do dobry kierunek. Mnie osobiście od zawsze bolał bałagan i brak standaryzacji w php. Oczywiście, trzymanie się PSR-2 w 100% czasem wygląda dziwnie (np łamanie deklaracji klas jeśli masz kilka interfejsów), jednak ~95% podanych tam zasad pozwala to nie załamywać się nad bałaganem w kodzie po przejęciu projektu od kogoś. Nie zgadzam się z tym, że zasady powinny być ustalane per team. Wg mnie powinny być ustalane na poziomie całej firmy. Nie wyobrażam sobie sytuacji gdzie w firmie w którym pracuje np 6 zespołów każdy z nich wymyślał sobie swoje odstępstwa od standardu. Przy przetasowaniach zespołów/projektów programiści będą o tych odstępstwach zapominali.

          PSR2 jest ok. Wygląda przyzwoicie. Pamiętacie mieszanie underscorów z Kohany z camel case jak używało się jakiegoś zewnętrznego vendora? To był dopiero koszmar 😉

          Dyskutować wg mnie ma sens tylko na temat PSR2, bo PSR0 i PSR1 to oczywista oczywistość która musiała się wydarzyć.

    • Sebastian Malaca

      No lepiej, ale przyzwyczajenie wygrywa 🙂 Jak zaczynałem, to (jak napisał @aPoCoMiLogin) w większości bibliotek i FW decydowano się na taki zabieg, więc i ja go przyjąłem 😛

  • Bartosz Grzybowski

    Jedna rzecz, jeżeli chcemy kogoś uczyć to od razu sądzę, że nawet jeżeli nie lubisz / nie zgadzasz / nie używasz, by trzymać się standardów PSR.

    • Sebastian Malaca

      Rozumiem, że chodzi ci o metodę getDataToInvoice(), czy tak? Punkt dla Ciebie. Jak już napisał Potfur, obiekty, w których się ona pojawia nie powinny jej zawierać. Propozycje rozwiązania i dostosowania kodu do SRP zaproponowałem w odpowiedzi do komentarza Potfura.
      Niemniej jednak cieszę się z takiej czujności (i wiedzy) wszystkich czytających 🙂

      • satyr

        Pomyliłeś PSR z SRP. To dwie zupełnie różne rzeczy.

        https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md

        Bartoszowi może chodzić o to, że:
        – używasz klasy Exception() zamiast Exception() (prawdopodobnie masz jakiegoś dziwnego autoloadera)
        – używasz podkreślników dla zmiennych niepublicznych
        – używasz dwóch spacji do wcięć zamist czterech (lub tabulatorów, które edytor zamienił na dwie spacje)
        – wstawiasz klamry otwierające pod foreach i pod ifem

        • Sebastian Malaca

          Masz rację 🙂 Pozmieniałem kolejność liter.

          Co do standardów, to postaram się na przyszłość trochę bardziej ich trzymać, choć pewnie jeszcze na niejednym faux pas mnie złapiecie.

        • Patryk Kala

          no i brak entera przed returnem.

  • Rafał Łużyński

    Co do zadanego w tekście pytania „jak to można zrobić lepiej?”. Jest parę rzeczy do których można się przyczepić, ale wspomnę tylko o jednym. Przede wszystkim ani obiekt kontrahenta, ani obiekt adresu nie powinien wiedzieć o istnieniu faktur. Łamiesz zasady SRP. Powinieneś mieć osobny obiekt, który zajmuje się wystawianiem faktur i do niego przekazać obiekt kontrahenta.

    • Sebastian Malaca

      Jeszcze raz – prawda 🙂 Cieszę się, że tak Was to razi, a jeszcze bardziej, że każdy argument jest rzeczowy 🙂

  • satyr

    Większość istotnych rzeczy wypunktował Potfur, natomiast ja jeszcze tylko dodam od siebie parę słów.

    W jednym z poprzednich Twoich wpisów dyskutowaliśmy nad sensem używania setterów i getterów do wszystkich pól. Dalej podtrzymuję swoje stanowisko, że powinno się je generować dla wszystkich pól bez dyskusji, chociażby dla tego żeby nie powstawały takie potworki:

    public function getDataToInvoice() – koledzy już wspomnieli że tej metody w ogóle nie powinno tu być

    public function getEmailAddress() – skoro pole nazywa się email to getter powinien się nazywać getEmail. Tu jeszcze jest łatwo bo IDE podpowie końcówkę jak zaczniesz wpisywać getEma…, ale mogą być inne pola gdzie wpadnie komuś do głowy nazwanie gettera w jakiś inny pokręcony sposób (np do pola zipcode -> getPostcode()) i będzie problem. Konkretnie – chodzi o konsekwencję w stosowaniu. Settery i gettery generuje się za pomocą IDE (lub generatorem jeśli framework posiada taką możliwość) a nie pisze ręcznie wg widzimisię 🙂

    public function changeAddress(Address $address) – to samo co powyżej. Przecież to jest setter, więc czemu nazywa się changeAddress? Pogubisz się nie będąc konsekwentnym w nazewnictwie.

    public function changeData($name, $nip) – w klasie Contractor masz teraz dwie metody zmieniające atrybuty. changeData i changeAddress. Uważam że jest to bardzo mylące, bo musisz się domyślać do czego służy changeData, czy za pomoca changeData można zmienić też adres itp. Gdybyś używał setterów nie miałbyś takich dylematów.

    null czy Exception – null. Łapanie co chwilę wyjątków przy tak prostych rzeczach jest wg mnie przesadzone, tym bardziej że zaraz trzebaby tworzyć swoje klasy wyjątków, żeby przypadkiem nie złapać jakiegoś Exception() z innego miejsca.

    Podsumowując, omawiam bardzo podstawową i prostą rzecz. Używanie setterów i getterów. To są na prawde banały, ale zobacz ile one są w stanie stworzyć bałaganu w tak prostym kodzie. Nie gniewaj się, ale niestety kod który zaproponowałeś jest słabiutki i bardzo mu blisko do bycia typowym spagetti.

    Moja sugestia. W przerwie między kolejnym wpisem zainteresuj się frameworkiem Symfony2. On w pewnym stopniu narzuca sposób pisania dobrego kodu. Po kilku miesiącach z SF2 nie będziesz mógł patrzeć na to co pisałeś kiedyś 😉

    • Potfur

      Ja bym unikał akcesorów gdy nie są potrzebne – ale to temat bez dna.

      • Sebastian Malaca

        A najlepsze jest to, że w dzisiejszej dobie coraz więcej ORMów pozwala na ich NIE używanie 🙂

    • Rafał Łużyński

      Sorry, ale połowa tego co napisałeś to wprowadzanie w błąd.

      Settery i gettery prowadzą jedynie do anemicznych encji, bez zachować biznesowych i to wtedy tworzy sie „spaghetti” czy też „raviolli”. Obiekty stają się strukturą danych. Rozróznia je to, że struktury danych mają publiczny dostęp do danych, a obiekty maja publiczny dostęp do zachowań. Także „changeAddress” jest tutaj bardzo trafione, „setAddress” już nie. Tak samo nieważne jak się nazywa pole wewnątrz obiektu, bo użytkownik tego obiektu nie ma o tym wiedzy, jego interesuje tylko metoda dostępowa.

      Co do wyjątków, jeśli nie wyjątki to co? „removeAccount” zwróci false? to jest dopiero niekonsekwencja. Wyjątki maja swoje zastosowanie i powinny być stosowane. W zasadzie większość obsługi błędów powinno być zrobione na wyjątkach w logice biznesowej.

      Rada z zainteresowaniem się Symfony, what? Symfony zapewnia jedynie infrastrukturę, jeśli couplujesz swoją logikę biznesową z frameworkiem, to pewnie Twoje widoki/kontrolery to tak zwane ośmiornice, ktoś coś mówił o spaghetti? Ja radzę się zainteresować architekturą Ports & Adapters, filozofią Domain Driven Design i chociażby książką „Clean Code” wujka Boba.

      I radzę przyjąć więcej pokory przy krytykowaniu kodu innych.

      • satyr

        Co do setterów i getterów to polecam Ci prezentację Krissa Wallsmitha „Treat your model like a princess”. Dokładnie tak się powinny wyglądać encje. Szczupło. Od logiki biznesowej są obiekty zwane serwisami i to one powinny wykonywać operacje na encjach. Dodam tylko, że w ten sposób pisze też Javowy światek używający Springa + Hibernate i nikt nie widzi w tym nic dziwnego.

        Co do ważności nazewnictwa – zachowanie konsekwencji w nazewnictwie zmniejsza ryzyko popełnienia błędów. Oczywistym jest, że lepiej jest mieć pole $email mapowane w bazie na pole „email” z akcesorami setEmail() i getEmail() niż pole $email które mapowane będzie w bazie np. na „mail” i do tego będzie miało gettera getEmailAdress().

        Co do wyjątków nie twierdzę, że nie należy ich używać. Jak najbardziej ich używane jest wskazane, ale w tak jak napisałeś – w warstwie logiki biznesowej za którą powinny być odpowiedzialne serwisy. Osadzanie logiki biznesowej w modelu jest – ehm – smutne.

        Co do Twojego komentarza odnośnie Symfony. Wyciągasz zbyt daleko idące wnioski. Twierdzę, że ten framework wymusza na programiście stosowanie dobrych praktyk, takie jak właśnie thin controllers, thin entities, fat service layer. Oczywiście że i w SF2 można napisać bubel, ale mimo wszystko jest trochę ciężej,

        >to pewnie Twoje widoki/kontrolery to tak zwane ośmiornice

        Kolego, zbyt pochopnie wyciągasz wnioski. Wyciągnąłeś błędny wnosek na temat wiązania mojej logiki biznesowej z konkretnym frameworkiem, po czym wysnułeś domysł o widokach(?)/kontrolerach ośmiornicach (nie widząc na oczy kodu który tworzę) po czym zasugerowałeś mi poleciłeś zapoznać się z rzeczami z którymi się zapoznałem dawno temu.

        >I radzę przyjąć więcej pokory przy krytykowaniu kodu innych.

        Pokorę chętnie przyjmuję jeśli ktoś jest dla mnie autorytetem, ma odpowiedni dorobek i potrafi rozsądnie uzasadnić stawiane przez siebie tezy. W artykule popełniono pewne błędy więc trudno mi być pokornym. Dodatkowo Sebastian wprost pytał „jak Wy byście to zrobili”, więc podzieliłem się swoimi uwagami. Uważam, że zrobiłem to w sposób kulturalny i neutralny. Uważam także z kolei, że Twoja odpowiedź jest bardzo emocjonalna i napastliwa.

        • Rafał Łużyński

          Przede wszystkim napisałeś, że zamieszczony kod jest słabiutki, dlatego zasugerowałem więcej pokory, bo w kontraście z tym co piszesz, ten kod jest „ok”.

          Chude encje i wielkie serwisy (tak zwane ośmiotysięczniki)? Lepiej szybko o tym zapomnij. Kto Ci takie bzdury nawciskał?

          Skoro już poruszyliśmy temat Javy i Hibernate.
          Napisałeś, że z DDD zapoznałeś się dawno temu. To ciekawe, bo to co Evans pisze w swojej książce (twórca DDD), to całkowita odwrotność tego o czym ty piszesz. To samo jest w ksiażce Vernona i ksiązkach Roberta Martina. Jest też świetna prezentacja Sławomira Sobótki (jednego z najlepszych programistów Javy w Polsce) https://www.youtube.com/watch?v=iaLeKHbspLg, który również o tym mówi.

          Zweryfikuj swoja wiedzę, bo poszedłeś totalnie złą drogą. Zakładasz z góry, że wiesz lepiej, a tak nie jest.

          • satyr

            >Chude encje i wielkie serwisy

            Napisałem o „fat SERVICE LAYER” a nie „fat SERVICES”. Nikt nikomu nie każe pisać opasłych klas.

            Również przekażę do zapoznania prezentację Krissa Wallsmitha do której nawiązywałem:
            https://www.youtube.com/watch?v=Pn2lYHvnoUQ

            >całkowita odwrotność tego o czym ty piszesz

            Czy możesz mi wskazać wprost miejsca w książce w których to o czym piszę jest całkowitą odwrotnością? Chętnie to zrewiduję wieczorem.

            Mam też do Ciebie prośbę. Staraj się używać konkretów, bo nie mam za bardzo jak odnieść się tego o czym piszesz:

            >Kto Ci takie bzdury nawciskał?
            >całkowita odwrotność tego o czym ty piszesz
            >Zweryfikuj swoja wiedzę, bo poszedłeś totalnie złą drogą

            Ale czy możesz mi podać konkretnie co wg Ciebie jest złe? Używasz ogólników i nie bardzo rozumiem o co Ci chodzi i gdzie wg Ciebie popełniam błędy w rozumowaniu.

            >Zakładasz z góry, że wiesz lepiej, a tak nie jest.

            Ehh, no i co mam Ci napisać? „I vice versa”? „Twoja prawda jest bardziej Twojsza”? 😉

          • Rafał Łużyński

            Ok, po prezentacji widzę w czym jest problem.

            Tam jest nawet od 10:38 pytanie od autora, który nie rozumie dlaczego „anemic domain model is an anti-pattern” i który nie zgadza się z Martinem Fowlerem nie podając argumentów dlaczego. W zasadzie już po tym wstępie powinieneś tę prezentację wyłączyć.

            Spróbuję wyjaśnić jak najkrócej czemu „anemic model is an anti pattern”, chociaż będzie to trudne, bo ludzie się tego uczą latami, dlatego ciężko mi przytoczyć konkrety, tej wiedzy jest po prostu za dużo. Tak samo nie wskażę Ci cytatów z DDD, bo ta książka ma około 600 stron i w połowie traktuje o wzorcach taktycznych, a konkretnie o programowaniu obiektowym. Jeżeli się zapoznałeś z tą książką to powinieneś to wiedzieć. Sobótka mówi, że DDD Lite to tak naprawdę po prostu bardzo dobrze zrobiony OOP i w pełni się z nim zgadzam.

            Wrzucanie zachowań w serwisy to programowanie strukturalne a nie obiektowe, niektórzy mówią na to „TURBO PASCAL”, bo tak się w nim programowało. Samo pisanie w języku obiektowym nie oznacza, że piszesz obiektowo.

            Załóżmy, że chcemy napisać funkcję, która odpowiada, za spożywanie pokarmu jakiejś osoby. Jeżeli osoba ma za mało miejsca w brzuchu to nie będzie jeść.

            do czego prowadzi wrzucanie logiki do serwisów:

            eat(person_id, food):
            stomach = stomachRepository.get_by_person_id(person_id)
            stomach_content = stomach.getContent()
            content_volume = stomach_content.getVolume()
            if content_volume + food.getVolume() < stomach.getContentMaxVolume():
            stomach.setContent(food)

            Przykład – obiektowo:

            eat(person_id, food):
            person = personRepository.get_by_id(person_id)

            if person.canEat(food):
            person.eat(food)

            W czym jest różnica? Widać gołym okiem.

            Przede wszystkim kod obiektowy jest czytelniejszy. Jasno jest określone zachowanie. Klient używający obiektu "person" musi wiedzieć jedynie o dwóch metodach, a nie znać strukturę innych obiektów w aplikacji.

            Jeżeli teraz chcielibyśmy dodać taki ficzer, że jak osoba zje coś na co jest uczulona to wymiotuje, to wtedy w kodzie obiektowym NIC nie musimy zmieniać, zmieni się wnętrze obiektu person, ale nas to nie obchodzi. W strukturalnym oczywiście dojdą nowe warunki i już zaczyna się robić spaghetti.

            To tylko jeden prosty przykład i wiem, że pewnie znajdziesz sposób jak to zamodelować serwisem i danymi w bardziej przejrzysty sposób, ale nie zmienia to faktu, że każda kolejna modyfikacja kodu, dodanie ficzera jest bolączką w takim kodzie.

          • satyr

            Kolego, ale Ty mówisz o czym innym i piszesz kod tyczący się czegoś innego. Chciałes napisać:

            > funkcję, która odpowiada, za spożywanie pokarmu jakiejś osoby

            a napisałeś funkcję serwisu który karmi daną osobę.

            Oczywistym jest, że w entity umieszcza się pomocnicze funkcje które są reużywalne w postaci „canEat”, „isAccountExpired”, bo przecież nikt nie będzie porównywał a każdym razem dat, czy też pisał tasiemców w ifach, ale funkcje takie jak canSth, isSth to nie jest logika biznesowa.

            Drugiego kawałka kodu zupełnie nie rozumiem. To jest metoda eat() w klasie Person, czy o co chodzi? Czy może to jest nadal serwis karmiący osobę? Rozumiem, że to też serwis skoro pojawia się tam repozytorium.

            Co do dodania ficzeru, to zamiast zmieniać wnętrze obiektu person zmienisz wnętrze obiektu PersonService. I tak zawsze „karmisz” Person za pomocą serwisu.

            Weźmy może coś bardziej realnego i funkcjonalnego. Załóżmy że masz obiekt jakiegoś miejsca, nazwijmy go sobie „Venue” i chcesz go usunąć, co spowoduje usunięcie go z bazy i wyświetlenie odpowiedniego komunikatu na ekranie. Banał. Jak chcesz to zrobić? Moja propozycja:

            1. W zakresie domen mam przygotowane serwisy odpowiedzialne za ten obszar. W tym przypadku jest to VenueManager, który ma wstrzykniętego entityManagera i eventDispatchera
            2. W kontrolerze wywołuję metodę removeVenue serwisu VenueManager
            3. Metoda removeVenue używa entityManagera do usunięcia rekordu z bazy, oraz triggeruje odpowiedni event za pomocą eventDispatchera
            4. W aplikacji mam zarejestrowane dedykowane EventListenery (FlashMessageListener, EmailListener, ActivityGeneratorListener itp) które na dany event mogą zareagować, np wyświetlając flash message, lub wykonać dowolną inną akcję.

            Mógłbyś mi wytłumaczyć jak chciałbyś to zrobić bez serwisu? Planujesz przekazać do metody w modelu entityManagera (lub jakikolwiek inny system zarządzania bazą)? Czy może chcesz całość oprogramować w kontrolerze?

            Podsumowując, kod który podałeś za przykład jest dla mnie niezrozumiały i wg mnie albo nie potrafisz wytłumaczyć o co Ci chodzi, albo nie potrafisz zobrazować tego odpowiednim przykładem. W kodzie tym widzę tam nic z programowania strukturalnego, poza tym że coś co napisałeś w 5 linijkach można było zapisać w jednej, lub wprost przerzucić jako metoda pomocnicza do entity w której i tak będziesz musiał dokonać tego sprawdzenia. Można też jeśli masz serwis o nazwie FeederService (bo tak zamodelowałeś to na przykładzie) który karmi osoby, to i on może mieć metodę canPersonEat(Person $person) : [bool].

            Co do prezentacji, to widziałem ją na żywo więc conajwyżej mogłem wyjść. Jesteś w stanie natomiast odpowiedzieć jakoś na pytanie Krissa?

          • Rafał Łużyński

            Dobra. Nie ma sensu kontynuowanie tej dyskusji. Zaznaczyłem swoją opinię w tym temacie aby inni zauważyli, że Twoja odpowiedź nie jest jedyną, która jest prawidłowa i żeby się nią nie kierowali.

            Zostawiam rzeczy które powinieneś przeczytać, dla własnego dobra:

            http://www.martinfowler.com/bliki/AnemicDomainModel.html
            http://programmers.stackexchange.com/questions/218011/how-accurate-is-business-logic-should-be-in-a-service-not-in-a-model

      • ProgramatorPiEjczPi

        Jak dobrze słyszeć duszę, która przemawia podobnym głosem.

        Ja ogólnie jestem już przejedzony MVC, a najbardziej programistami Symfony 2, którzy aplikacje projektują pod Encje z Doctrine (ludzie, qr… zaczynacie aplikację od projektu bazy danych? Really?), i pchają do kontrolerów cała logikę – a najbardziej wielbię hardkorów, którzy tworzą własne kontrolery z toną logiki.

        I potem się dziwić, że tacy jak tam wyżej klasy z metodami „get” i „set” kojarza jedynie z ORM-ami i bazą danych.

        Zabić to mało… Za jaja na drzewie powiesić i niech tak umierają.

    • Sebastian Malaca

      1) Co do setterów i getterów.

      Widzę, że dla Ciebie każda metoda, która zmienia wartość atrybutu (bądź go wyciąga) powinna zaczynać się od set/get. Dlaczego? Z powodu tego, że biblioteki i ORMy w taki właśnie sposób często funkcjonują?

      Dlaczego changeAddress()? Bo jest to obiekt domenowy i kotrahent zmienia adres na inny (Contrhent.changeAddress(address)), a nie ustawia adres na inny. Niby drobiazg, ale takie nazwanie metody sprawia, że rozmawiając z ekspertami domenowymi uwspólniasz język, a jest to naprawdę nieocenione ułatwienie. I pamiętaj, że mnogość takich drobiazgów jest już czymś istotnym.

      Dlaczego changeData(), a nie setName() i setNip()? Ponieważ użytkownik zmienia swoje dane, a nie ustawia nazwę czy też NIP.

      Zdaję sobie sprawę, że w przykładzie mogą się te rzeczy wydawać nieistotne, ale właśnie na takich podstawach budujesz przyzwyczajenia, co skutkuje tym, że nie musisz później tłumaczyć sobie tego, co powiedział klient na kod. Bo zarówno on może przeczytać kod i go zrozumieć, jak i Ty słuchając jego jesteś w stanie odwzorowywać to bezpośrednio na kod.

      Swoje zdanie co do akcesorów podtrzymuję – powinno się ich unikać, bo jeżeli są (w dodatku do każdego pola, jak proponujesz) to już nie można mówić o poprawnym OOP, bo nie ma mowy o enkapsulacji, jest tylko jej marna namiastka.

      A że czasami jest uzasadnienie zmienić lub dobrać się do jednego atrybutu? Cóż, jak coś ma sens to… ma sens 🙂

      2) Exception/null

      Ogólnie zgadzam się, że z wyjątkami nie można przesadzać i decydować się na ich wykorzystanie świadomie. Zwróć jednak uwagę na to, które operacje wyrzucają wyjątek i w jakiej sytuacji – chcemy pobrać/wykonać operację na czymś czego nie ma, czyli chcemy wykonać niedozwoloną operację, a co za tym idzie -> wyrzucamy wyjątek.
      Moim zdaniem, wszystkie metody z przykładu wyrzucają go w odpowiednim momencie, choć typ wyjątku powinien być konkretniejszy i w rzeczywistym kodzie warto byłoby stworzyć własne klasy wyjątków. Nie wiem jednak dlaczego przedstawiasz taki zabieg jako coś negatywnego?

      Prawda jest taka, że prostym sposobem na wyeliminowanie możliwości wyrzucenia wyjątku jest dodanie metody, która sprawdza czy dany obiekt istnieje i zwraca boolean. Jeżeli true to wykonujemy akcję. I już nas nie boli ten wyjątek, bo w poprawny sposób obsłużyliśmy brak danego obiektu. A jeżeli ktoś kiedyś gdzieś jednak spróbuje wykonać taką operację? Cóż, niech wyjątek powie mu, że robi źle 🙂

      3) Tak słaby, że prawie spaghetti

      Poza metodą getDataToInvoice() (co było celowym zabiegiem), to nie widzę jakichś rażących błędów. Są miejsca na ulepszenia (m.in. to co zaproponował Potfur), ale wydawało mi się, że jakość kodu jest wystarczająca do utrzymania wartości artykułu i równocześnie taka, że umożliwi przedstawienie Waszych pomysłów na jego poprawę 🙂 Mam nadzieję, że się nie myliłem 🙂

      4) SF2
      I bez tego po kilku miesiącach nie będę mógł patrzyć na to, co pisałem kiedyś 🙂 A jeżeli pewnego dnia będzie inaczej, to znak, że pora zmienić zawód 😛

      • asdasdasdasfasd

        Skoro gettery i settery są do każdego pola, to dlaczego nie można zrobić jednego gettera i settera? np function set($field,$value); i function get($field) ??

        • Sebastian Malaca

          Gdyby akcesory były do każdego pola to moim zdaniem pola w ogóle powinny być publiczne, bo po co taka ułuda enkapsulacji?

          Jednak poza DTO raczej ciężko wyobrazić mi sobie konieczność posiadania takich obiektów.

          • dfsdfsdfsdf

            ja bym zrobił jednego settera, oraz tablicę pól, które mogą być edytowane, setter by sprawdzał czy pole znajduje sie w tablicy, jesli tak to zmieni wartosc, jesli nie to taki ch**

          • Sebastian Malaca

            Ale wtedy tracisz widoczność tego, co jest istotne, a co nie. API klasy nie odzwierciedlałoby jej kontraktów. Mimo tego, że funkcjonalnie wszystko działałoby tak samo.

Send this to friend

webmastah.weekly
Cotygodniowa porcja linków ze świata WEBDEV BEZ spamu, TYLKO samo mięcho!
Zobacz poprzednie wydania. Dołącz do 2 tysięcy webdeveloperów!
HTML5, CSS3, JS (React, Angular, Ember, Vue), PHP, SQL
webmastah.weekly
Cotygodniowa porcja linków ze świata WEBDEV BEZ spamu, TYLKO samo mięcho!
Zobacz poprzednie wydania. Dołącz do 2 tysięcy webdeveloperów!
HTML5, CSS3, JS (React, Angular, Ember, Vue), PHP, SQL