Polimorficzne wyjątki - pomysł i wykonanie

TomRiddle napisał(a):

Np gdyby ktoś podał zły SECRET_KEY w .env, to chciałbym żeby część userowa na to zareagowała inaczej niż część adminiowa albo cronowa.

No to jest akurat przykład prawidłowego użycia wyjątku, tylko jak niby różne części mają na to różnie reagować? Aplikacja nie działa, jest pożar na prodzie, co tu więcej zrobić? :P

A co do postu @somekind, to to nie są 3 aplikacje. To jest jedna aplikacja, do której mogą się zalogować dwa typy userów (plus job w cronie). Panel admina i panel usera to jedna aplikacja.

Ok, więc jedna aplikacja, ale powiedzmy, że 3 moduły. Moje pytania nadal aktualne - po co więcej niż jeden catch dla jednego modułu?

TomRiddle napisał(a):

To tak na prawdę, gdyby takie potencjalne źródło problemu mogło wyjść z bardzo niskiego poziomu z logiki biznesowej, to to by znaczyło że praktycznie każda funkcja u mnie musiałaby zwracać Either. I'm not sure :/

To jakiś duży problem?
Poza tym, jeśli upewnisz się co do poprawności danych w logice aplikacyjnej, wtedy nie potrzebujesz eitherów w logice biznesowej, bo jesteś w stanie tworzyć ją w spójny sposób.

No, niby tak, ale ten sam problem jest z Eitherami, ktoś go może przechwycić i zwrócić inny either.

No nie bardzo, bo to jest wartość zwracana z funkcji, więc musisz ją jawnie przetworzyć, tego się nie da zepsuć przypadkiem jak flow opartego na wyjątkach (wpinając catch gdzieś w połowie między źródłem a celem).

Wysyłanie eventów IMO byłoby spoko, gdyby to były jakieś peryferia aplikacji (typu właśnie z web do kafki), ale w samej logice biznesowej to mi się wydaje bad idea.

Eventy służą do informowania kogoś o czymś, mogą wychodzić z różnych źródeł, w zależności od potrzeb.

Dla mnie ma sens :D Może po prostu trzeba dobrze znać wzorce, a nie tylko o nich słyszeć kiedyś :D Joke. No offence.

No tu trochę masz racji, bo o wizytorze tylko słyszałem, sam go nigdy nie używałem, ani nawet nie widziałem, żeby ktoś używał.
Nigdy też nie pytałem na 4p jak obsługiwać wyjątki. No offence. :P

No ale to są case'y których nie da się/ciężko sprawdzić przed wywołaniem flow. To jest coś w stylu

user.buyItem("Monitor");

...gdy nie wiemy ile user ma kasy i nie wiemy ile "Monitor" kosztuje, cała ta sprawdzajka dzieje się w środku logiki biznesowej. Żeby to sprawdzić zanim się ją zawoła, to trzebaby ją zduplikować w sumie.

Logiki nie trzeba duplikować - wystarczy ją rozdzielić.Najpierw sprawdzamy, czy operacja jest w ogóle możliwa (jeśli nie, to zwracamy błąd), potem ją wykonujemy (już bez problemów, za to możemy rzucić wyjątek, skoro stał się cud i niemożliwa do wystąpienia sytuacja prześlizgnęła się przez walidację).

No tak, i takie sytuacje też są, i takie również chcę obsłużyć - tylko że inaczej dla różnych caller'ów (inaczej dla usera, dla admina i dla crona).

To się generalnie wszystko zazębia - mniej rzucania wyjątków w aplikacji, a więcej zwracania wyników, sprawia, że nie trzeba komplikować obsługi wyjątków.

somekind napisał(a):

TomRiddle napisał(a):

Np gdyby ktoś podał zły SECRET_KEY w .env, to chciałbym żeby część userowa na to zareagowała inaczej niż część adminiowa albo cronowa.

No to jest akurat przykład prawidłowego użycia wyjątku, tylko jak niby różne części mają na to różnie reagować? Aplikacja nie działa, jest pożar na prodzie, co tu więcej zrobić? :P

No chociażby chciałbym userowi pokazać ładnego HTML'a z info że nie działa, a adminowi powiedzmy JSON'a z dokładnym napisem i informacją co się zjeb***. A job w cronie niech się wywali z hukiem.

A co do postu @somekind, to to nie są 3 aplikacje. To jest jedna aplikacja, do której mogą się zalogować dwa typy userów (plus job w cronie). Panel admina i panel usera to jedna aplikacja.

Ok, więc jedna aplikacja, ale powiedzmy, że 3 moduły. Moje pytania nadal aktualne - po co więcej niż jeden catch dla jednego modułu?

No bo w każdym chcę obsłużyć 4 wyjątki domenowe w slightly różne sposoby.

TomRiddle napisał(a):

No, niby tak, ale ten sam problem jest z Eitherami, ktoś go może przechwycić i zwrócić inny either.

No nie bardzo, bo to jest wartość zwracana z funkcji, więc musisz ją jawnie przetworzyć, tego się nie da zepsuć przypadkiem jak flow opartego na wyjątkach (wpinając catch gdzieś w połowie między źródłem a celem).

No tak, jak mówiłem wyżej - mniej prawdopodobne, ale still.

Jeśli masz drabinkę funkcji, to z Eitherami też wystarczy że jedna z nich połknie Eithera. I nie mam tu na myśli zmienić Either<Integer,?> na int, tylko np zwróci Either.left() zamiast Either.right(). Poza tym co do tego "niejawnie", to też nie jestem pewien, bo jak chcesz niejawnie dodać catcha? :D To by było "niejawne" tylko dla ludzi którzy automatycznie dodają catch (RuntimeException) wszędzie, albo ewentualnie olewają je w kodzie, bo tak do nich przywykli. Dla mnie taki catch byłby totalnie jawny.

Wysyłanie eventów IMO byłoby spoko, gdyby to były jakieś peryferia aplikacji (typu właśnie z web do kafki), ale w samej logice biznesowej to mi się wydaje bad idea.

Eventy służą do informowania kogoś o czymś, mogą wychodzić z różnych źródeł, w zależności od potrzeb.

Po prostu nie wydaje mi się żeby w corze domeny biznesowej to byłby dobry pomysł.

No ale to są case'y których nie da się/ciężko sprawdzić przed wywołaniem flow. To jest coś w stylu

user.buyItem("Monitor");

...gdy nie wiemy ile user ma kasy i nie wiemy ile "Monitor" kosztuje, cała ta sprawdzajka dzieje się w środku logiki biznesowej. Żeby to sprawdzić zanim się ją zawoła, to trzebaby ją zduplikować w sumie.

Logiki nie trzeba duplikować - wystarczy ją rozdzielić.Najpierw sprawdzamy, czy operacja jest w ogóle możliwa (jeśli nie, to zwracamy błąd), potem ją wykonujemy (już bez problemów, za to możemy rzucić wyjątek, skoro stał się cud i niemożliwa do wystąpienia sytuacja prześlizgnęła się przez walidację).

A to nie łamie zasady Tell-don't-ask?

@TomRiddle:

No właśnie zacząłem zauważać, że to co opisujesz jest bardzo podobne do checked-wyjątków.

I w istocie tak jest! Z tą różnicą, ze Eithery czy Optionale się wygodnie komponuje, szczególnie jak używasz jakichś streamów. Z wyjątkami sie tak nie da i wygląda to dramatycznie.

Ale mówiłeś że wyjątek ktoś może złapać i rzucić inny. Ja tylko mówię, że z Eitherami jest podobnie, ktoś może zjeść Either i zwrócić inny.

Tylko że jedno zrobić bardzo prosto przypadkiem, a drugiego w zasadzie sie nie da.

wyciszenie wyjątku to jest bug w domenie biznesowej

Ja nie mówie o łyknięciu wyjątku bez żadnej akcji! Ja mówie o złapaniu wyjątku "przypadkiem" robiąc handler który miał złapać coś innego. Np. twój kawałek kodu jest wołany przez metodę X a programista wie że ta metoda ma różne cuda na kiju w sobie i rzuca czasem wyjątkami, więc robi sobie nad nią try..catch i pisze obsługę dla tych wyjątków o których wie. Runtime'ów nie widać, więc nie ma pojęcia że twoja metoda też mogła coś rzucić a tym bardziej że ty oczekujesz ze ten wyjątek poleci gdzieś wyżej.

Shalom napisał(a):

TomRiddle napisał(a):

No właśnie zacząłem zauważać, że to co opisujesz jest bardzo podobne do checked-wyjątków.

I w istocie tak jest! Z tą różnicą, ze Eithery czy Optionale się wygodnie komponuje, szczególnie jak używasz jakichś streamów. Z wyjątkami sie tak nie da i wygląda to dramatycznie.

No dobra, no to załóżmy roboczo że wprowadziłbym Either<>, zamiast wyjątku. Jego left to byłby wynik operacji, a right to error.

Czy to by miało sens, żeby right byłby polimorficzny, i delegował handling do wizytora, tak jak opisałem w początkowym poście?

@TomRiddle:

Czy to by miało sens, żeby right byłby polimorficzny, i delegował handling do wizytora, tak jak opisałem w początkowym poście?

to jakiś contest pt. jak utrudnić wszystkim wejście w projekt? :P

Potwierdź mi tylko: wychodzimy od czegoś takiego?

public enum InvokedFromContext
{
    Cron,
    AdminWebPanel,
    AdminInternal
}

public static void DoStuff(InvokedFromContext context, decimal money)
{
    if (money < 0)
    {
        throw context switch
        {
            InvokedFromContext.Cron => new Cron_LackOfMoneyException(),
            InvokedFromContext.AdminWebPanel => new AdminWeb_LackOfMoneyException(),
            InvokedFromContext.AdminInternal => new AdminInternal_LackOfMoneyException(),
            _ => new NotImplementedException(),
        };
    }

    if (money == 5 && DateTime.Now.Hour == 21)
    {
        throw context switch
        {
            InvokedFromContext.Cron => new Cron_EdgeCase1OfMoneyException(),
            InvokedFromContext.AdminWebPanel => new AdminWeb_EdgeCase1OfMoneyException(),
            InvokedFromContext.AdminInternal => new AdminInternal_EdgeCase1OfMoneyException(),
            _ => new NotImplementedException(),
        };
    }

    Console.WriteLine("yolo");
}

static void Main(string[] args)
{
    var ctx = (InvokedFromContext)new Random().Next(0, 4);

    try
    {
        DoStuff(ctx, -1);
    }
    catch (Cron_LackOfMoneyException cron)
    {
        Console.WriteLine("cron lack");
    }
    catch (AdminWeb_LackOfMoneyException admin_web)
    {
        Console.WriteLine("admin_web lack");
    }
    catch (AdminInternal_LackOfMoneyException admin_internal)
    {
        Console.WriteLine("admin_internal lack");
    }
    catch (Cron_EdgeCase1OfMoneyException cron)
    {
        Console.WriteLine("cron edge");
    }
    catch (AdminWeb_EdgeCase1OfMoneyException admin_web)
    {
        Console.WriteLine("admin_web edge");
    }
    catch (AdminInternal_EdgeCase1OfMoneyException admin_internal)
    {
        Console.WriteLine("admin_internal edge");
    }
    catch (Exception ex)
    {
        // lol XD
    }
}

Miałem kiedyś podobny problem. Gdy coś się zadziało w systemie rzucałem wyjątkiem
po to aby 5 poziomów wyżej w warstwie web zostało to jakoś obsłużone. Pojawiał się nowy
case to się tworzyło nowy wyjątek generalnie dramat. Nawet zadałem pytanie na forum tutaj
ale chyba trochę zbyt skomplikowane bo za dużo odpowiedzi tam nie miałem :P

TomRiddle napisał(a):

No chociażby chciałbym userowi pokazać ładnego HTML'a z info że nie działa, a adminowi powiedzmy JSON'a z dokładnym napisem i informacją co się zjeb***. A job w cronie niech się wywali z hukiem.

No to pokazujesz i tyle. Nic więcej z wyjątkiem nie można zrobić. Potrzebujesz co najwyżej jednego catch per moduł do tego.

No bo w każdym chcę obsłużyć 4 wyjątki domenowe w slightly różne sposoby.

Nie, wywaliło się, więc koniec. Nie ma tak, że jeden wyjątek to katolik, inny muzułmanin, a trzeci buddysta, i kończą inaczej. Koniec jest taki sam dla wszystkich -> do piachu.

Jeśli masz drabinkę funkcji, to z Eitherami też wystarczy że jedna z nich połknie Eithera. I nie mam tu na myśli zmienić Either<Integer,?> na int, tylko np zwróci Either.left() zamiast Either.right(). Poza tym co do tego "niejawnie", to też nie jestem pewien, bo jak chcesz niejawnie dodać catcha? :D To by było "niejawne" tylko dla ludzi którzy automatycznie dodają catch (RuntimeException) wszędzie, albo ewentualnie olewają je w kodzie, bo tak do nich przywykli. Dla mnie taki catch byłby totalnie jawny.

Zobaczymy jak się okaże, że ktoś w tym misternym domku z wizytorów wstawi sobie catcha ignorującego wyjątki typu X, więc wszystkie dziedziczące po X już nie dotrą do użytkownika.

A to nie łamie zasady Tell-don't-ask?

Do aplikacji przychodzi jakiś request i:

1. najpierw walidujemy, czy dane w requeście są prawidłowe;
2. potem walidujemy czy operacja nie narusza reguł biznesowych;
3. wykonujemy operację;
4. jeśli się powiedzie, to: utrwalamy wynik, zwracamy informacje do użytkownika/emitujemy event, itp.

1 i 2 zwracają błąd, albo przepuszczają flow dalej, 1-4 mogą rzucić wyjątek w faktycznie wyjątkowej sytuacji (awaria infrastruktury, zewnętrznej zależności systemu, czy okazuje się, że dane, które miały być prawidłowe jednak nie są).
Jakie zasady to łamie?

Wzorzec "Odwiedzającego" jest prawdopodobnie najbardziej nielubianym wzorcem projektowym - przez to że nieintuicyjny.
Już bardziej lubiany jest singleton, przynajmniej przez część ludzi (tę mniej zorientowaną).

Zamiast tego co wymyśliłeś poszukaj czegoś o:

• (uncaught) exception handler, np. https://www.baeldung.com/java-global-exception-handler
• error code in exception, np. https://medium.com/@h4t0n/java-error-handling-pattern-f059d20ca5dc
• exception enrichment, np. http://tutorials.jenkov.com/java-exception-handling/exception-enrichment.html

somekind napisał(a):

No bo w każdym chcę obsłużyć 4 wyjątki domenowe w slightly różne sposoby.

Nie, wywaliło się, więc koniec. Nie ma tak, że jeden wyjątek to katolik, inny muzułmanin, a trzeci buddysta, i kończą inaczej. Koniec jest taki sam dla wszystkich -> do piachu.

No ale ja muszę je obsłużyć w inny sposób - albo inaczej, muszę wiedzieć dokładnie co się zjeb***, ergo który wyjątek poleciał.

Jeśli masz drabinkę funkcji, to z Eitherami też wystarczy że jedna z nich połknie Eithera. I nie mam tu na myśli zmienić Either<Integer,?> na int, tylko np zwróci Either.left() zamiast Either.right(). Poza tym co do tego "niejawnie", to też nie jestem pewien, bo jak chcesz niejawnie dodać catcha? :D To by było "niejawne" tylko dla ludzi którzy automatycznie dodają catch (RuntimeException) wszędzie, albo ewentualnie olewają je w kodzie, bo tak do nich przywykli. Dla mnie taki catch byłby totalnie jawny.

Zobaczymy jak się okaże, że ktoś w tym misternym domku z wizytorów wstawi sobie catcha ignorującego wyjątki typu X, więc wszystkie dziedziczące po X już nie dotrą do użytkownika.

To dla mnie jest zbyt słaby argument.

Może tak być, ale na to już odpisałem wyżej:

A) Z Either też się da tak zmapować na inny either żeby olał jakiś jeden przypadek
B) chciałbym mieć możliwość żeby jakiś wyjątek obsłużyć niżej, i do tego muszę mieć możliwość żeby go złapać i nie puścić dalej. A nawet jeśli coś go złapie, to dla mnie to jest bug w logice biznesowej, i moje unity powinny to wykryć.

A to nie łamie zasady Tell-don't-ask?

Do aplikacji przychodzi jakiś request i:

1. najpierw walidujemy, czy dane w requeście są prawidłowe;
2. potem walidujemy czy operacja nie narusza reguł biznesowych;
3. wykonujemy operację;
4. jeśli się powiedzie, to: utrwalamy wynik, zwracamy informacje do użytkownika/emitujemy event, itp.

1 i 2 zwracają błąd, albo przepuszczają flow dalej, 1-4 mogą rzucić wyjątek w faktycznie wyjątkowej sytuacji (awaria infrastruktury, zewnętrznej zależności systemu, czy okazuje się, że dane, które miały być prawidłowe jednak nie są).
Jakie zasady to łamie?

No dobra, 1 się zgadzam, musimy sprawdzić czy ktoś nie wsadził stringa do pola numerycznego, i inne takie, sure. Zgadzam się.

Ale 2 i 3 moim zdaniem łamie "Tell, don't ask principle".

Shalom napisał(a):

Nie wiem czym tu dla ciebie jest caller. Popatrz na ten kod z perspektywy kto chce prześledzić tą logikę. W normalnej sytuacji masz kod który woła jakąśtam metodę i jak wróci mu Either.Left to obsługuje otrzymany błąd. U ciebie ta obsługa błędu jest oddelegowana w jakieś zupełnie inne miejsce (trochę jak w jakimś AOP) co sprawia że nie ma szans odnaleźć się w kodzie.

No nie, ja chcę mieć coś takiego:

class DomainLogic {
  public void act() {
    if (błąd) throw DomainFirstException();
    if (błąd) throw DomainSecondException();
    if (błąd) throw DomainThirdException();
    if (błąd) throw DomainFourthException();
  }
}

I teraz mam tak

• Wołany z UserController

  try {
       domainLogic.act();
  } catch (DomainFirstException ex)  { handleFirstAsUser(ex);  }
  } catch (DomainSecondException ex) { handleSecondAsUser(ex); }
  } catch (DomainThirdException ex)  { handleThirdAsUser(ex);  }
  } catch (DomainFourthException ex) { handleFourthAsUser(ex); }
  

• Wołany z AdminController

  try {
       domainLogic.act();
  } catch (DomainFirstException ex)  { handleFirstAsAdmin(ex);  }
  } catch (DomainSecondException ex) { handleSecondAsAdmin(ex); }
  } catch (DomainThirdException ex)  { handleThirdAsAdmin(ex);  }
  } catch (DomainFourthException ex) { handleFourthAsAdmin(ex); }
  

• Wołany z crona

  try {
       domainLogic.act();
  } catch (DomainFirstException ex)  { cronHandleAndRetryFirst(ex);  }
  } catch (DomainSecondException ex) { cronHandleAndRetrySecond(ex); }
  } catch (DomainThirdException ex)  { cronHandleAndRetryThird(ex);  }
  } catch (DomainFourthException ex) { cronHandleAndRetryFourth(ex); }
  

Mój plan z wizytorem był taki:

• Wołany z UserController

  try {
       domainLogic.act();
  } catch (DomainException ex) { ex.visit(userVisitor); }
  

• Wołany z AdminController

  try {
       domainLogic.act();
  } catch (DomainException ex) { ex.visit(adminVisitor); }
  

• Wołany z crona

  try {
       domainLogic.act();
  } catch (DomainException ex) { ex.visit(cronVisitor); }
  

@TomRiddle: mogłeś zacząć od tego kodu w 1 poście. Wyjaśnij mi teraz w czym dokładnie pomaga tutaj ten twój wizytor? Bo pierwszy kod wygląda ok, wołasz logikę, może wylecieć kilka wyjątków, ty je obsługujesz. Gdzie jest zysk z tego wizytora?

TomRiddle napisał(a):

No ale ja muszę je obsłużyć w inny sposób - albo inaczej, muszę wiedzieć dokładnie co się zjeb***, ergo który wyjątek poleciał.

A możesz wyjaśnić dlaczego? Czy są jakieś wyjątki, które ukrywasz przed użytkownikiem/adminem? Bo do tej pory pisałeś, że trzeba wyświetlić komunikat - a do tego nie trzeba znać typu wyjątku.

A) Z Either też się da tak zmapować na inny either żeby olał jakiś jeden przypadek

Nie da się popełnić analogicznej pomyłki tak łatwo, bo musisz jawnie coś przyjąć i jawnie zwrócić. O przypadkowe połknięcie wyjątku/hierarchii wyjątków jest dużo łatwiej.

B) chciałbym mieć możliwość żeby jakiś wyjątek obsłużyć niżej, i do tego muszę mieć możliwość żeby go złapać i nie puścić dalej. A nawet jeśli coś go złapie, to dla mnie to jest bug w logice biznesowej, i moje unity powinny to wykryć.

No tak, powinny.

Ale 2 i 3 moim zdaniem łamie "Tell, don't ask principle".

W jaki niby sposób? Rozumiesz, że walidator reguł biznesowych jest częścią domeny?

Shalom napisał(a):

@TomRiddle: mogłeś zacząć od tego kodu w 1 poście. Wyjaśnij mi teraz w czym dokładnie pomaga tutaj ten twój wizytor? Bo pierwszy kod wygląda ok, wołasz logikę, może wylecieć kilka wyjątków, ty je obsługujesz. Gdzie jest zysk z tego wizytora?

@Shalom: Bo chciałbym dodawać nowe catche i nowe handlery, bez potrzeby edycji UserController, AdminController ani innych wołaczy.

somekind napisał(a):

TomRiddle napisał(a):

No ale ja muszę je obsłużyć w inny sposób - albo inaczej, muszę wiedzieć dokładnie co się zjeb***, ergo który wyjątek poleciał.

A możesz wyjaśnić dlaczego? Czy są jakieś wyjątki, które ukrywasz przed użytkownikiem/adminem? Bo do tej pory pisałeś, że trzeba wyświetlić komunikat - a do tego nie trzeba znać typu wyjątku.

To chyba nie ma znaczenia.

A) Z Either też się da tak zmapować na inny either żeby olał jakiś jeden przypadek

Nie da się popełnić analogicznej pomyłki tak łatwo, bo musisz jawnie coś przyjąć i jawnie zwrócić. O przypadkowe połknięcie wyjątku/hierarchii wyjątków jest dużo łatwiej.

Odnosiłem się już do tego we wcześniejszym poście.

Dla mnie złapanie wyjątku i rzucenie innego, to mniej więcej taka sama implementacja jak przyjęcie Either<> i zwrócenie innego. Co do tego "jawne", to już mówiłem, nie da się "niejawnie złapać wyjątku". Musisz napisać catch ().

Dla niektórych catch (RuntimeException e) {} jest "niejawnym", ale tylko dlatego że zdążyli do tego przywyknąć? I guess? Dla mnie taki catch jest totalnie jawny.

Ale 2 i 3 moim zdaniem łamie "Tell, don't ask principle".

W jaki niby sposób? Rozumiesz, że walidator reguł biznesowych jest częścią domeny?

No, zasada TDA mówi

domainLogic.act(); // tell

vs

if (domainLogic.canAct(params)) { // ask
  domainLogic.act();
}

No zasada "Tell, don't ask" mówi, żeby robić "tell", zamiast "ask" :D https://martinfowler.com/bliki/TellDontAsk.html

TomRiddle napisał(a):

Shalom napisał(a):

@TomRiddle: mogłeś zacząć od tego kodu w 1 poście. Wyjaśnij mi teraz w czym dokładnie pomaga tutaj ten twój wizytor? Bo pierwszy kod wygląda ok, wołasz logikę, może wylecieć kilka wyjątków, ty je obsługujesz. Gdzie jest zysk z tego wizytora?

Bo chciałbym dodawać nowe catche i nowe handlery, bez potrzeby edycji UserController, AdminController ani innych wołaczy.

Nadal musisz edytować admin visitora, user visitor i cron visitora. Przeniosłeś logię w inne miejsce. Nie widzę zysku. Jest jedynie warstwa pośrednik.

TomRiddle napisał(a):

No zasada "Tell, don't ask" mówi, żeby robić "tell", zamiast "ask" :D https://martinfowler.com/bliki/TellDontAsk.html

Jak już wklejasz Fowlera, to przeczytaj co tam pisze:

*But personally, I don't use tell-dont-ask. I do look to co-locate data and behavior, which often leads to similar results. One thing I find troubling about tell-dont-ask is that I've seen it encourage people to become GetterEradicators, seeking to get rid of all query methods. But there are times when objects collaborate effectively by providing information. A good example are objects that take input information and transform it to simplify their clients, such as using EmbeddedDocument. I've seen code get into convolutions of only telling where suitably responsible query methods would simplify matters [1]. For me, tell-don't-ask is a stepping stone towards co-locating behavior and data, but I don't find it a point worth highlighting.
*

Czasami złamanie zasady może być warte.

nalik napisał(a):

TomRiddle napisał(a):

Shalom napisał(a):

@TomRiddle: mogłeś zacząć od tego kodu w 1 poście. Wyjaśnij mi teraz w czym dokładnie pomaga tutaj ten twój wizytor? Bo pierwszy kod wygląda ok, wołasz logikę, może wylecieć kilka wyjątków, ty je obsługujesz. Gdzie jest zysk z tego wizytora?

Bo chciałbym dodawać nowe catche i nowe handlery, bez potrzeby edycji UserController, AdminController ani innych wołaczy.

Nadal musisz edytować admin visitora, user visitor i cron visitora. Przeniosłeś logię w inne miejsce. Nie widzę zysku. Jest jedynie warstwa pośrednik.

No tak? Dokładnie taki efekt chciałem uzyskać.

Co, Ty myślałeś że sobie "usunę logikę", tak że będzie "nigdzie" to nadal będzie działać?

Ten handling nadal gdzieś musi być, ale nie chcę żeby był w miejscu który woła domenę biznesową.

Zysk jest taki że można niezaleznie od siebie rowzijać callery od handlerów.

TomRiddle napisał(a):

No zasada "Tell, don't ask" mówi, żeby robić "tell", zamiast "ask" :D https://martinfowler.com/bliki/TellDontAsk.html

Jak już wklejasz Fowlera, to przeczytaj co tam pisze:

Wkleiłem to żeby przybliżyć zasadę, tym którzy o niej nie słyszeli.

Masz jakiś merytoryczny argument do dodania? Czy chcesz tylko wklejać wpisy innych ludzi? Jeśli to pierwsze, to chętnie posłucham jakie masz alternatywne pomysły na wizytora w wyjątkach. Jeśli nie, to bardzo dziękuję za odpowiedź.

TomRiddle napisał(a):

nalik napisał(a):

TomRiddle napisał(a):

Shalom napisał(a):

@TomRiddle: mogłeś zacząć od tego kodu w 1 poście. Wyjaśnij mi teraz w czym dokładnie pomaga tutaj ten twój wizytor? Bo pierwszy kod wygląda ok, wołasz logikę, może wylecieć kilka wyjątków, ty je obsługujesz. Gdzie jest zysk z tego wizytora?

Bo chciałbym dodawać nowe catche i nowe handlery, bez potrzeby edycji UserController, AdminController ani innych wołaczy.

Nadal musisz edytować admin visitora, user visitor i cron visitora. Przeniosłeś logię w inne miejsce. Nie widzę zysku. Jest jedynie warstwa pośrednik.

No tak? Dokładnie taki efekt chciałem uzyskać.

Co, Ty myślałeś że sobie "usunę logikę", tak że będzie "nigdzie" to nadal będzie działać?

Ten handling nadal gdzieś musi być, ale nie chcę żeby był w miejscu który woła domenę biznesową.

Ważna zasadą wydaje mi się obsługa wyjątku jak najbliżej miejsca gdzie wystąpił. Usuwanie tego dalej, hmm, no niepotrzebne.
Kod się pisze dla ludzi. Forum nie jest oczywiście reprezentatywne, ale jak na razie ciężko przekonać, że to dobry pomysł.

Co do punkty drugiego. Chodziło mi o pokazanie, iż nie zawsze owa zasada ma sens. Walidacja jest częścią domeny. Znajduje się blisko danych. To nie to samo co odpytywanie o dane przy pomocy geterozy i robienie walidacji poza domeną.

nalik napisał(a):

Ważna zasadą wydaje mi się obsługa wyjątku jak najbliżej miejsca gdzie wystąpił.

E?

Pierwsze słyszę.

Co do punkty drugiego. Chodziło mi o pokazanie, iż nie zawsze owa zasada ma sens. Walidacja jest częścią domeny. Znajduje się blisko danych. To nie to samo co odpytywanie o dane i robienie walidacji poza domeną.

Noi walidacja jest w domenie. Reakcja na nieudaną walidację już nie.

TomRiddle napisał(a):

nalik napisał(a):

Ważna zasadą wydaje mi się obsługa wyjątku jak najbliżej miejsca gdzie wystąpił.

E?

Pierwsze słyszę. Ważna zasadą wydaje mi się obsługa wyjątku jak najbliżej miejsca gdzie wystąpił.

Doprecyzuję. Ważna zasadą wydaje mi się obsługa wyjątku jak najbliżej miejsca gdzie wystąpił, tam gdzie jesteśmy wstanie zareagować. Bez zbędnego propagowania wyjątku zbyt wysoko. W tym wypadku reakcja to przerwanie operacji domenowej, ewentualnie w ogole jej nie wykonanie, zalogowanie odpowiedniej wiadomości i coś związanego z kontekstem wykonania. W domenie oczywiście nikt nie będzie obsługiwał wyjątku specjalnie dla admina czy crona, jeżeli nie jest częścią domeny.

Co do punkty drugiego. Chodziło mi o pokazanie, iż nie zawsze owa zasada ma sens. Walidacja jest częścią domeny. Znajduje się blisko danych. To nie to samo co odpytywanie o dane i robienie walidacji poza domeną.

Noi walidacja jest w domenie. Reakcja na nieudaną walidację już nie.

Tak musi być. Reakcją w tym momencie jest nie wykonanie akcji domenowej. A także wykonanie akcji specyficznej dla kontekstu wykonania (admin, cron, user). Ta reakcja i tak będzie poza domeną, bo domena ma gdzieć crona czy logowanie dla admina, jeżeli admin nie jest częścią domeny. Więc wartwa aplikacji wydaje się odpowiednią warstwę na obsługę tego wyjątku. Dodatkowy skok w obdłudze wyjątku przez domenę (wyjątek -> akceptacja odwiedzającego w wyjątku domenowy -> handler będący odwiedzającym poza domeną) w moich oczach nie jest benefitem, a jedynie utrudnia zrozumienie kodu (zamiast jednego rzutu okiem na kod trzeba debugować, sprawdzać jaki jest wizytor, zmapować mentalnie schemat wykonania). Podczas próby rozwiązania niekoniecznie bezpośrednio związanego problemu jest to dodatkowy narzut poznawczy. Jakby benefit był naprawde spory, to być może warto. A tutaj po dodaniu wyjątków i tak trzeba modyfikować kod obsługi wyjątków. Roboty jest tyle samo. Jedynie w innym miejscu. Kosztem jest z kolei niespodziewana i nietypowa architektura, co widać po reakcji wypowiadających się.

nalik napisał(a):

[...]

Doprecyzuję. Ważna zasadą wydaje mi się obsługa wyjątku jak najbliżej miejsca gdzie wystąpił, tam gdzie jesteśmy wstanie zareagować. Bez zbędnego propagowania wyjątku zbyt wysoko. W tym wypadku reakcja to przerwanie operacji domenowej, ewentualnie w ogole jej nie wykonanie, zalogowanie odpowiedniej wiadomości i coś związanego z kontekstem wykonania. W domenie oczywiście nikt nie będzie obsługiwał wyjątku specjalnie dla admina czy crona, jeżeli nie jest częścią domeny.

[...]

Tak musi być. Reakcją w tym momencie jest nie wykonanie akcji domenowej. A także wykonanie akcji specyficznej dla kontekstu wykonania (admin, cron, user). Ta reakcja i tak będzie poza domeną, bo domena ma gdzieć crona czy logowanie dla admina, jeżeli admin nie jest częścią domeny. Więc wartwa aplikacji wydaje się odpowiednią warstwę na obsługę tego wyjątku. Dodatkowy skok w obdłudze wyjątku przez domenę (wyjątek -> akceptacja odwiedzającego w wyjątku domenowy -> handler będący odwiedzającym poza domeną) w moich oczach nie jest benefitem, a jedynie utrudnia zrozumienie kodu (zamiast jednego rzutu okiem na kod trzeba debugować, sprawdzać jaki jest wizytor, zmapować mentalnie schemat wykonania). Podczas próby rozwiązania niekoniecznie bezpośrednio związanego problemu jest to dodatkowy narzut poznawczy. Jakby benefit był naprawde spory, to być może warto. A tutaj po dodaniu wyjątków i tak trzeba modyfikować kod obsługi wyjątków. Roboty jest tyle samo. Jedynie w innym miejscu. Kosztem jest z kolei niespodziewana i nietypowa architektura, co widać po reakcji wypowiadających się.

Dziękuję za wypowiedź, jednak moim zdaniem, to byłoby 10 kroków w tył, w dziedzinie wytwarzania dobrego oprogramowania. Oddzielenie logiki biznesowej od peryferiów jest czymś absolutnie niezbędnym do poprawnego utrzymania aplikacji, także nie mogę przenieść handlowania w głąb logiki biznesowej.

Pozdrawiam.

nalik napisał(a):

Nie widzę tight couping. Wartswa aplikacji decyduje co zrobić. Domena nic nie wie jak wyjątek będzie obsłużony. Zależność jest w jedną stronę. Wręcz przeciwnie, to w przypadku wizytora domena narzuca implementację obsługi, bo zmusza do implementacji odwiedzającego.

Ehh, dobrze, dam Ci szansę.

Jak więc Ty rozwiązałbyś ten problem? Mógłbyś pokazać przykład implementacji, lub opisać Twoje rozwiązanie? Zwięźle?

Nic bym nie rozwiązywał, bo trudno mi dostrzec benefity z ruszania takiego kodu. Zostawiłbym tak jak było. Dla mnie obsługa w kontrolerze ma sens. Proste, głupie, każdy zrozumie o co chodzi, kodu tyle samo co w przypadku odwiedzającego.

nalik napisał(a):

Skoro kontroler u was z wie, że akcja wykonała się w kontekście crona czy admina, to rozumiem, że to część jego zadania. Wie jak obsłużyć niepowodzenie w warstwie domeny. Obsługę wyjątku w tym miejscu nie uważam za złamanie SRP. Co do open/close, to nie wiem jak to odnieść w ogóle. Chyba nie tworzycie hierarchii klas kontrolerów, więc zastosowania open/close tu nie dostrzegam.

Okej, przeczytałem Twoją opinię - dziękuję za sprezentowanie jej tak dobitnie.

Jednak uważam że to krok wtył, dlatego raczej nie pójdę z Twoim pomysłem. Dzięki.

TomRiddle napisał(a):

Jednak uważam że to krok wtył, dlatego raczej nie pójdę z Twoim pomysłem. Dzięki.

Tylko w tył względem czego? Względem wizytora?
Bo jak dla mnie wizytor to jakiś dziwny krok w bok, który w sumie nie wiadomo co miałby dać.

a tak?

public interface IExceptionHandler
{
    void Handle(DomainException exception);

    void Handle(LackOfMoneyException exception);

    void Handle(EdgeCaseException exception);
}

i wtedy

var controller = new UserController(new CronExceptionHandler());
controller.DoStuff(-5);

public class UserController
{
    public IExceptionHandler Handler { get; }

    public UserController(IExceptionHandler handler)
    {
        Handler = handler;
    }

    public void DoStuff(decimal money)
    {
        try
        {
            Validate(money);
            Console.WriteLine(money);
        }
        catch (DomainException ex)
        {
            Handler.Handle(ex);
        }
    }

    private void Validate(decimal money)
    {
        if (money < 0)
            throw new LackOfMoneyException();

        throw new EdgeCaseException();
    }
}

a handler tak:

public class CronExceptionHandler : IExceptionHandler
{
    public void Handle(DomainException exception)
    {
        if (exception is LackOfMoneyException lack)
        {
            Handle(lack);
        }
        else if (exception is EdgeCaseException edge)
        {
            Handle(edge);
        }
        else
        {
            throw new NotImplementedException();
        }
    }

    public void Handle(LackOfMoneyException exception)
    {
        Console.WriteLine("lack");
    }

    public void Handle(EdgeCaseException exception)
    {
        Console.WriteLine("edge");
    }
}

i teraz, gdy dodasz nowy wyjątek np EdgeCaseException2.

public interface IExceptionHandler
{
    void Handle(DomainException exception);

    void Handle(LackOfMoneyException exception);

    void Handle(EdgeCaseException exception);

    void Handle(EdgeCaseException2 exception);
}

to dostaniesz errora

'CronExceptionHandler' does not implement interface member 'Program.IExceptionHandler.Handle(EdgeCaseException2)'

a zatem masz tam jakąś safety, no chyba że zaimplementujesz metodę pod dany Exception, a zapomnisz dodać do Handle(DomainException exception), chociaż jak bardzo ci na tym zależy to może za pomocą refleksji?

masz przeniesioną tą logikę obsługi i nie musisz Controllera ruszać

co sądzisz?

TomRiddle napisał(a):

To chyba nie ma znaczenia.

Ma znaczenie, bo od początku wygląda jakbyś rozwiązywał problem x/y powstały najprawdopodobniej z nadużywania wyjątków do sterowania logiką biznesową.

Dla mnie złapanie wyjątku i rzucenie innego, to mniej więcej taka sama implementacja jak przyjęcie Either<> i zwrócenie innego. Co do tego "jawne", to już mówiłem, nie da się "niejawnie złapać wyjątku". Musisz napisać catch ().

I tym właśnie catchem można złapać przypadkiem więcej wyjątków niż się zamierza. Z Eitherem nie ma tego problemu, bo się go "nie łapie", tylko przekazuje dalej do miejsca, które jest w stanie go zinterpretować.

No, zasada TDA mówi

domainLogic.act(); // tell

vs

if (domainLogic.canAct(params)) { // ask
  domainLogic.act();
}

No zasada "Tell, don't ask" mówi, żeby robić "tell", zamiast "ask" :D https://martinfowler.com/bliki/TellDontAsk.html

Czy ta zasada mówi, że logika biznesowa to jest jedna klasa? Bo nie wydaje mi się. Zasada dotyczy hermetyzowania operacji w obiekcie, zamiast udostępniania danych na zewnątrz, aby ktoś inny mógł na nich operować.

Ja niczego podobnego nie sugeruję. walidacja biznesowa to proces, który może wykorzystywać wiele obiektów, zupełnie różnych niż obiekty użyte później do przeprowadzenia samej operacji.

TomRiddle napisał(a):

Ten handling nadal gdzieś musi być, ale nie chcę żeby był w miejscu który woła domenę biznesową.

I tak będzie - po prostu przeniesiony do czegoś, co nazywasz wizytorem. I zamiast dopisać nowy catch tam, gdzie trzeba, to będziesz musiał dopisać nową metodę. (Czasami pewnie pustą, bo tam gdzieś pewnie pałęta się boski interfejs łamiący ISP.)

Zysk jest taki że można niezaleznie od siebie rowzijać callery od handlerów.

Równie dobrze mógłbyś mieć klasę pomocniczą (albo trzy), która przyjmuje różne wyjątki i coś z nimi robi. To nadal będzie dziwne, nadal będzie wyglądało jak dziwaczne używanie wyjątków, ale przynajmniej nie będzie udawało wizytora.

Celem istnienia wizytora jest dodawanie nowych operacji do istniejących hierarchii obiektów. A wg Ciebie problemem jest to, że pojawiają się nowe obiekty. Coś tu nie gra.

Dla mnie złapanie wyjątku i rzucenie innego, to mniej więcej taka sama implementacja jak przyjęcie Either<> i zwrócenie innego. Co do tego "jawne", to już mówiłem, nie da się "niejawnie złapać wyjątku". Musisz napisać catch ().

I tym właśnie catchem można złapać przypadkiem więcej wyjątków niż się zamierza. Z Eitherem nie ma tego problemu, bo się go "nie łapie", tylko przekazuje dalej do miejsca, które jest w stanie go zinterpretować.

Semantyka. W obu przypadkach wystarczy jedna nieudana metoda żeby połknąć błąd. Nie mówię że wyjątki lepsze od Either<>ów; ale z tym konkretnym argumentem się nie zgadzam. Być może są inne?

TomRiddle napisał(a):

Ten handling nadal gdzieś musi być, ale nie chcę żeby był w miejscu który woła domenę biznesową.

I tak będzie - po prostu przeniesiony do czegoś, co nazywasz wizytorem. I zamiast dopisać nowy catch tam, gdzie trzeba, to będziesz musiał dopisać nową metodę. (Czasami pewnie pustą, bo tam gdzieś pewnie pałęta się boski interfejs łamiący ISP.)

No, dokładnie taki był mój początkowy zamysł. A SOLID to umnie recytują przez sen, więć o złamanie ISP bym się nie martwił.

Zysk jest taki że można niezaleznie od siebie rowzijać callery od handlerów.

Równie dobrze mógłbyś mieć klasę pomocniczą (albo trzy), która przyjmuje różne wyjątki i coś z nimi robi. To nadal będzie dziwne, nadal będzie wyglądało jak dziwaczne używanie wyjątków, ale przynajmniej nie będzie udawało wizytora.

Celem istnienia wizytora jest dodawanie nowych operacji do istniejących hierarchii obiektów. A wg Ciebie problemem jest to, że pojawiają się nowe obiekty. Coś tu nie gra.

Panie i Panowie, 19ty post w wątku; 18 poprzednich wylicza rzeczy których nie robić - 19ty wątek od @somekind rzuca pierwszą nieśmiałą sugestię co robić. Brawo.

Trwało to trzy dni, ale w końcu jest jedna sugestia, yupi. Czekamy na następne. Jeśli to wzrost liniowy, to teraz muszę zcierpić kolejne 18 nic nie wartch postów, zanim kolejny 19ty da mi drugą wartościową sugestię.

Ale żarty, na bok. Ja może przypomnę skąd się wziął ten temat i zrefrazuję pierwszy post, bo większość tutaj chyba tego nie zczaiła.

Mam hierarchię wyjątków w swojej aplikacji, widzę że dochodzą szybko nowe case'y do handlowania, i stwierdziłem że dobrze byłoby przerobić taką drabinkę catch na coś bardziej ogarniętego. Nie zacząłem jeszcze żadnej pracy nad tym, na razie się przystawiam.

Pomysł jaki miałem to wizytor w wyjątkach, i wpadłem na niego z dwóch powodów:

• Po pierwsze, mógłbym dodawać nowe wyjątki bez edycji callerów, o tym już mówiłem
• Jak jakiś caller nie będzie chciał używać wizytora, to nadal może robić zwykłe catche, no problemo

Nie byłem jednak do końca przekonany, więc podbiłem na forum, z pytaniem o to, czy ktoś inny ma jakiś pomysł, jak to można ogarnąć inaczej. Jakieś inne podejście, jak obronić callerów przed dodawaniem catchy - spodziewałem się oporu, dziesiątek postów typu "stahp, don't do this", kilku ludzi którym nie spodoba się odejście od przyklepanych schematów; ale liczyłem że pomiędzy nimi prędzej czy później pojawi się merytoryczna, sensowna odpowiedź - były, od @Shalom i od @somekind - Niestety one dotyczyły zagadnienia, czemu nie powinienem dodawać wizytora i zostawić tak jak jest; a ja szukałem alternatywnych rozwiązań, może jakichś nietypowych innych podejść. Opisałem wizytor bo to był po prostu mój pierwszy pomysł na to. Gdybym wiedział że 19 postów się do niego przyczepi to nigdy bym o nim nie wspominał.

Bracie TomRiddle, a może tak nieco inaczej podejść do zagadnienia [pomysł tak z grubsza]:

1. Stwórz klasę BaseException który będzie zawierał wartość jakiegoś enuma ExceptionType; na chwilę obecną będziesz miał 4 wartości. Będzie trzeba więcej - dokodujesz.
2. Stwórz klasę ExceptionSmth (wymyśl jakąś dobrą nazwę) który będzie siedział w każdym catche'u, i łapał wszystkie wyjątki jakie polecą. Będzie także zawierał mapę <ExceptionType, nazwa dedykowanego dla typu handlera>. Może też zawierać jakieś stany czy coś, w zależności od których obsługa złapanego wyjątku może nieco się różnić.
3. Jeśli złapany wyjątek nie będzie pochodną od BaseException to ma zostać rzucony w świat ponownie.
4. Jak się trafi wyjątek właściwego typu z mapy odczytasz jakiego handlera do niego użyć.
5. Przyda się teraz jakaś klasa HandlerFactory, która stworzy i zwróci obiekt do obsługi wyjątku. To przyda się na sytuacje kiedy czasem coś tam tym obiekcie handlera potrzebowałbyś porzeźbić, np. jakoweś dependency injection mu zapodać zanim wpuścisz do niego wyjątek.
6. Wpuszczasz wyjątek do Handlera, a on robi robotę.

MasterBLB napisał(a):

Bracie TomRiddle, a może tak nieco inaczej podejść do zagadnienia [pomysł tak z grubsza]:

1. Stwórz klasę BaseException który będzie zawierał wartość jakiegoś enuma ExceptionType; na chwilę obecną będziesz miał 4 wartości. Będzie trzeba więcej - dokodujesz.
2. Stwórz klasę ExceptionSmth (wymyśl jakąś dobrą nazwę) który będzie siedział w każdym catche'u, i łapał wszystkie wyjątki jakie polecą. Będzie także zawierał mapę <ExceptionType, nazwa dedykowanego dla typu handlera>. Może też zawierać jakieś stany czy coś, w zależności od których obsługa złapanego wyjątku może nieco się różnić.
3. Jeśli złapany wyjątek nie będzie pochodną od BaseException to ma zostać rzucony w świat ponownie.
4. Jak się trafi wyjątek właściwego typu z mapy odczytasz jakiego handlera do niego użyć.
5. Przyda się teraz jakaś klasa HandlerFactory, która stworzy i zwróci obiekt do obsługi wyjątku. To przyda się na sytuacje kiedy czasem coś tam tym obiekcie handlera potrzebowałbyś porzeźbić, np. jakoweś dependency injection mu zapodać zanim wpuścisz do niego wyjątek.
6. Wpuszczasz wyjątek do Handlera, a on robi robotę.

No a to nie byłby specjalny przypadek wizytora, w którym implementacją acceptów jest get() z mapy?