Dlaczego metoda nie powinna generować wielu typów sprawdzonych wyjątków?

Używamy SonarQube do analizy naszego kodu Java i ma on następującą regułę (ustawioną na krytyczną):

Metody publiczne powinny zgłaszać maksymalnie jeden sprawdzony wyjątek

Użycie sprawdzonych wyjątków zmusza osoby wywołujące metody do radzenia sobie z błędami, propagując je lub obsługując. To sprawia, że ​​te wyjątki są w pełni częścią interfejsu API metody.

Aby utrzymać złożoność rozmówców na rozsądnym poziomie, metody nie powinny zgłaszać więcej niż jednego rodzaju sprawdzonego wyjątku. ”

Kolejny kawałek Sonaru ma to :

Metody publiczne powinny zgłaszać maksymalnie jeden sprawdzony wyjątek

Użycie sprawdzonych wyjątków zmusza osoby wywołujące metody do radzenia sobie z błędami, propagując je lub obsługując. To sprawia, że ​​te wyjątki są w pełni częścią interfejsu API metody.

Aby utrzymać złożoność rozmówców na rozsądnym poziomie, metody nie powinny generować więcej niż jednego rodzaju sprawdzonego wyjątku.

Poniższy kod:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

należy przekształcić w:

public void delete() throws SomeApplicationLevelException {  // Compliant
    /* ... */
}

Metody zastępujące nie są sprawdzane przez tę regułę i mogą zgłaszać kilka sprawdzonych wyjątków.

Nigdy nie spotkałem się z tą zasadą / zaleceniem w moich czytaniach dotyczących obsługi wyjątków i próbowałem znaleźć pewne standardy, dyskusje itp. Na ten temat. Jedyne, co znalazłem, to CodeRach: ile wyjątków powinna zgłosić najwyżej metoda?

Czy to dobrze przyjęty standard?

Rozważmy sytuację, w której masz podany kod:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

Niebezpieczeństwo polega na tym, że kod, który piszesz, aby zadzwonić, delete()będzie wyglądał następująco:

try {
  foo.delete()
} catch (Exception e) {
  /* ... */
}

To też jest złe. I zostanie złapany z inną regułą, że flagi przechwytują podstawową klasę wyjątków.

Kluczem jest nie pisać kodu, który powoduje, że chcesz pisać zły kod w innym miejscu.

Zasada, którą napotykasz, jest dość powszechna. Checkstyle ma to w swoich regułach projektowych:

ThrowsCount

Ogranicza wyrzuca instrukcje do określonej liczby (domyślnie 1).

Uzasadnienie: wyjątki stanowią część interfejsu metody. Zadeklarowanie metody generowania zbyt wielu różnie zakorzenionych wyjątków powoduje, że obsługa wyjątków jest uciążliwa i prowadzi do złych praktyk programistycznych, takich jak pisanie kodu, np. Catch (Exception ex). Ta kontrola zmusza programistów do umieszczenia wyjątków w hierarchii, tak aby w najprostszym przypadku wywołujący mógł sprawdzić tylko jeden typ wyjątku, ale w razie potrzeby można było przechwycić wszelkie podklasy.

To precyzyjnie opisuje problem i na czym polega problem i dlaczego nie powinieneś tego robić. Jest to dobrze przyjęty standard, który identyfikuje i oflaguje wiele narzędzi do analizy statycznej.

I chociaż możesz to zrobić zgodnie z projektem języka, a czasem mogą to być właściwe działania, jest to coś, co powinieneś zobaczyć i od razu powiedzieć „hmm, dlaczego to robię?” Może to być akceptowalne w przypadku kodu wewnętrznego, w którym każdy jest wystarczająco zdyscyplinowany, aby nigdy catch (Exception e) {}, ale częściej widziałem, jak ludzie skracają rogi, szczególnie w sytuacjach wewnętrznych.

Nie zmuszaj osób korzystających z twojej klasy do pisania złego kodu.

Powinienem zauważyć, że znaczenie tego jest mniejsze w Javie SE 7 i nowszych, ponieważ pojedyncza instrukcja catch może wychwycić wiele wyjątków (przechwytywanie wielu typów wyjątków i ponowne sprawdzanie wyjątków dzięki ulepszonemu sprawdzaniu typów z Oracle).

W Javie 6 i wcześniejszych kod wyglądałby następująco:

public void delete() throws IOException, SQLException {
  /* ... */
}

i

try {
  foo.delete()
} catch (IOException ex) {
     logger.log(ex);
     throw ex;
} catch (SQLException ex) {
     logger.log(ex);
     throw ex;
}

lub

try {
    foo.delete()
} catch (Exception ex) {
    logger.log(ex);
    throw ex;
}

Żadna z tych opcji w Javie 6 nie jest idealna. Pierwsze podejście narusza SUSZENIE . Wiele bloków robi to samo, raz po raz - raz dla każdego wyjątku. Chcesz zarejestrować wyjątek i ponownie go wrzucić? Dobrze. Te same wiersze kodu dla każdego wyjątku.

Druga opcja jest gorsza z kilku powodów. Po pierwsze, oznacza to, że wychwytujesz wszystkie wyjątki. Wskaźnik zerowy zostaje tam złapany (i nie powinien). Ponadto, jesteś Ponowne generowanie e Exceptionco oznacza, że podpis metodą byłoby deleteSomething() throws Exceptionktóry właśnie robi bałagan dalej na stosie jako osób korzystających z kodu są teraz zmuszeni do catch(Exception e).

W Javie 7 nie jest to tak ważne, ponieważ zamiast tego możesz:

catch (IOException|SQLException ex) {
    logger.log(ex);
    throw ex;
}

Ponadto rodzaj sprawdzenia, czy jeden ma złapać rodzaje wyjątków wyrzucane:

public void rethrowException(String exceptionName)
throws IOException, SQLException {
    try {
        foo.delete();
    } catch (Exception e) {
        throw e;
    }
}

Kontroler typów rozpozna, że emogą być tylko typy IOExceptionlub SQLException. Nadal nie jestem zbyt entuzjastycznie nastawiony do korzystania z tego stylu, ale nie powoduje on tak złego kodu, jak był w Javie 6 (gdzie zmusiłoby cię to, aby podpis metody był nadklasą, którą rozszerzają wyjątki).

Pomimo tych wszystkich zmian, wiele narzędzi analizy statycznej (Sonar, PMD, Checkstyle) wciąż wymusza przewodniki po stylu Java 6. To nie jest złe. I mają tendencję do uzgodnienia z ostrzeżeniem do nich nadal być egzekwowane, ale można zmienić priorytet na nich do większych lub mniejszych w zależności od sposobu ich zespół priorytet.

Jeśli wyjątki powinny być zaznaczone lub odznaczone ... że jest to kwestia g r e a t debaty , które mogą łatwo znaleźć niezliczonych blogach biorących się z każdej strony argumentu. Jeśli jednak pracujesz z zaznaczonymi wyjątkami, prawdopodobnie powinieneś unikać rzucania wielu typów, przynajmniej w Javie 6.

Idealnym powodem, dla którego chciałbyś rzucić tylko jeden typ wyjątku, jest to, że w przeciwnym razie prawdopodobnie narusza zasady Inwersji Odpowiedzialności i Zależności . Użyjmy przykładu, aby to zademonstrować.

Powiedzmy, że mamy metodę, która pobiera dane z trwałości i że trwałość jest zestawem plików. Ponieważ mamy do czynienia z plikami, możemy mieć FileNotFoundException:

public String getData(int id) throws FileNotFoundException

Teraz zmieniamy wymagania, a nasze dane pochodzą z bazy danych. Zamiast FileNotFoundException(ponieważ nie mamy do czynienia z plikami), teraz rzucamy SQLException:

public String getData(int id) throws SQLException

Musielibyśmy teraz przejść przez cały kod, który używa naszej metody i zmienić wyjątek, który musimy sprawdzić, w przeciwnym razie kod nie zostanie skompilowany. Jeśli nasza metoda zostanie wywołana daleko, to może być wiele do zmiany / do zmiany przez innych. To zajmuje dużo czasu, a ludzie nie będą szczęśliwi.

Odwrócenie zależności mówi, że tak naprawdę nie powinniśmy zgłaszać żadnego z tych wyjątków, ponieważ ujawniają one wewnętrzne szczegóły implementacji, nad którymi pracujemy. Kod wywołujący musi wiedzieć, jakiego rodzaju trwałości używamy, kiedy naprawdę należy się martwić, czy rekord można odzyskać. Zamiast tego powinniśmy zgłosić wyjątek, który przekazuje błąd na tym samym poziomie abstrakcji, jaki ujawniamy za pośrednictwem naszego interfejsu API:

public String getData(int id) throws InvalidRecordException

Teraz, jeśli zmienimy wewnętrzną implementację, możemy po prostu owinąć ten wyjątek InvalidRecordExceptioni przekazać go dalej (lub nie zawinąć i po prostu rzucić nowy InvalidRecordException). Kod zewnętrzny nie wie ani nie obchodzi, jaki rodzaj trwałości jest używany. Wszystko jest zamknięte.

Jeśli chodzi o Pojedynczą Odpowiedzialność, musimy pomyśleć o kodzie, który generuje wiele niepowiązanych wyjątków. Powiedzmy, że mamy następującą metodę:

public Record parseFile(String filename) throws IOException, ParseException

Co możemy powiedzieć o tej metodzie? Po sygnaturze możemy stwierdzić, że otwiera plik i analizuje go. Kiedy widzimy spójnik, taki jak „i” lub „lub” w opisie metody, wiemy, że robi ona więcej niż jedną rzecz; ma więcej niż jedną odpowiedzialność . Metodami z więcej niż jedną odpowiedzialnością trudno jest zarządzać, ponieważ mogą ulec zmianie w przypadku zmiany któregokolwiek z obowiązków. Zamiast tego powinniśmy rozbić metody, aby ponosili jedną odpowiedzialność:

public String readFile(String filename) throws IOException
public Record parse(String data) throws ParseException

Wyodrębniliśmy odpowiedzialność za odczytanie pliku z odpowiedzialności za przetwarzanie danych. Jednym z efektów ubocznych tego jest to, że możemy teraz przekazać dowolne dane typu String do parsowania danych z dowolnego źródła: w pamięci, pliku, sieci itp. Możemy również parseteraz łatwiej testować, ponieważ nie potrzebujemy pliku na dysku przeprowadzić testy przeciwko.

Czasami naprawdę są dwa (lub więcej) wyjątki, które możemy rzucić z metody, ale jeśli trzymamy się SRP i DIP, czasy, w których spotykamy tę sytuację, stają się rzadsze.

Pamiętam, jak bawiłem się tym trochę podczas grania w Javie, ale nie byłem świadomy różnicy między zaznaczoną a niezaznaczoną, dopóki nie przeczytałem twojego pytania. Znalazłem ten artykuł w Google dość szybko i przechodzi on do niektórych pozornych kontrowersji:

http://tutorials.jenkov.com/java-exception-handling/checked-or-unchecked-exceptions.html

Biorąc to pod uwagę, jednym z problemów, o których wspominał ten facet ze sprawdzonymi wyjątkami, jest to (że osobiście natknąłem się na to od samego początku w Javie), jeśli nadal dodajesz kilka sprawdzonych wyjątków do throwsklauzul w deklaracjach metod, nie tylko czy musisz wprowadzić o wiele więcej kodu pomocniczego, aby go obsługiwać, gdy przechodzisz do metod wyższego poziomu, ale robi to również większy bałagan i psuje kompatybilność, gdy próbujesz wprowadzić więcej typów wyjątków do metod niższego poziomu. Jeśli dodasz sprawdzony typ wyjątku do metody niższego poziomu, musisz ponownie uruchomić kod i dostosować kilka innych deklaracji metod.

Jednym z punktów łagodzenia wspomnianym w artykule - a autorowi nie podobało się to osobiście - było stworzenie wyjątku klasy bazowej, ograniczenie throwsklauzul do korzystania tylko z niego, a następnie podniesienie jego podklas wewnętrznie. W ten sposób możesz tworzyć nowe sprawdzone typy wyjątków bez konieczności przeglądania całego kodu.

Autorowi tego artykułu może nie podobało się to tak bardzo, ale ma to sens w moim osobistym doświadczeniu (szczególnie jeśli możesz sprawdzić, jakie są wszystkie podklasy), i założę się, że dlatego otrzymujesz porady ogranicz wszystko do jednego sprawdzonego typu wyjątku. Co więcej, wspomniana rada faktycznie dopuszcza wiele sprawdzonych typów wyjątków w metodach niepublicznych, co ma sens, jeśli jest to ich motywem (lub nawet innym). Jeśli to tylko prywatna metoda lub coś podobnego, nie zmienisz połowy bazy kodu, gdy zmienisz jedną małą rzecz.

W dużej mierze pytałeś, czy jest to akceptowany standard, ale między badaniami, o których wspomniałeś, tym rozsądnie przemyślanym artykułem, a mówiąc po prostu z osobistego doświadczenia w programowaniu, z pewnością nie wyróżnia się w żaden sposób.

Zgłaszanie wielu sprawdzonych wyjątków ma sens, gdy istnieje wiele rozsądnych rzeczy do zrobienia.

Powiedzmy na przykład, że masz metodę

public void doSomething(Credentials cred, Work work) 
    throws CredentialsRequiredException, TryAgainLaterException{...}

narusza to zasadę wyjątku pne, ale ma sens.

Niestety zwykle zdarzają się takie metody

void doSomething() 
    throws IOException, JAXBException,SQLException,MyException {...}

W tym przypadku osoba dzwoniąca ma niewielkie szanse na zrobienie czegoś konkretnego na podstawie typu wyjątku. Więc jeśli chcemy zmusić go do uświadomienia sobie, że te metody MOGĄ, a czasem MOGĄ się nie udać, wyrzucenie tylko SomethingMightGoWrongException jest lepsze i lepsze.

Stąd reguła co najwyżej jeden sprawdzony wyjątek.

Ale jeśli projekt używa projektu, w którym istnieje wiele znaczących sprawdzonych wyjątków, ta reguła nie powinna mieć zastosowania.

Sidenote: Coś może się nie udać prawie wszędzie, więc można pomyśleć o użyciu? rozszerza wyjątek RuntimeException, ale istnieje różnica między „wszyscy popełniamy błędy” i „to mówi system zewnętrzny i czasami BĘDZIE nie działać, poradzić sobie z tym”.