Optional i wyjątki

0

Sprawa wygląda następująco, mam metodę która zwraca Optionala, którego wartość muszę użyć w kolejnej metodzie(makeRequest), która znowu może rzucić wyjątek. Próbowałem na wszelkie możliwe sposoby z map, ifPresent, i innymi, ale żaden sposób nie chcę działać. W końcu zrobiłem coś takiego

 ActiveUserDTO getActiveUser(String sessionID) throws Exception {
        Optional<OAuth2AccessToken> accessToken = authorizationService.getAccessToken(UUID.fromString(sessionID));
        
        if(accessToken.isPresent()) {
            Response response = authorizationService.makeRequest(Verb.GET, Constants.ACTIVE_USER_URL, accessToken.get());
            return new ActiveUserDTO(response.getBody());
        } else {
            throw new Exception("Something went wrong...");
        }
    }

W teorii robi to co chcę, jeżeli optional zwróci pustą wartość, poleci wyjątek który sobie jakoś obsłużę w fasadzie/controllerze, jeżeli uda się znaleźć wartość, strzelam do zewnętrznej usługi i znów, jeżeli uda się, zwracam wynik, jeżeli nie, rzucam wyjątek.

Za pewne jednak, da się to zrobić dużo ładniej niż optional.isPresent() - pytanie tylko jak, ktoś mógłby pomóc?

0

Nie widzę nic tragicznego w tym kodzie. Chyba, że zaraz rzucą się puryści, którym nic nie pasuje.
Jak używasz JDK >= 9, to użyj sobie metody ifPresentOrElse​(...)

0

Problem w tym, że ifPresentOrElse, itp, krzyczy że muszę wewnątrz niego obsłużyć wyjątki. A ja ewentualny wyjątek który poleci w makeRequest, chcę przekazać do klasy wyżej.

0

No to złap wyjątek i rzuć sobie nowy RuntimeException może.

2

Optional ma jeszcze taką fajną metodę jak orElseThrow.

OAuth2AccessToken accessToken = authorizationService.getAccessToken(UUID.fromString(sessionID))
                                                    .orElseThrow(() -> new Exception("Something went wrong..."));
Response response = authorizationService.makeRequest(Verb.GET, Constants.ACTIVE_USER_URL, accessToken);
(...)
0

Jeszcze tylko pytanie na kiego grzyba ten wyjątek i co z nim robisz. Bo może lepiej po prostu zwrócić tego Optionala? Optional<ActiveUserDTO>

0
jarekr000000 napisał(a):

Jeszcze tylko pytanie na kiego grzyba ten wyjątek i co z nim robisz. Bo może lepiej po prostu zwrócić tego Optionala? Optional<ActiveUserDTO>

Wyjątek może polecieć bo:
-komuś wygasł token, czyli ten Optional jest na starcie pusty
-coś poszło nie tak przy wywołaniu RESTa

Zwracając po prostu Optionala na dobrą sprawę nie wiem co się stało, a to istotne z punktu widzenia klienta aplikacji.

Wymyśliłem coś takiego

ActiveUserDTO getActiveUser(String sessionID) throws RuntimeException {
     Optional<OAuth2AccessToken> accessToken = authorizationService.getAccessToken(UUID.fromString(sessionID));
     return accessToken.map(this::callMethod)
             .orElseThrow(() -> new RuntimeException("Token expired"));

 }

 private ActiveUserDTO callMethod(OAuth2AccessToken token) {
     try {
         Response response = authorizationService.makeRequest(Verb.GET, Constants.ACTIVE_USER_URL, token);
         return new ActiveUserDTO(response.getBody());
     } catch (Exception e) {
         throw new RuntimeException("Error while invoking " + Constants.ALL_USERS_DATA_URL + e.getMessage());
     }
 }

Ale to w sumie rozwiązuje problem nieco prościej i czytelniej

OAuth2AccessToken accessToken = authorizationService
                .getAccessToken(UUID.fromString(sessionID))
                .orElseThrow(() -> new RuntimeException("Token expired"));
0

Szybkim rozwiązaniem mogłoby być nie rzucanie wyjątkiem, a użycie Try. W twoim przypadku mogłoby to wyglądać np. tak:

 Try<ActiveUserDTO> getActiveUser(String sessionID) {
       authorizationService.getAccessToken(UUID.fromString(sessionID)).map(token -> {
          Response response = authorizationService.makeRequest(Verb.GET, Constants.ACTIVE_USER_URL, token);
          return new ActiveUserDTO(response.getBody());
       }).toTry(() -> new Exception("Something went wrong"));

Nie mniej nie jest to wielce optymalne rozwiązanie, brnąc w Try'a to kod pi razy drzwi zapewne powinien mieć mniej więcej taką postać:

Try<ActiveUserDTO> getActiveUser(String sessionID) {
    authorizationService
        .getAccessToken(UUID.fromString(sessionID))
        .toTry(() -> new Exception("could not get access token"))
        .flatMap(token -> authorizationService.makeRequest(Verb.GET, Constants.ACTIVE_USER_URL, token))
        .map(response -> new ActiveUserDTO(response.getBody());
}

Nadal modelowanie potencjalnych błędów kuleje ale przynajmniej w jakiś tam sposób zabezpieczasz się przed błędami związanymi z wysyłaniem requestów.

Pytanie czego oczekujesz od tej metody. Jeśli nie interesuje Cię dlaczego coś się nie powiodło, a jedynie ostateczny wynik to wtedy Optional ma sens. Mówi bezpośrednio, że albo coś jest albo tego nie ma, bez żadnych szczegółów. Jeśli interesuje Cię przyczyna niepowodzenia to wtedy zdecydowanie bardziej nadaje się Try albo Either.

Twoja sygnatura metody (ActiveUserDTO getActiveUser(String sessionID) throws Exception ) zaprzecza temu po co zostały stworzone Optional, Try czy Either. Poinformuj potencjalnego użytkownika metody bezpośrednio o tym, że może coś pójść nie tak czy o tym, że może nie być wyniku.

0

Czyli zlym rozwiązaniem byłoby stworzenie dedykowanego wyjątku np. TokenExpiredException i rzucanie go w razie potrzeby z tej metody? Dodam ze to serwis zamknięty wewnątrz pakietu, dostępny tylko poprzez fasade.

Optional tutaj odpada z tego względu ze chce wiedzieć co konkretnie poszło nie tak. Zastanawia mnie jedynie przewaga Try badz Either nad zwykłym wyjątkiem pod ten konkretny przypadek.

4
  1. Zrobienie TokenExpiredException jest ok. W zamyśle twórców javy to powinno być zrobione jako checked exception (żeby klient wiedział, że ma obsłużyć). (ale małym programie możesz sobie zrobić dziedziczenie z RuntimeException jeśli skutki nieobsłużenia są niewielkie - przy runtime można łatwo zapomnieć).
  2. Skoro jest tylko jeden rodzaj problemu to Optional jest ok. Jak dostajesz empty to wiesz, że expired :-)
  3. Gdyby było więcej rodzajów problemów to możesz użyć Either<Problem, ActiveUserDTO>
    Gdzie Problem to np.:
enum Problem {
 TokenExpired,
 BadToken,
 EvenWorseToken
}

(nie musi być enum, to może być dowolna klasa, np. rekord ze szczegółami błedu).
Przewagi Eithera nad CheckedException są takie:

  • referential transparency -> łatwiej testować - można np testować parametrycznie. Testowanie, że poleciał wyjątek wymaga ( dodatkowej gimnastyki, (małej ale jednak). Konsekwencje referential transparency są takie, że wyjątkiem/Either posługujesz sie jak zwykła wartością - możesz łatwiej przekazać do innych metod (np. jako listę). Natomiast klasyczne Exceptiony zawsze mają trochę osobny flow.
  • Rzucanie wyjątku jest troszkę bardziej kosztowne z punktu widzenia JVM (trzeba zapenić odtworzenie Stack Trace) - u Ciebie ten argument nie ma znaczenia, raczej nie będziesz miał 10k expired tokenów na sekundę,
  • klasa Left nie musi dziedziczyć z Exception/Throwable czyli np. łatwo zrobić enum, dużo mniej pitolenia jak masz dużo różnych błedów,
    (ale to pomniejszy problem, bo w wersji Checked Exceptiony opakowujesz dodatkowo tego enuma Exceptionem i masz prawie to samo... ).
  1. Try to taki ograniczony Either, gdzie problem (czyli Left) to Exception
    czyli:
    Try<T> =~= Either<Exception, T>
    (mniej więcnie równa się).
    Try ma raczej sens tam gdzie już są Exceptiony, bo lecą z istniejącego API i chcemy je opakować, bo lubimy referential transparency.

  2. Wszelkie błedy typu Panic - czyli np. co tam zły token, jak sieci nie ma - nadal najlepiej modelować jako RuntimeException, w założeniu, że w takiej sytuacji i tak obsługa błędu nie ma sensu. Można co najwyżej wywalić info na ekran, do logów lub zakończyć program.

  3. (uzupełnienie) Dla szeleńców, którzy kochają sterowanie (flow) przy pomocy Exceptionów, a płaczą nad wydajnością jest ratunek w postaci writableStackTrace (false)
    https://stackoverflow.com/questions/11434431/exception-without-stack-trace-in-java
    Nie polecam, ale jak ktoś bardzo lubi programować dyskunkcyjnie, to ma możliwość od Javy 7.

1 użytkowników online, w tym zalogowanych: 0, gości: 1