6 porad jak ulepszyć obsługę wyjątków

Dele Taylor na swoim blogu wymienia 6 wskazówek dotyczących poprawy obsługi wyjątków w aplikacjach napisanych w Javie.

5 komentarzy to “6 porad jak ulepszyć obsługę wyjątków”

  • dembol says:

    Ciekawy pomysł z kodami błędów oraz propertisami dołączanymi do wyjątku – daje to możliwość poznania pełnego kontekstu w procedurze obsługi. Sceptycznie jednak podchodzę do tworzenia jednej klasy wyjątku per cały system. Po to wyjątki zostały stworzone aby można było w szczególny sposób obsługiwać wyjątkowe sytuacje rozróżniając je właśnie na podstawie klasy wyjątku. Jeżeli jednak nie istnieje szansa na obsługę sytuacji wyjątkowej z sukcesem bądź wyjątek pochodzi ze środowiska lub został spowodowany przez programistę to oczywiście runtime jak najbardziej jest wskazany.

  • Dagguh says:

    Ad. 1
    Kiepski pomysł. Do rozróżniania typów wyjątków należy użyć polimorfizmu. Zgadzam się z preferowaniem unchecked exceptions nad checked exceptions.

    Ad. 2
    Wyjątek powinien zawierać wszystkie parametry wejściowe, tj. argumenty metody i użyte pola klasy. Dla każdej sytuacji wyjątkowej należy mieć klasę wyjątku, która przyjmuje takie parametry w konstruktorze. Składanie wiadomości jest odpowiedzialnością tej klasy. Jeśli potrzebna jest internacjolizacja, można użyć standardowego podejścia I18N. ResourceBundle może być nazwany tak jak klasa wyjątku.

    PS. Okropne ify w podanym kodzie.

    Ad. 3
    Kolejny identyfikator. Dodatkowo trzeba ręcznie pilnować unikalności wartości. W podejściu polimorficznym, unikalność nazw klas jest gwarantowana przez język.

    Ad. 4
    Intencja jest dobra, ale wykonanie jest słabe. Zapis danych wejściowych – ok. Użycie mapy stringów z magicznymi kluczami – nie ok. Podejście polimorficzne nie wymusza znajomości kluczy i nie ogranicza typów parametrów do Stringa.

    Ad. 5
    Autor nie rozumie, że stacktrace jest kompaktowany. Poza tym, rethrow jest potrzebny głównie do tego:

    public void onMessage(Message message) {
        try {
            processMessage(message);
        catch (Exception exception) {
            throw new RuntimeException(exception);
        }
    }
    

    PS. Kod w artykule łamie wiele zasad. M. in. rzutowanie klas, sprawdzanie instanceof, przesyłanie nulli wskazują na design flaw.

    Ad. 6
    Z tym punktem zgadzam się w całej rozciągłości 🙂

    Artykuł porusza ważne punkty, ale proponowane rozwiązania są nieodpowiednie. Autor próbuje napisać maksymalnie generyczny kod, na czym cierpią inne, ważniejsze cechy kodu.

  • dembol says:

    Ad. Ad. 2
    Nie zgadzam się z Twoim rozwiązaniem ponieważ:

    – wiąże klasę wyjątku ze wszystkimi metodami, które dany wyjątek rzucają (chyba że źle zrozumiałem twoją wypowiedź) co spowoduje efekt domina a tym samym ograniczy modyfikowalność kodu, jeden z kluczowych atrybutów jakościowych. Lepszym rozwiązaniem jest przekazywanie do konstruktora wyjątku tylko i wyłącznie tych atrybutów metody, które spowodowały powstanie wyjątku. Chyba, że to właśnie miałeś na myśli.

    – posiadanie osobnej klasy wyjątku dla każdej możliwej sytuacji przeczy wg mnie zasadzie KISS. Dobrym przykładem mógłby być obsługa protokołu HTTP. Wg Twojego rozwiązania każdy kod błędu HTTP powinien posiadać odpowiednią klasę wyjątku? Spowoduje to dużą komplikację kodu, dziesiątki klas. Wyjdzie nowe RFC dodające nowe kody błędów to będzie trzeba modyfikować kod. Dlatego lepszym pomysłem jest jakaś klasa wyjątku opisująca pewien typ sytuacji możliwych do zinterpretowania „wyżej” i uszczegóławianie jej pewnymi właściwościami.

    Ad. Ad. 3
    Jeżeli chodzi o samą klasę reprezentującą kod błędu to jest ona okej, zgodna z zaleceniami dotyczącymi specyficznych wartości związanych z konkretną stałą wyliczeniową. Ręczne nadawanie tych wartości w takich przypadkach jest wskazane (o ile w ogóle jest to konieczne). Jeżeli chodzi o zachowanie unikatowości tych kodów w całym systemie to oczywiście jest to mega zły pomysł, ale w obrębie jednej klasy – czemu nie.

    Ad. Ad. 4
    Mi to przypomina rozwiązanie podobne do Service Registry Springa-dm umożliwiającego export pewnych propertiesów łącznie z exportowaną usługą. Rozwiązanie to jest dobre ponieważ nie wiąże klienta z konkretnymi typami tylko z pewnymi nazwami/właściwościami. Tak samo tutaj – jeżeli wyjątek będzie zawierał pewne właściwości, o określonych, ogólnodostępnych typach, opisujące kontekst wywołania to jest to dobre rozwiązanie ponieważ nie spowoduje uzależnienia kodu obsługującego wyjątek od konkretnych typów leżących na niższych poziomach abstrakcji.

    Ad. Ad. 5
    Zgadzam się z przedmówcą.

    Rozwiązania proponowane przez autora może nie są dopracowane, ale pewne fragmenty jego idei w niektórych sytuacjach mogą się sprawdzić. Co do kodu to oczywiście nie jest on wysokich lotów ale chyba w tym wpisie chodziło Właśnie o ideę a nie kod.

    • Dagguh says:

      @2:
      —- Przekazywanie parametrów do wyjątku przez jego konstruktor —-
      Masz rację, tylko istotne dane wejściowe powinny być przekazywane do wyjątku.

      Gdy kod jest dobry, to ja też mam rację 🙂
      Gdy metody są dobrze podzielone, tj. mają jedną odpowiedzialność (SRP), to parametry każdej z nich są istotne. Innymi słowy, mają maksymalnie jeden powód do rzucenia wyjątku. E.g.

      @Stateless
      class UserFinder {
      
          @PersistenceContext
          private EntityManager entityManager;
      
          public User findUser(int userId) {
              User user = entityManager.find(User.class, userId);
              if (null == user) {
                  throw new UserNotFoundException(userId);
              }
              return user;
          }
      
      }
      
      class UserNotFoundException extends RuntimeException {
      
          public UserNotFoundException(int userId) {
              super("User with id=" + userId + " could not be found");
          }
      
      }
      

      Znanym antywzorcem jest java.net.ConnectException. W stacktrace widać:
      „java.net.ConnectException: Connection refused: connect”.
      Połączenie odrzucone, ale do jakiego hosta i na którym porcie?

      —- Eksplozja klas vs. KISS —-
      Cały koncept opiera się na usuwaniu duplikacji (DRY). Warto zaznaczyć, że KISS też jest spełnione.
      E.g. Po wygrepowaniu kodu znaleziono 70 wywołań:

      new HttpResponseException(404, "Not Found");

      Usunięcie duplikacji polega na zamknięciu powtarzanej konstrukcji w reużywalnym komponencie (w tym wypadku klasa):

      class NotFoundException extends HttpResponseException {
      
          public NotFoundException() {
              super(404, "Not Found");
          }
      
      }
      
      class HttpResponseException { 
      
          public HttpResponseException(int code, String status) {
              super(code + " " + status);
          }
      
      }
      
      class ServerErrorException extends HttpResponseException {
      
          public ServerErrorException() {
              super(500, "Internal Server Error");
          }
      
      }
      

      Gdy wyjdzie nowe RFC, klienci takiego kodu mogą użyć:

      new HttpResponseException(666, "Heaven Unreachable");

      Gdy zacznie pojawiać się dużo takich wywołań, warto stworzyć odpowiednią klasę.

      @3:
      Kody błędów to sprawa analogiczna do punktu 2. Tj. promuje duplikację, którą usuwa się najczęściej poprzez ekstrakcję klasy. Wtedy każda klasa miałaby jeden kod błędu, który staje się zbędny, bo nazwa klasy jest wystarczająco unikatowa 🙂

      @4:
      Muszę przemyśleć kwestię wiązania klienta z typami w konstruktorze wyjątku. Z reguły klient nie będzie tworzył tych wyjątków, tylko je łapał. Problem może wystąpić podczas tworzenia mocków, wtedy typy parametrów konstruktora muszą być dostępne z CLASSPATH.

      —- Idee a kod —-
      Zgadzam się, chodzi o pewne heurystyki. Wskazałem kilka wad wymuszanej generyczności. Niedopracowany kod nie jest problemem. To właśnie idea jednego wyjątku na wszystkie sytuacje jest problemem i będzie produkowała kod, którego nie da się dopracować.

  • dembol says:

    @2 Dagguh, chodzi o to, że bardzo uogólniłeś. Ja się z Tobą zgadzam, że należy umieszczać w wyjątku cały kontekst, który spowodował jego powstanie ale nie zawsze i wszędzie 😉 (dobrym antywzorcem jest przykład z ConnectionException, który przytoczyłeś – nie da się zinterpretować kontekstu). Jeżeli metody mają jeden powód do zmian / do rzucenia wyjątku (innymi słowy spełniona jest zasada SRP) to Twoje rozwiązanie jest okej. Jeżeli zasada nie jest spełniona – nie jest okej 😉

    „Gdy wyjdzie nowe RFC, klienci takiego kodu mogą użyć: new HttpResponseException(666, "Heaven Unreachable");

    Gdy zacznie pojawiać się dużo takich wywołań, warto stworzyć odpowiednią klasę.”

    Wcześniej napisałem „Dlatego lepszym pomysłem jest jakaś klasa wyjątku opisująca pewien typ sytuacji możliwych do zinterpretowania “wyżej” i uszczegóławianie jej pewnymi właściwościami.”. To jest to o czym piszesz odnośnie tworzenia klasy NotFoundException.

    @4 klient nie będzie tworzył tych wyjątków, tylko je interpretował. Jeżeli ma opierać się na kontekście przekazywanym z miejsca powstania wyjątku (które reprezentowane są za pomocą pewnych typów, w tym przypadku są nimi atrybuty metody) to aby dobrze wyjątek zinterpretować musiałby powiązać się składniowo i semantycznie z danym typem. A tego nie chcemy. Dlatego podoba mi się rozwiązanie z propertiesami, które wiąże klienta tylko z pewnym podzbiorem typów ogólnodostępnych.

Leave a Reply