Jeśli wejdziesz głębiej, możesz zobaczyć że to że 2 drugi to też jest stytuacja wyjątkowa, ale nie z punktu widzenia usera; tylko klasy/programisty
@TomRiddle:
No to jest kwestia interpretacji, co oznacza "sytuacja wyjątkowa".
Chyba zbliżamy się do porozumienia. Nie sądzę że to kwestia interpretacji, tylko bardziej spojrzenia na sytuację z różnych poziomów abstrakcji. Z wysokiego poziomu - nie jest, z niskiego - jest.
Nie podlega dyskusji, że "typowy" wyjątek czyli chociażby dzielenie przez zero jest sytuacją wyjątkową.
To prawda. Taki wyjątek ma sens, nikt go nie łapie; nikt się nie dziwi że wystąpił; wszyscy po prostu do niego nie dopuszczają. Takie sytuacje jak z dzieleniem przez zero można mnożyć. Np przekazanie niepoprawnego regexpa do Pattern.compile()
powinno skutkować wyjątkiem, albo próba otwarcia nieistniejącego pliku, lub wyciągnięcie czegoś z pustego arraya/listy.
Ale czy na pewno dostarczenie pliku w innym formacie, który dodatkowo i tak może zostać przetworzony, tylko inną ścieżką, jest sytuacją wyjątkową, czy po prostu innym sposobem obsłużenia danej sytuacji? Skoro liczysz się z tym, że jeśli poda się plik html do konwersji to on i tak ma zostać przerobiony, to taka sytuacja nie jest wyjątkiem (czyli w moim rozumieniu - czymś nagłym, nieprzewidzianym) tylko jedną z zaprojektowanych ścieżek postępowania.
Odpowiem dwojako.
Myślisz o całej tej sytuacji z globalnego punktu widzenia, z punktu widzenia usera. Tak, plik tak czy tak ma być obsłużony; więc userowi żaden błąd i żaden wyjątek się nie pokaże. Dla użytkownika aplikacji to nie jest sytuacja wyjątkowa. Ale klasa PdfToHtmlConverter
spodziewa się PDF'a. Nie powinna dostać htmla, a jeśli go dostanie powinna rzucić wyjątek. Klasy niższe nie powinny polegać na założeniach klas wyżej. Wyobraź sobie pięć aplikacji. Nazwijmy ją Apple, Orange, Pear, Cherry i Banana.
- Aplikacja Apple działa tak jak opisaliśmy, jeśli dostanie pdf'a to konwertuje go na html'a, a jeśli coś innego, po prostu go zwraca.
- Aplikacja Orange działa tak, ze jak dostanie pdf'a to go konwertuje na html'a, a jak dostanie coś innego, np html'a właśnie, to go wsadza w iframe'a i dopiero potem wsadza w html'a
- Aplikacja Pear działa tak, że jak dostanie pdf'a to go konwertuje na html'a, a jak dostanie coś innego, to robi screenshota i tego screenshota wkłada w html'a
- Aplikacja Cherry to mikroserwis który może dostać tylko pdf'a i nie może dostać nigdy html'a, jeśli dostanie to odpowiada statusem 400 bad request.
- Aplikacja Banana crawluje internet w poszukiwaniu pdf'ów, znajduje pliki które faktycznie są pdf'em i zamienia je na HTML przy pomocy
PdfToHtmlConverter
.
Moim zdaniem, klasa PdfToHtmlConverter
w każdej z tych aplikacji powinna wyglądać dokładnie tak samo. Tzn, gdyby faktycznie te aplikacje istniały, to klasę PdfToHtmlConverter
można by wynieść do biblioteki fruit-commons
i jej użyć tak samo w każdej z nich. Pytanie pada, jak jej użyć tak samo, skoro każda aplikacja wymaga innej reakcji na użycie pliku który nie jest pdf'em? Apple go zwraca tak jak wszedł; Orange umiesza go w iframie przed zwróceniem, peer robi screenshota, cherry odpowiada błędem http, a w banana taka sytuacja wgl nie powinna mieć miejsca (i jeśli miała to na 100% mamy buga w szukaniu pdf'ów).
Jedyny sensowny sposób to jest taki, że klasa rzuca wyjątek, który mówi: "Jestem PDF' konwerterem a nie dostałem PDF'a", i dopiero warstwa wyżej która użyła tej klasy wie co wtedy zrobić. Bo to aplikacja ma decydować co zrobić z nie-pdf'em, a nie klasa która ma go skonwertować. Oczywiście mógłbyś zrobić 5 różnych klas, z których każda obsługuje ten sam błąd w różny sposób, ale po pierwsze to by złamało SRP; a po drugie nie czujesz że to w gruncie rzeczy byłby zły design? Nie czujesz że konwertowanie Pdf'a na HTML w każdej z tej aplikacji działa dokładnie tak samo, i powinno być handlowane przez tą samą klasę, i tylko wyjątek od tej sytuacji (czyli przekazany html albo doc) powinien być obsłużony inaczej?
Dodam może, że to że klasa rzuca wyjątek, kiedy dostanie html'a, nie znaczy że kod musi wyglądać tak:
try {
return pdfConverter.convert(fileFromUser);
} catch (NotPdfException exception) {
return fileFromUser; // handling
}
Nie musi.
Może wyglądać też tak:
if (fileFromUser.isPdf()) { // nie dopuść do użycia nie-pdf'a
return pdfConverter.convert(fileFromUser); // rzuć wyjątek jeśli dostaniesz nie pdf'a,
}
return fileFromUser; // handling
Dla mnie dwa kody wyżej są tożsame, w zupełności i mogą być używane zamiennie. Jeśli wolisz wersję z if
em, to spoko. Byleby tylko ten kod nie wyglądał tak:
convert(fileFromUser); // jeśli pdf to convertuj, a jeśli html to zwróć - to jest zło.
Ale pdfConverter.convert()
musi rzucić wyjątek jeśli nie dostanie PDF'a.
W przykładzie aplikacji Cherry to mogłoby być (albo if
, albo try
, wszystko jedno).
if (fileFromUser.isPdf()) { // nie dopuść do użycia nie-pdf'a
return response(200, pdfConverter.convert(fileFromUser)); // rzuć wyjątek jeśli dostaniesz nie pdf'a,
}
return response(400); // http bad request - handling
A w przykładzie Banana, ten kod może wyglądać tak:
pdfConverter.convert(fileFromUser); // rzuć wyjątek, jeśli nie-pdf
..., pod warunkiem że pdfConverter
rzuci wyjątek jeśli dostanie nie-pdf'a, bo to i tak ma się nie zdarzyć nigdy. Banana szuka tylko pdf'ów.
I ta aplikacja Banana to jest to, co próbuję dowieść. W aplikacji banana nie chcę nigdy użyć html'a, nie chcę pisać if
a o pdf'ach i html'ach, bo tam mają być tylko pdf'y, jeśli wsadzę coś innego to aplikacja ma się wywalić, bo na 100% coś zwaliłem. Ale wiem to ja, programista który patrzy z warstwy wyjżej. Klasa PdfConverter
tego nie wie. Zadaniem klasy PdfConverter
jest tylko dać mi znać, że nie dostała pdf'a, a to ja mam zdecydować co później z nim zrobić. Jeśli nic z tym nie zrobię, bo np nie wiem albo nie zauważę, to język daje mi znać bardziej dobitnie, wyłączając mi aplikacje.
gdyby się okazało że te dane nie są od usera, tylko są wynikiem jebnięcia się programisty to możesz przemilczeć buga. Buga o którym byś się dowiedział, gdyby klasa rzuciła wyjątek.
Ale przecież to i tak jest decyzja po stronie programisty, co zrobi w chwili wyjątku.
Tak i nie.
Czasem są wyjątki, które nie powinny się zdarzyć. Posłużę się już skrajnym przypadkiem, ale nie mogę znaleźć innych. Przykładem takiego wyjątku jest NullPointerException
(czy NullReferenceException
w c#). Czy kiedykolwiek ktokolwiek przy zdrowych zmysłach, zrobiłby try {} catch (NullReferenceException ex)
? Oczywiście nie. Dlaczego? Dlatego że w tym przypadku (i w przypadku NPE w żadnym przypadku) nie ma sensu obsługa tego wyjątku; bo do niego w ogóle nie powinno dojść. To jest oczywisty sygnał, że gdzieś indziej w aplikacji mamy buga. Najpewniej w warstwie wyżej która chciała coś zrobić na null
u, mimo że nie powinna. Innym przykładem takiego wyjątku jest DivisionByZeroException
albo StackOverflowException
. Nigdy nie ma sensu wołanie metody na nullu. Ale czy to powstrzymuje userów przed używaniem null
i? Nie, wszędzie zobczysz if (x == null)
albo if (x != null)
. Idąc Twoim tropem, Twoim pomysłem, środowisko nie powinno rzucić NPE
jeśli użyjesz metody na nullu, tylko zrobić to co aplikacja ma robić, czyli np null.toString()
powinien zwrócić ""
. No bo skoro user nie wie nic o null
u, to dla niego to nie jest wyjątek prawda? :D :D Oczywiście żartuje, takie przemilczanie null
i prowadziłoby do jeszcze większej ilości błędów, bo miałbyś czasem gdzieś ""
i nie wiedziałbyś skąd się tam wziął. Dostając wyjątek który Ci rozwala aplikacje NullPointerException
wiesz jakiej natury to był błąd. Dodam może że metoda toString()
, zawołana na obiekcie klasy nie wiem Fruit
, ta metoda toString()
nie wie czy ten owoc jest opcjonalny czy nie. Ty jako programista wiesz czy może czy nie może, ale funkcja nie wie. Czy gdyby wszystkie instancje klasy Fruit
były opcjonalne w Twojej aplikacji, czy miałoby sens żeby Fruit fruit = null; fruit.toString()
zwróciło ""
? Ktoś powiedziałby że tak, skoro wszystkie owoce z punktu widzenia usera są opcjonalne.
Podobnie jak w moim przykładzie z PdfConverter
, klasa
Równie dobrze można zapisać informację do jakiegoś loga błędów, a potem udawać że wszystko jest OK, albo poinformować usera że coś się nie udało, ale nie trzeba mu pchać gołego exceptiona na front. Korzystanie z wyjątku w przypadku błędu po stronie użytkownika nie wyklucza "przyjaznego" potraktowania sprawy.
Czyli przemilczeć buga? No nie wiem czy to taki dobry pomysł. Języki programowania to narzędzia, mają być reliable. Jeśli ja ich źle używam (bo np przekazuje html'a do pdf konwertera) to chcę żeby ta klasa mi o tym powiedziała. Nie chcę, żeby zakładała że nie-pdf' może się tam znaleźć.
Bo niektórych błędów nie powinno się obsłużyć, niektóre powinny pozostać nieobsłużone. I to są sytuacje w których dane pochodzą od programisty/nie od usera.
Czyli co proponujesz? Zamiast jakiejś sytuacji awaryjnej, aplikacja ma się wywalić? Nie rozumiem, o co Ci w tym miejscu chodziło.
No, mniej więcej to samo co opisywałem z NullPointerException
. Jeśli programista zrobi coś głupiego, np podzieli przez 0 albo moim zdaniem również wsadzi html'a do pdf'a, to tak, powinien polecieć wyjątek, i jeśli zostanie nie obsłużony to aplikacja powinna się wywalić. Niemniej, lepiej albo do takiej sytuacji nie dopuścić (nie wkłądać html'a do PdfConverter
), a jeśli został włożony to złapać wyjątek i obsłużyć.
Tylko teraz powiedz mi na pytanie :D Mimo pośrednika, co się powinno stać kiedy PdfToHtmlConverter dostanie *.html?
OK, zgadza się. Ale ja to inaczej widzę.
Ty proponujesz (a przynajmniej tak to zrozumiałem) że przekazujemy do convertera jakiś plik, a jeśli jego format nie pasuje, to w normalnym trybie zwracamy wyjątek i działamy dalej, testujemy kolejne ścieżki.
Do mnie to nie trafia.
No to git :)
Ja bym raczej widział to tak, jak napisałem (i jak mi się wydaje - podobnie uważają m.in. @Miang czy @axelbest) - mamy pośrednika, który weryfikuje typ pliku i na tej podstawie przekazuje jego obsługę dalej. Jeśli stwierdzi, że plik jest w formacie PDF, to przekazuje go do handlera odpowiedniego dla tego typu plików.
I teraz mamy dwie sytuacje:
- plik można obsłużyć, wtedy wszystko idzie OK
- pliku nie można obsłużyć - wtedy wyjątek jest jak najbardziej OK.
Ale widzisz różnicę? Nie steruję przebiegiem programu w "normalnej" sytuacji poprzez wyjątki, tylko mam od tego "zwykłą" logikę. ALE jeśli konwerter dostanie plik niezgodny z tym, czego oczekiwał, to mamy idealny przykład sytuacji wyjątkowej - mimo, że wcześniej było jakieś sprawdzenie, to coś poszło nietak i konwerter dostał dane niezgodne z tym, co powinien otrzymać.
Podsumowując - normalny przebieg pracy robimy w oparciu o "zwykłą" logikę, a wyjątki rzucamy gdy mamy problem.
Zgodnie z moją klauzulą sumienia, jeśli mamy 2 typy plików i dla każdego z nich osobną ścieżkę obsłużenia, to nie ma miejsca na wyjątek, tylko na jakiegoś if
. Wyjątek rzucamy, jeśli mimo tej normalnej ściezki, dostaniemy niepoprawne dane wejściowe.
No dobra, i co powinien zrobić PdfConverter
kiedy dostanie *.docx
albo *.html
? PdfConverter
przetowrzy *.pdf
, rzuci wyjątek dla każdego formatu na świecie (*.docx
, *.bmp
), ale *.html
przepuści normalnie?
Brzmi jakby to "obsłużenie ścieżki" to była cicha pułapka którą PdfConverter
na mnie zastawił, żebym w nią wpadł, i jak będę miał kiedyś buga, to zamiast mi powiedzieć "Masz buga, wsadziłeś html'a do PdfConvertera" to po prostu to przemilczy i nie będę wiedział że mam buga.
moim zdaniem ten error_get_last() to wgl słaby pomysł z wielu powodów
Też tego nie lubię, ale dałem jako przykład, że nawet sytuacje nieprzewidziane/krytyczne da się ogarnąć bez wyjątków. Przy czym - jak pisałem kilka zdań wcześniej, uważam że wyjątek jest OK - ale w sytuacjach nieplanowanych, a nie jako sterowanie standardowym przebiegiem wykonania aplikacji. Za to raczej powinna odpowiadać zwykła logika, a nie rzucanie wyjątku jako forma udzielenia odpowiedzi.
No oczywiście że się da. Jeśli się uprzesz to nawet w brainfucku napiszesz genialną aplikacje. Ale to że się da, nie znaczy że to dobry sposób. Języki ewooluują, i naszczęście teraz mamy elementy języka które to ogarniają.
Justin Bieber i PHP są najlepszymi przykładami na to, że nie musisz być dobry, żeby być popularny.
Ale z perspektywy parsera HTMLa, lub nawet managera plików to jest sytuacja wyjątkowa.
Zgadza się, przy czym o ile Ty mi zarzucasz, że patrzę zbyt globalnie, to mam wrażenie, że Ty przeginasz w drugą stronę :P
Może i tak. Ale dopóki łapiesz te wyjątki ostatecznie nisko, to z mojej strony wszystko jest okej :D
Tak, parser jak dostanie nieprawidłowe dane, to powinien rzucić wyjątkiem. Tutaj się zgadzam.
Fajnie.
Tylko nie podoba mi się Twój sposób stworzenia logiki - w stylu że daję dane do parsera, a jeśli on mi rzuci wyjątek, to wtedy przetwarzam dane w inny sposób.
Możesz zrobić też if (file.isPdf()) { converter.convert(file); } else { /* handel */ }
. Dla mnie if + convert + else, to jest to samo co try { convert } catch {else}
. Z punktu widzenia języka dla mnie one są tożsame ze sobą, przejdą ten sam zestaw unit testów, dla mnie to szczegół implementacyjny. Klasa która używa takiego pośrednika nawet nie będzie wiedziała czy ten wyjątek tam w środku jest czy nie. Dla klasy która umie decydować który typ może co, to możesz wybrać co Ci pasuje, i nie przeszkadzałoby mi gdybyś Ty w tej aplikacji zrobił to if
em. ALE! ALE! Ale co w sytuacji w której *.html
nie może wejść. W której tylko *.pdf
jest dostępny, i nie ma żaden ifologii która decyduje o extensionach. Wtedy taki wyjątek jest jak tarcza przeciwko bugom.
@cerrato Ale, no właśnie jest "ale". Spytasz pewnie, skoro dla mnie to to samo, to czemu wolę wyjątki, prawda? :D
Odpowiadam.
Żeby zrobić if
a, zanim się da plik do PdfConverter.convert(file)
, to trzeba zawołać coś takiego if (file.isPdf())
prawda? No albo ewentualnie if (file.getName().endsWith(".pdf"))
czy coś podobnego. I teraz, co to znaczy. To znaczy, że klasa wyżej, ponad PdfConverter
umie rozpoznać czy coś jest plikiem pdf czy nie. To prawda, możesz to wydelegować do utilsa, if (FileUtils.isPdf(file))
, ale to ciągle oznacza, żę klasa która woła PdfConverter
musi wiedzieć o tym czy plik jest pdf'em czy nie, albo bezpośrednio, albo pośrednio przez ten util. Więc złamała poziomy abstrakcji. Moim zdaniem, tylko PdfConverter.convert()
, albo jego klasy niżej, powinny wiedzieć co jest pdf'em a co nie. Np gdyby doszedł drugi format pliku, np *.pdf
oraz *.pdfx
. Oczywiście PdfConverter
musiałby o tym wiedzieć, ale wszystkie warstwy które go użyły też musiałyby o tym wiedzieć, żeby zrobić odpowiedniego ifa
do ohandlowania tego. A nawet jeśli masz ten util, to wszystkie warstwy muszą pamiętać żeby użyć tego utila. Innymi słowy, PdfConverter
moim zdaniem lepiej wie co jest poprawnym pdf'em, a co nie. I tej klasy powinniśmy słuchać przy decyzji czy coś jest pdf'em a co nie; nie warstwy która go woła if (file.getName().endsWith(".pdf"))
, i nie utila, tylko PdfConverter
a właśnie, a jedyny sposób w jaki może nam o tym powiedzieć to albo exception, albo ten PageResult
który @somekind zaproponował, ale z tym jeszcze nie mam doświadczenia. I nie ma też sensu wystawiać metody publicznej w PdfConverter
, tak: if (pdfConverter.isPdf(file)) { pdfConverter.convert(file));
, bo po pierwsze łamie to hermetyzację; po drugie wymusza kontrakt że cokolwiek co isPdf() == true
nie rzuca wyjątk, a cokolwiek isPdf() == false
rzuca wyjątek, a po trzecie prezentuje interfejs który trochę nie jest SRP.
To jest oczywiście prosty przykład, ale wyobraź sobie system w którym nie jest tak łatwo sprawdzić czy coś pyknie, zanim się nie spróbuje.
Poza tym, słyszałeś może o zasadzie " Tell, don't ask"? Wygooglaj.
Logika powinna być tak zrobiona, że dane trafiają tam, gdzie powinny i w odpowiedniej formie. Jeśli mam kilka ścieżek wykonania, to wybieram odpowiednią i mam być pewny, że jeśli przekazałem do elementu przetwarzającego PDF coś, to to coś jest PDF'em, który się da przetworzyć. A co za tym idzie - jeśli konwerter rzuci wyjątkiem to nie zastanawiam się, gdzie jeszcze mogę przekazać te dane (bo widać nie było to PDF tylko HTML albo DOC), tylko mam informację, że mam gdzieś skopaną logikę i dane są źle przekazywane. W takiej sytuacji nie odpalam kolejnych ścieżek, tylko widzę, że jest problem i szukam rozwiązania, czyli gdzie popełniłem błąd.
Czyli chciałbyś żyć w takim świecie w którym wyjątek == "na 100% ja coś zjeb*** i muszę to szybko naprawić" i w którym wyjątek nie ważne czy na samym dole czy na samej górze, zawsze znaczy to samo.
Nie chcesz żyć w świecie w którym wyjątek == "Jestem klasą/funkcją która dostała parametr i nie wie jak go obsłużyć".
Dobrze rozumiem? :D
Tylko znowu pytanie, z czyjej perspektywy wyjątkowa. Z punktu widzenia aplikacji, raczej nie. Z punktu widzenia jednostki niżej/ warstwy niżej, już tak.
Ponowne - z punktu widzenia convertera to otrzymanie błędnych danych wejściowych jest wyjątkiem. Z tym nie mam problemu. Ale nie podoba mi się logika, w której po otrzymaniu takiego wyjątku Ty dalej szukasz sposobu, w jaki te dane można przemielić.
Dla mnie to jedno i to samo. Jak chcesz, to zrób if
a.
Dla mnie te dwa kody są identyczne, np w pythonie
try:
return int(value);
except NumberFormatException:
return 0;
oraz
if isinstance(value, int):
return int(value)
return 0
Te same unity mogą otestować ten kod, przejdzie dokładnie ten sam test, działa dokładnie tak samo, z zewnątrz klasy te dwa case'y są nierozróżnialne. Dla mnie to szczegół implementacyjny. Różnica jest tylko wtedy kiedy nie robisz ani try
ani if
, tylko robisz:
return int(value); # i tu co by było lepiej, skoro ktoś nie obsłużył ani try'em ani if'em złej wartości?
Skoro konwerter PDF dostał coś, co się nie nadaje to nie szukajmy innego sposobu konwersji - bo na to był czas wcześniej, tylko przyjmijmy wyjątek na klatę i stwierdźmy, że się nam coś powaliło.
Ale to by znaczyło że jeśli mamy klasę A
, która woła PdfConverter
, i ten converter rzucił wyjątek, i według ciebie to znaczy że "przyjmijmy wyjątek na klatę i stwierdźmy, że się nam coś powaliło" to to również oznacza że klasa niżej, decyduje jak działa klasa wyżej. A to mi się już nie podoba zupełnie. To trochę tak, jakbyś pisząc klasę PdfConverter
zkaładał, że w momencie w którym w tej klasie napiszesz throw new NotPdf()
, to znaczy że cokolwiek co zawołało ten converter już nie ma opcji żeby cokolwiek zrobić. Tak jakby nie było możliwości złapania go wyżej, i obsłużenia, tylko że ten wyjątek poleci na samą górę, i pokaże się userowi : "I tried, but not".
Widać łatwo, że korzystając z A, jest szansa że trafimy na scenariusz 1 i musimy pamiętaj jakiej wartości się spodziewamy, zrobić ifa na niej. Jeśli tego ni
Ale z drugiej strony są sposoby (aczkolwiek trochę starożytne) żeby tą wartość -1
oznaczającą bład, opakować jakąś stałą.
A potem można zrobić coś w stylu if (result < OK_VALUE)
i dalej kilkoma if'ami
odczytać (jeśli będzie taka potrzeba) jakiego typu jest ten konkretny błąd.
No bez jaj, nie używajmy magic values. Pls.
Skoro może nie wiedzieć że wyjątek może wystąpić; to może też nie wiedzieć że niepoprawna wartość może być zwrócona. W obu przypadkach jest to błąd wynikły z niewiedzy programistów
Tutaj się w pełni zgadzam - jakiekolwiek obsłużenie błędu/nietypowej sytuacji, wymaga działania ze strony programisty. Jedynie można się zastanawiać, co jest gorsze - czy dalej puszczenie w świat wartości błędu jako czegoś poprawnego, czy zostawienie wyjątku bez obsługi. To drugie jest trudniej przeoczyć, bo albo coś wyskoczy na ekranie, albo aplikacja się wychrzani, przez co nie ma ryzyka, że błędne dane pójdą dalej w świat i narobią zamieszania.
Zawsze możesz łapać szerzej błędy, coś jak catch (Exception)
na samej górze. Masz milion możliwości do obsłużenia tego, jaki wyjątek w której warstwie.