Witaj Gościu! ( Zaloguj | Rejestruj )

Forum PHP.pl

 
Reply to this topicStart new topic
> Używanie Exceptionów a typowanie
athabus
post 31.12.2018, 14:04:37
Post #1





Grupa: Zarejestrowani
Postów: 898
Pomógł: 48
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Ostatnio ciekawi mnie temat używania Exceptionów - muszę przyznać, że trochę słabo to czuję. Kiedyś Exception stosowałem do rzeczy, które po prostu mogły spowodować niedziałanie aplikacji - np. brak dostępu do pliku czy coś podobnego. Ostatnio jednak widzę, że coraz częściej w nowych bibliotekach Exceptiony stosuje się w inny sposób tj. do wymuszenia typowania metody bez zwracania NULL.

Np taka klasa:

  1. <?php
  2.  
  3. class MySampleRepository
  4. {
  5. public function getItemV1(int $id): SomeObject
  6. {
  7. //pobieramy item, który może nie istnieć (null)
  8. $item = $this->fetch($id);
  9.  
  10. if (!$item) {
  11. throw new ItemNotFoundException('Item does not exist');
  12. }
  13.  
  14. return $item;
  15. }
  16.  
  17. public function getItemV2(int $id): ?SomeObject
  18. {
  19. //pobieramy item, który może nie istnieć (null)
  20. $item = $this->fetch($id);
  21.  
  22. return $item;
  23. }
  24. }


Pierwsza metoda (podejście o którym mówię) nigdy nie zwraca nulla, tylko obiekt lub rzuca wyjątek. Druga metoda (tradycyjna) zwraca obiekt lub null.

Które podejście stosujecie? Dlaczego.

Ja po rozmowie z kolegą zacząłem skłaniać się ku pierwszej wersji i brak możliwości zwrócenia oczekiwanego obiektu traktuję jako wyjątek.Podświadomie wydaje mi się to bardziej logiczne i wymusza trochę więcej dyscypliny.
Go to the top of the page
+Quote Post
Pyton_000
post 31.12.2018, 14:18:34
Post #2





Grupa: Zarejestrowani
Postów: 8 068
Pomógł: 1414
Dołączył: 26.10.2005

Ostrzeżenie: (0%)
-----


1-sza bo:
- rzucasz w 1 miejscu wyjątek
- zawsze dostajesz w return to czego oczekujesz bez zbędnego sprawdzania czy aby nie jest to null czy coś innego
- Exception możesz sobie złapać w 1 miejscu jeśli masz taką fantsazję

Oczywiście 2-gi przykład też jest ok dla pewnych przypadków (tam gdzie oczekujemy że faktycznie może być null).

Dodatkowo jak sobie organizujesz aplikację w DDD to widzisz jakie wyjątki może zwrócić twoja Domena.

Oczywiście nic na siłę smile.gif
Go to the top of the page
+Quote Post
viking
post 31.12.2018, 14:42:44
Post #3





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


W pierwszym dodatkowo może polecieć type error jeśli parametr nie będzie int więc łatwiej to ogólnie obsłużyć.


--------------------
Go to the top of the page
+Quote Post
markonix
post 31.12.2018, 15:10:21
Post #4





Grupa: Zarejestrowani
Postów: 2 707
Pomógł: 290
Dołączył: 16.12.2008
Skąd: Śląsk

Ostrzeżenie: (0%)
-----


Ja bym jednak wolał aby repozytoria zwracały puste tablice/kolekcje oraz null w przypadku gdy pobieram w zamierzeniu jeden obiekt.

  1. if (MySampleRepository::getItemV1($id) !== null) {
  2. //
  3. }


  1. if ($object = MySampleRepository::getItemV1($id)) {
  2. //
  3. }


  1. $object = MySampleRepository::getItemV1($id) ? ? Alternative::get($id);


Generalnie wszystko np. w Laravel typu get(), find(), first(), firstWhere(), config() zwraca null gdy nie znajduje nic i ciężko by mi się było przerzucić na łapanie wyjątków. Najnormalniej w świecie było by więcej kodu i byłby dla mnie mniej czytelny.

edit: Oprócz findFail(), który właśnie zwraca zgodnie z intencją wyjątek ResourceNotFound, idealny do wyświetlania resource/{id} i wyświetlenia po prostu 404ki bez zbędnych warunków.

Temat w każdym razie mnie zaciekawił bo akurat ostatnio często rozmyślam na repository pattern. Znalazłem na Stacku temat: https://stackoverflow.com/questions/175532/...en-it-cant-prod

Ten post edytował markonix 31.12.2018, 17:29:56


--------------------
Go to the top of the page
+Quote Post
athabus
post 31.12.2018, 15:48:00
Post #5





Grupa: Zarejestrowani
Postów: 898
Pomógł: 48
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Co do kwestii przyzwyczajeń to trochę rozumiem bo też na początku mi się było trudno przestawić, ale w sumie kodu wychodzi podobnie i z perspektywy jednak kod oparty o wyjątki jest bardziej czytelny. Jeśli funkcja może zwrócić 2 rezultaty (null albo object) to zawsze jakoś trzeba to obsłużyć - albo if albo try catch. Try/catch (po przezywyczajeniu) wydaje mi się bardziej czytelny, jeśli oczywiście stosujemy szczegółowe wyjątki a nie \Exception. Oczywiście kwestia gustu - z tym nie dystkutuję.

Co do zwracania pustej kolekcji to to jest moim zdaniem inny przypadek niż brak obiektu, bo pusta kolekcja/tablica może być traktowana polimorficznie tak samo jak tablica/kolekcja z elementami. Tj. można po niej iterować, przekazać jako parametr dalej, zwracany typ też się zgadza itp., a z obiektem, który nie jest nagle obiektem tylko nullem już tego nie zrobisz.

Moim zdaniem zwracanie function(): ?object trochę przeczy typowaniu - bo po to typuję zwracany typ, żeby nie musieć sprawdzać czy otrzymuję ten typ.

Ale oczywiście to tylko moje zdanie i widzę też zalety "starego" podejścia.
Go to the top of the page
+Quote Post
Pilsener
post 31.12.2018, 17:11:46
Post #6





Grupa: Zarejestrowani
Postów: 1 590
Pomógł: 185
Dołączył: 19.04.2006
Skąd: Gdańsk

Ostrzeżenie: (0%)
-----


Jeśli przy próbie pobrania czegoś leci wyjątek, to problemem jest brak tego czegoś i to ten problem należy rozwiązać, czyli najpierw ->hasItem($id) a potem ->getItem($id) zamiast try+catch

Cytat
Try/catch (po przezywyczajeniu) wydaje mi się bardziej czytelny, jeśli oczywiście stosujemy szczegółowe wyjątki a nie \Exception


Kiedyś widziałem taki kod:
  1. if($isLogged){
  2. throw new isLoggedException();
  3. }else{
  4. throw new isNotLoggedException();
  5. }


A potem try + catch jak trzeba wyświetlić formularz logowania, od wuja "instance of" facepalmxd.gif
Używanie wyjątków do implementacji logiki nie jest w niczym lepsze od używania goto. Potem masz pincet wyjątków, jeden jest tłumaczony na inny, łapane są w każdych możliwych miejscach, tworzy się pajęczyna zależności i zamiast aplikacji jest burdel.

Cytat
Ja bym jednak wolał aby repozytoria zwracały puste tablice/kolekcje oraz null
- jeśli obsługa czegoś nie jest jednoznaczna (np. Athabus chce okodować sytuację, kiedy host nie odpowiada, natomiast Markonix woli mieć w takiej sytuacji exception i stronę błędu) to wiele bibliotek daje możliwość konfiguracji:
http://docs.guzzlephp.org/en/stable/reques...tml#http-errors

I powiem znowu: wg mnie wyjątki są po to, aby czegoś nie obsługiwać a nie odwrotnie, jeśli leci wyjątek a mimo to go łapiemy bo chcemy, żeby aplikacja działała dalej to oznacza to, że jest to workaround bo albo czegoś nie przewidzieliśmy (np. tego, że brak połączenia z jakimś API nie musi wywalać aplikacji) albo nie zaimplementowaliśmy (np. w podanym przykładzie próbujemy coś pobrać bez wcześniejszego sprawdzenia, czy to coś istnieje).

Cytat
Które podejście stosujecie? Dlaczego.

Zdroworozsądkowe, generalnie nie przesadzać z rzucaniem wyjątku, jak najczęściej dawać wybór, nawet w sposób dynamiczny, np. jeśli mamy obiekt ACL i metodę ->hasRole, to możemy przekazać parametr czy chcemy boolean'a czy exception. Jak sprawdzamy w kontrolerze to lepszy jest exception, jak w templacie to boolean Lkingsmiley.png
Znajdziemy też pewnie sporo takich przykładów w samym PHP:
http://php.net/manual/pl/pdo.error-handling.php
Go to the top of the page
+Quote Post
athabus
post 1.01.2019, 13:29:58
Post #7





Grupa: Zarejestrowani
Postów: 898
Pomógł: 48
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Haha nie rozawżajmy przypadków skrajnych jak ten z początku posta ;-)

Przykład z repozytorium jest dosyć ciekawym casem. Czy jeśli odpytuję bazę czegoś (np. produktów) po ID/SKU to nie zakładam, że to coś istnieje? Przecież takiego SKU nie wygenerowałem losowo. Poprawnie logicznie w takiej sytuacji faktycznie byłoby najpierw sprawdzić czy coś istnieje a potem dopiero to pobrać, ale z wiadomych względów nie zawsze jest to dobre rozwiązanie (np. jeśli to obiekt w bazie to wykonujemy 2 zapytania zamiast 1)

Weźmy też na tapet inny przykład - PSR-6, czyli interfejs dla cache. Tam właśnie chyba powstał problem co ma zwracać metoda $cache->getItem($key). Co ciekawe ta metoda nie posługuje się ani wyjątkiem, ani null, tylko null object pattern. Zawsze zwracany jest taki sam obiekt CacheItem, który w interfejsie ma metodą isHit() umożliwiającą sprawdzenie, czy jest pusty czy nie. Pośrednio jest to coś o czym piszesz, ale to jednak zupełnie co innego niż zwrócenie null.

Dla mnie osobiście null object pattern nie przemawia - rozumiem, że jego zaletą jest możliwość polimofricznego traktowania każdego rezultatu, ale w sumie tak czy siak jest to taki jakby slow failing, bo przed klientem ukrywa nulla.

Taki jeszcze jeden aspekt przemawiający za wyjątkami to właśnie failing fast. Zwracając null (czy null object), możemy doprowadzić w większym kodzie do dość niespodziewanych zachowań. Przykładowo załóżmy, że z jakichś powodów metoda pobiera 3 obiekty po id (np. $repository->getBestsellerInCategory($categoryId)) i zapisuje je do jakiejś tablicy. Ta tablica potem wędruje sobie po systemie i 30 funkcji dalej dostajemy errora "trying using null as a object". Takie sytuacje bywają dosyć trudne do debugowania. Ja na przykład pracuje na Magento, gdzie przy mocno customizowanych sklepach pod niektóre metod wpina się wiele pluginów/obserwatorów i potem znalezienie takiego błędu trochę trwa.

Użycia Exceptiona zmusza klienta kodu do jawnego obsłużenia takiej sytuacji. Przy dużej bazie kodowej trzeba więcej na kliencie wymuszać.

Tutaj ciekawy artykuł w temacie: https://www.yegor256.com/2014/05/13/why-null-is-bad.html

PS. oczywiście zgadam się z tym, że nie ma jednej uniwersalnej zasady. Mamy wiele możliwości do wyboru (null, exception, wartość domyślna, null object itp). Z drugiej strony staram się ostatnio coraz częściej myśleć gdy używam pewnych rzeczy - np. staram się unikać wartości domyślnych w funkcjach czy na przykład dziedziczenia gdy tylko mogę. Ostatnio do listy dodałem function(): ?someObject.

Ten post edytował athabus 1.01.2019, 13:31:34
Go to the top of the page
+Quote Post
Pilsener
post 1.01.2019, 17:04:28
Post #8





Grupa: Zarejestrowani
Postów: 1 590
Pomógł: 185
Dołączył: 19.04.2006
Skąd: Gdańsk

Ostrzeżenie: (0%)
-----


Cytat(athabus @ 1.01.2019, 13:29:58 ) *
Użycia Exceptiona zmusza klienta kodu do jawnego obsłużenia takiej sytuacji.


Nie zgodzę się, jak masz return ?User to od razu widzisz, że User lub null, natomiast zazwyczaj nikt nie będzie zakładał, że dostanie UserNotFoundException zamiast null czy false, więc prędzej programista zrobi:

  1. $user = $repo->getUser();
  2. if($user){
  3.  
  4. }else{
  5. //tu obsługa braku usera
  6. }


niż:
  1. try
  2. {
  3. $user = $repo->getUser();
  4. }
  5. catch( Exception $exception )
  6. {
  7. if ($exception instanceof UserNotFoundException) {
  8. //tu obsługa braku usera
  9. } else {
  10. throw $exception;
  11. }
  12. }


Używanie drugiego kodu zamiast pierwszego niczego nie rozwiązuje a stwarza tylko dodatkowe problemy (można np. przez przypadek złapać wszystkie wyjątki i nie puścić ich dalej, trzeba wiedzieć, jakie wyjątki/wyjątek złapać, piszemy n linijek nadmiarowego kodu, tracimy zalety hermetyzacji i inne takie), nie rozumiem kompletnie na czym przewaga drugiego rozwiązania ma polegać. I tak za każdym razem mam robić, kiedy chcę pobrać Usera?

Cytat(athabus @ 1.01.2019, 13:29:58 ) *


To jest pogląd jednego developera, który w dodatku insynuuje, że null jest sprzeczny z OOP. Null object pattern jest akceptowany i używany od lat w wielu językach, opisany w wielu książkach np. "Agile Software Development, Principles, Patterns, and Practices".
Poza tym takich artykułów są tysiące, różnego rodzaju pryncypiów setki i często są one ze sobą sprzeczne. Dziś KISS, jutro DRY, pojutrze CQRS - 99% problemów z kodem leży zupełnie gdzie indziej, niestety to tak nie działa, że jak się pozbędziemy "Null object pattern" to już będzie super smile.gif Co chwila słychać o nowych, wspaniałych paradygmatach (np. że frameworki to zło), które stają się koszmarem dla programistów mających niefart je realizować arrowheadsmiley.png
Go to the top of the page
+Quote Post
athabus
post 1.01.2019, 19:55:29
Post #9





Grupa: Zarejestrowani
Postów: 898
Pomógł: 48
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Hej ale mamy XXI wiek i od takich rzeczy jest IDE wink.gif Przcież w Javie też nikt nie klepie wyjątków z ręki a tam to chleb powszedni i obligatoryjnie musisz je obsłużyć.

W PHP storm metoda z nieobsłużonym wyjątkiem jest domyślnie podświetlona (+ edytor w docku domaga się dodania @throws jeśli wyjątek jest nieobsłużony).

Pisać też nic nie musisz - po prostu stajesz na takiej metodzie alt+enter i wybierasz surround with try/catch block i dostajesz kod z odpowiednim wyjątkiem/wyjątkami:

  1.  
  2. try {
  3. $cacheItem->getValue();
  4. } catch (EmptyCacheItemException $e) {
  5. }
  6.  


Taki zapis jak zaproponowałeś faktyczni byłby bez sensu bo po a - łapiesz WSZYSTKIE wyjątki co jest zaprzeczeniem idei tego mechanizmu, a po drugie obsługujesz je przez dziwaczną strukturę z if, która jest nieczytelna.

Takie porównanie kodu, aby było sprawiedliwie:

  1. //wersja 1
  2. try {
  3. return $cacheItem->getValue();
  4. } catch (Cache\EmptyCacheItemException $e) {
  5. $item = $this->prepareItem();
  6. $this->cache->save($inlineImage);
  7. return $item;
  8. }
  9.  
  10.  
  11. //wersja 2
  12. if ($cacheItem->isHit()){
  13. return $cacheItem->getValue();
  14. }
  15.  
  16. $item = $this->prepareItem();
  17. $this->cache->save($inlineImage);
  18. return $item;


Obie wersje są wg mnie czytelne i klarowne. Wersja 1 zmusza do świadomego obsłużenia wyjątku (IDE będzie o to krzyczeć) - w wersji 2 moim zdaniem łatwiej przeoczyć konieczność kontroli bo IDE już nie podpowie, żebyś sprawdził czy coś jest nullem. Przy fast fail musisz coś zrobić od razu z niewartościowym obiektem, a nulla możesz przekazać dalej w miejsce, gdzie ktoś już nie będzie zakładał, że może on być nullem a nie obiektem, którego oczekiwał.

Trochę wychodzi jakbym było orędownikiem wersji z exceptionami, a w sumie sam się zastanawiam, które podejście jest lepsze.

Ten post edytował athabus 1.01.2019, 19:57:16
Go to the top of the page
+Quote Post
kapslokk
post 3.01.2019, 09:26:06
Post #10





Grupa: Zarejestrowani
Postów: 965
Pomógł: 285
Dołączył: 19.06.2015
Skąd: Warszawa

Ostrzeżenie: (0%)
-----


Ja pozwolę sobie zostawić tutaj link: https://www.nikolaposa.in.rs/np/slides/deal...itions/phpce17/

Ten post edytował kapslokk 3.01.2019, 09:26:20
Go to the top of the page
+Quote Post
Crozin
post 3.01.2019, 15:37:02
Post #11





Grupa: Zarejestrowani
Postów: 6 476
Pomógł: 1306
Dołączył: 6.08.2006
Skąd: Kraków

Ostrzeżenie: (0%)
-----


1. Nie ma jednoznacznej odpowiedzi na pytanie null VS wyjątek VS null object VS optional VS jeszcze coś innego. Jest to niemal zawsze zależne od konkretnej sytuacji i od przewidzianego zachowania. Podaj konkretny przypadek użycia - wtedy można zastanawiać się co wybrać.
2. @Pilsener akurat Twoja sugestia odnośnie czegoś takiego:
  1. if ($sth->hasItem($key)) {
  2. $value = $sth->getItem($key);
  3. }
W przypadku zasobów nie będących w bezpośredniej władzy programu (tj. wszystko wykorzystujące sieć, dyski czy ogólnie środowisko systemu) jest po prostu zła i nieprawidłowa - prowadzi do błędów, bo pomiędzy sprawdzeniem (has()), a wykorzystaniem (get()) stan systemu może się zmienić. Tutaj tylko wyjątki dają sensowny interfejs, bo trzeba spróbować pobrać zasób (get()) by w ogóle stwierdzić czy jest on dostępny (has()).

Ten post edytował Crozin 3.01.2019, 15:37:36
Go to the top of the page
+Quote Post
athabus
post 3.01.2019, 16:14:36
Post #12





Grupa: Zarejestrowani
Postów: 898
Pomógł: 48
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


@kapslokk - fajne slidy - sporo mi dały do myślenia jeśli chodzi o strukturę samych Exeptionów i zarządzanie nimi. Muszę to w weekend dokładniej rozpracować.

@Crozin
ad 1) Nie mam na myśli konkretnego przypadku, raczej chodzi mi o dyskusję.
ad 2) Zgadza się - ogólnie ogarnięcie race condition to czasami kłopotliwa sprawa.
Go to the top of the page
+Quote Post
Pyton_000
post 3.01.2019, 17:19:27
Post #13





Grupa: Zarejestrowani
Postów: 8 068
Pomógł: 1414
Dołączył: 26.10.2005

Ostrzeżenie: (0%)
-----


Co do race condition to przypomniała mi się moja dyskusja z kolegą z teamu.
Realizował on upload pliku CSV. Po wgraniu pliku sprawdzał mime i filesize.

Była sobie metoda z typehint
Kod
function validateFilesize(int $filesize);


a kod który ją wywoływał:

[code]if(!is_uploaded_file($file)) die;

$validator->validateFilesize(filesize($file));[code]

Problem polegał na tym że `filesize()` może zwrócić również false i warning. Nie docierało do niego że powinien sprawdzić czy filesize !== false.
On na to że nie musi bo skoro plik się poprawnie wysłał na serwer to i na pewno zwróci rozmiar. Nie docierał argument że pomiędzy sprawdzeniem czy plik się wysłał a sprawdzeniem rozmiaru może stać się cokolwiek. Skoro ludzie od PHP wstawili false to po coś to jest? A może to ja się czepiałem ?

(chodziło też o to że PHPStan z lvl = max się czepiał tego zapisu)
Go to the top of the page
+Quote Post

Reply to this topicStart new topic
1 Użytkowników czyta ten temat (1 Gości i 0 Anonimowych użytkowników)
0 Zarejestrowanych:

 



RSS Wersja Lo-Fi Aktualny czas: 28.03.2024 - 14:05