Jak poprawnie obsłużyć wyjątek?

private void invoke() {
      map.put("users", getUsers());
}

private List<String> getUsers() {
    try {
        List users = fetchUsers();
  catch (Exception e) {
      map.put("error", "dupa");
   }
}

Czy w tej sytuacji lepiej jest dodać throws do getUsers() i przechwycić wyjątek w invoke() czy lepiej zostawić tak jak jest? Czy może lepiej zwrócić pustą liste/nulla w getUsers() następnie dodać checka w invoke i w zależności od wyniku dodać do kolekcji? Czy może jeszcze inaczej i przenieść map.put("users", getUsers()); do getUsers() tylko jak wtedy ładnie nazwać metodę? Który sposób będzie najlepszy i dlaczego?

catch (Exception x) raczej nie jest dobrą praktyką.
Jaki konkretnie wyjątek, jakie mogą polecieć?
Metoda getUsers nie skompiluje sie. Brak returns.
Wrzucanie czegoś pod kołderką do jakiegoś map to też średni pomysł. To SpringMVC?

Potrzeba więcej kontekstu, żeby dać jakąś radę, ale na razie ten kod to dramat.

jarekr000000 napisał(a):

catch (Exception x) raczej nie jest dobrą praktyką.
Jaki konkretnie wyjątek, jakie mogą polecieć?
Metoda getUsers nie skompiluje sie. Brak returns.
Wrzucanie czegoś pod kołderką do jakiegoś map to też średni pomysł. To SpringMVC?
Potrzeba więcej kontekstu, żeby dać jakąś radę, ale na razie ten kod to dramat.

Struktura jest znaczeni bardziej rozbudowana, ale dla uproszczenia sytuacji, poprawiona wersja getUsers():

private List<String> getUsers() {
List users = new ArrayList<>();
    try {
        fetchUsersWithRole("admin", users);
        return users;
  catch (JakisDBException e) {
      map.put("error", "blad");
      return users;
   }

Nie doszukujcie się logiki w tej metodzie wymyślałem ją na szybko w celu zobrazowania problemu.
map jest dziedziczony przez wiele klas, które są obsługiwane przez fabryke i ostatecznie zwracane na jednym endpoincie jako json. Tz 1 endpoint obsługuje np 100 klas. Czy to również zły pomysł?

A może niech fetchUsersWithRole zwraca na przykład Etiher'a: Either<FetchUserError, List<String>>. W zależności od dalszej logiki, albo możesz obsłużyć lewą stronę w metodzie getUsers albo przekazać taki rezultat dalej, aż do momentu gdzie będziesz mógł coś sensownego z tymi typami zrobić.

Zalety rozwiązania:

• Masz dokładnie powiedziane w typie co się może stać wywołując taką metodę (nie musisz zaglądać w wewnętrzną logikę metody)
• Zmuszasz użytkownika tej metody do obsługi/przekazania dalej typu
• Łatwo rozszerzać o kolejne "wyjątki"

Nie wczytywałem się dokładnie ale tutaj masz opisane jak możesz zmodelować lewą stronę jako ADT i co dalej można robić z takim Eitherem: https://medium.com/@sderosiaux/using-vavr-to-write-more-robust-java-code-87ccb0be12d7

zaylin napisał(a):

Celowo w fetchUsersWithRole dałem zwracanie przez referencje, więc either odpada.

Nie masz możliwości zmiany tego? Mutowanie listy poprzez przekazywanie referencji do niej to proszenie się o bugi. Tak napisany kod jest ciężki w czytaniu, a o rozumowaniu o nim już nie wspominając. W takim przypadku nigdy nie wiesz która metoda co zrobi z daną kolekcją i co się stanie.

zaylin napisał(a):

Poza tym wcześniej czy później zawartość either musi trafić do map więc musiałbym dodać checka czyli sprowadza się to do rozwiazania ze zwróceniem pustej listy.

Tak, dokładnie o to w tym przypadku rozbija się Either. W momencie, gdy już musisz coś z nim zrobić to używasz sobie np. fold i obsługujesz przypadki zarówno dla lewej jak i prawej strony. Logika jest przejrzysta i ładnie odseparowana pomiędzy "błędem", a normalnym przebiegiem programu.

Względem checka z pustą listą różnica jest taka, że obsługę błędu zostawiasz użytkownikowi metody.
W aktualnej funkcjonalności jakikolwiek błąd możesz zamienić na pustą listę. W innym przypadku, błąd tej metody mógłby powodować, że nie ma sensu kontynuować dalszego przebiegu funkcjonalności. Jeszcze w innym przypadku, niektóre błędy (pusta lista, lista która nie spełnia określonych warunków) mogłaby powodować zakończenie flow, a inne błędy nie. Either daje łatwą możliwość obsługiwania tych samych błędów w różny sposób, w zależności od miejsca użycia

1. Twój wyjątek to faktycznie wyjątek, bo raczej jak jakiś dbexception poleci to się nie masz jak z tego uratować - coś nie działa w programie, bazie trzeba się poddać. Czyli samo iżycie exception - prawidłowe.

2. trzymanie map jako parametr i wrzucanie tam pokątnie rzeczy to proszenie się o kłopoty.

3. jesli już gdzieś tego map wypełniasz (rezultatem zdrowym) to w tej samej metodzie wrzucaj też ten exception do map (czyli tam też rób catch) - trzeba to wrzucanie w map ograniczyć do konkretnych miejsc (metod) - im mniej tych miejsc tym lepiej.

4. map jest dziedziczony przez wiele klas, które są obsługiwane przez fabryke i... to raczej na pewno zły pomysł.
   generalnie warto unikać systuacji gdzie dane są gdzieś pod spodem zapisywane do czegoś -
   jak widzę metodę List<String> getUsers() no to spodziewam się, że zwróci mi listę... to że coś wrzuca do jakiegoś map to raczej jest duże zaskoczenie).
   Lepiej jakby ten map był parametrem i / lub wynikiem metody.

5. Spróbuj nie obsługiwać tego exception tak długo jak się da i dopiero na końcu robić catch i wsadzić do tego map. Tak żeby mieć najlepiej jeden takich catch na aplikację (im mniej tym lepiej).

Ale nadal za mało wiem o tym kodzie, żeby wiedzieć jak to zrobić dobrze, na razie mówie, że śmierdzi.