Czy `catch (…) {throw; } `zła praktyka?

74

Chociaż zgadzam się, że łapanie ... bez ponownego rzucania jest rzeczywiście złe, uważam jednak, że przy użyciu takich konstrukcji:

try
{
  // Stuff
}
catch (...)
{
  // Some cleanup
  throw;
}

Jest akceptowalny w przypadkach, w których RAII nie ma zastosowania . (Proszę, nie pytaj ... nie wszyscy w mojej firmie lubią programowanie obiektowe, a RAII jest często postrzegane jako „bezużyteczne rzeczy szkolne” ...)

Moi współpracownicy mówią, że zawsze powinieneś wiedzieć, jakie wyjątki mają być zgłaszane i że zawsze możesz używać konstrukcji takich jak:

try
{
  // Stuff
}
catch (exception_type1&)
{
  // Some cleanup
  throw;
}
catch (exception_type2&)
{
  // Some cleanup
  throw;
}
catch (exception_type3&)
{
  // Some cleanup
  throw;
}

Czy istnieje dobrze przyjęta dobra praktyka w tych sytuacjach?

ereOn
źródło
3
@Pubby: Nie jestem pewien, czy to dokładnie to samo pytanie. Połączony jest bardziej pytanie o „powinienem złapać ...”, a moje pytanie nacisk na „Powinienem lepiej złapać ...lub <specific exception>przed Ponowne generowanie”
ereOn
53
Przykro mi to mówić, ale C ++ bez RAII to nie C ++.
fredoverflow
46
Czyli twoi krowi robotnicy odrzucają technikę, która została wymyślona, ​​aby poradzić sobie z pewnym problemem, a następnie spierają się o to, którą z gorszych alternatyw należy zastosować? Przykro mi to mówić, ale wydaje się to głupie , bez względu na to, jak na to patrzę.
sbi
11
„łapanie ... bez ponownego rzucania jest rzeczywiście złe” - mylisz się. W main, catch(...) { return EXIT_FAILURE; }może mieć rację w kodzie, który nie działa w debuggerze. Jeśli nie złapiesz, stos może się nie rozwinąć. Tylko wtedy, gdy twój debugger wykryje niewyłapane wyjątki, chcesz, aby odeszły main.
Steve Jessop,
3
... więc nawet jeśli jest to „błąd programowania”, niekoniecznie wynika z tego, że nie chcesz o tym wiedzieć. W każdym razie twoi koledzy nie są dobrymi specjalistami od oprogramowania, więc jak mówi sbi, bardzo trudno jest mówić o tym, jak najlepiej poradzić sobie z sytuacją, która jest chronicznie słaba na początku.
Steve Jessop,

Odpowiedzi:

196

Moi współpracownicy mówią, że zawsze powinieneś wiedzieć, jakie wyjątki należy wprowadzić [...]

Twój współpracownik, chciałbym to powiedzieć, najwyraźniej nigdy nie pracował nad bibliotekami ogólnego przeznaczenia.

Jak, u licha, klasa może std::vectornawet udawać, że wie, co rzucą konstruktorzy kopii, jednocześnie gwarantując wyjątkowe bezpieczeństwo?

Gdybyście zawsze wiedzieli, co robi callee w czasie kompilacji, polimorfizm byłby bezużyteczny! Czasami głównym celem jest abstrahowanie od tego, co dzieje się na niższym poziomie, więc nie chcesz wiedzieć, co się dzieje!

Mehrdad
źródło
32
Właściwie nawet gdyby wiedzieli, że należy wprowadzić wyjątki. Jaki jest cel tego powielania kodu? O ile obsługa się nie różni, nie widzę sensu wymieniać wyjątków, by pochwalić się swoją wiedzą.
Michael Krelin - hacker
3
@ MichaelKrelin-hacker: To też. Dodaj do tego fakt, że wycofali specyfikacje wyjątków, ponieważ wymienianie wszystkich możliwych wyjątków w kodzie zwykle powodowało błędy później ... to najgorszy pomysł na świecie.
Mehrdad,
4
Niepokoi mnie to, co może być początkiem takiej postawy w połączeniu z postrzeganiem użytecznej i wygodnej techniki jako „bezużytecznych rzeczy szkolnych”. Ale cóż ...
Michael Krelin - haker
1
+1, wyliczenie wszystkich możliwych opcji jest doskonałym przepisem na przyszłą porażkę, dlaczego, u licha, ktoś miałby to zrobić ...?
littleadv,
2
Niezła odpowiedź. Przydałoby się wspomnieć, że jeśli kompilator, który należy obsługiwać, ma błąd w obszarze X, to korzystanie z funkcji z obszaru X nie jest inteligentne, a przynajmniej nie do bezpośredniego użycia. Na przykład, biorąc pod uwagę informacje o firmie, nie zdziwiłbym się, gdyby użyli Visual C ++ 6.0, który zawierał kilka niemądrych błędów w tym obszarze (np. Dwukrotne wywołanie niszczycieli obiektów wyjątków) - niektórzy mniejsi potomkowie wczesnych błędów przetrwali, aby tego dnia, ale wymagają ostrożnych uzgodnień, aby się przejawić.
Alf P. Steinbach,
44

To, co wydaje się, że Cię złapano, to piekło kogoś, kto próbuje zjeść swoje ciasto i je zjeść.

RAII i wyjątki są zaprojektowane tak, aby iść w parze. RAII to sposób, w jaki nie trzeba pisać wielu catch(...)instrukcji, aby wykonać czyszczenie. Oczywiście stanie się to automatycznie. Wyjątki są jedynym sposobem pracy z obiektami RAII, ponieważ konstruktory mogą odnieść sukces lub wyrzucić (lub wprowadzić obiekt w stan błędu, ale kto tego chce?).

catchOświadczenie może zrobić jedną z dwóch rzeczy: obsługiwać błąd lub wyjątkową okoliczność, czy prac porządkowych. Czasami robi to obie, ale każde catchoświadczenie istnieje, aby wykonać przynajmniej jedną z nich.

catch(...)nie jest w stanie poprawnie obsługiwać wyjątków. Nie wiesz jaki jest wyjątek; nie możesz uzyskać informacji o wyjątku. Nie masz absolutnie żadnych informacji poza faktem, że wyjątek został zgłoszony przez coś w pewnym bloku kodu. Jedyną uzasadnioną rzeczą, jaką możesz zrobić w takim bloku, jest wyczyszczenie. A to oznacza ponowne zgłoszenie wyjątku na koniec czyszczenia.

To, co RAII oferuje w odniesieniu do obsługi wyjątków, to bezpłatne czyszczenie. Jeśli wszystko jest poprawnie zamknięte w RAII, wszystko zostanie odpowiednio wyczyszczone. Nie musisz już mieć catchinstrukcji do czyszczenia. W takim przypadku nie ma powodu, aby pisać catch(...)oświadczenie.

Zgadzam się więc, że catch(...)to głównie zło ... tymczasowo .

Przepis ten dotyczy właściwego wykorzystania RAII. Ponieważ bez niego musisz być w stanie wykonać pewne porządki. Nie można tego obejść; musisz być w stanie wykonać prace porządkowe. Musisz mieć pewność, że zgłoszenie wyjątku pozostawi kod w rozsądnym stanie. I catch(...)jest do tego niezbędnym narzędziem.

Nie możesz mieć jednego bez drugiego. Nie można powiedzieć, że zarówno RAII, jak i catch(...) są złe. Potrzebujesz co najmniej jednego z nich; w przeciwnym razie nie jesteś wyjątkowo bezpieczny.

Oczywiście istnieje jedno ważne, choć rzadkie zastosowanie, catch(...)którego nawet RAII nie może odrzucić: exception_ptrskierowanie do kogoś innego. Zazwyczaj za pośrednictwem promise/futurepodobnego interfejsu.

Moi współpracownicy mówią, że zawsze powinieneś wiedzieć, jakie wyjątki mają być zgłaszane i że zawsze możesz używać konstrukcji takich jak:

Twój współpracownik jest idiotą (lub po prostu strasznie ignorantem). Powinno to być natychmiast oczywiste ze względu na to, ile kodu kopiuj i wklej sugeruje, abyś napisał. Czyszczenie każdej z tych instrukcji catch będzie dokładnie takie samo . To koszmar konserwacji, nie wspominając o czytelności.

W skrócie: jest to problem, który RAII stworzono do rozwiązania (nie dlatego, że nie rozwiązuje on innych problemów).

To, co mnie dezorientuje w tym pojęciu, to fakt, że zasadniczo jest to odwrotne do tego, jak większość ludzi twierdzi, że RAII jest zły. Ogólnie argumentem jest, że „RAII jest zły, ponieważ musisz użyć wyjątków, aby zasygnalizować awarię konstruktora. Ale nie możesz rzucać wyjątków, ponieważ nie jest to bezpieczne i będziesz musiał mieć wiele catchinstrukcji, aby wszystko wyczyścić”. Co jest zepsutym argumentem, ponieważ RAII rozwiązuje problem, który powoduje brak RAII.

Jest bardziej niż prawdopodobne, że jest przeciwko RAII, ponieważ ukrywa szczegóły. Wywołania niszczycieli nie są natychmiast widoczne w zmiennych automatycznych. Otrzymujesz kod, który jest wywoływany niejawnie. Niektórzy programiści naprawdę tego nie znoszą. Najwyraźniej do tego stopnia, że ​​uważają, że posiadanie 3 catchinstrukcji, z których wszystkie robią to samo z kodem kopiuj i wklej, jest lepszym pomysłem.

Nicol Bolas
źródło
2
Wygląda na to, że nie piszesz kodu, który zapewnia silną wyjątkową gwarancję bezpieczeństwa. RAII pomaga w zapewnieniu podstawowej gwarancji. Jednak w celu zapewnienia silnej gwarancji należy cofnąć niektóre działania, aby przywrócić system do stanu sprzed wywołania funkcji. Podstawową gwarancją jest czyszczenie , silną gwarancją jest wycofanie . Wykonanie przywracania jest specyficzne dla funkcji. Więc nie możesz umieścić go w „RAII”. I wtedy przydaje się blok typu catch-all . Jeśli piszesz kod z silną gwarancją, często używasz catch-all .
anton_rh
@anton_rh: Być może, ale nawet w tych przypadkach instrukcje typu catch-all są narzędziem ostateczności . Preferowanym narzędziem jest robienie wszystkiego, co rzuca, zanim zmienisz stan, który musiałbyś przywrócić w przypadku wyjątku. Oczywiście nie można zaimplementować wszystkiego w ten sposób we wszystkich przypadkach, ale jest to idealny sposób na uzyskanie silnej gwarancji wyjątku.
Nicol Bolas,
14

Naprawdę dwa komentarze. Po pierwsze, podczas gdy w idealnym świecie zawsze powinieneś wiedzieć, jakie wyjątki mogą zostać rzucone, w praktyce, jeśli masz do czynienia z bibliotekami stron trzecich lub kompilujesz za pomocą kompilatora Microsoft, to nie. Bardziej jednak do rzeczy; nawet jeśli znasz dokładnie wszystkie możliwe wyjątki, czy ma to znaczenie tutaj? catch (...)wyraża zamiar znacznie lepiej niż catch ( std::exception const& ), nawet zakładając, że wszystkie możliwe wyjątki pochodzą std::exception(co miałoby miejsce w idealnym świecie). Jeśli chodzi o stosowanie kilku bloków catch, jeśli nie ma wspólnej podstawy dla wszystkich wyjątków: jest to wręcz zaciemnianie i koszmar konserwacji. Jak rozpoznać, że wszystkie zachowania są identyczne? I o to właśnie chodziło? A co się stanie, jeśli będziesz musiał zmienić zachowanie (na przykład naprawić błąd)? Zbyt łatwo jest przegapić jedną.


źródło
3
W rzeczywistości mój współpracownik zaprojektował własną klasę wyjątków, która nie wywodzi się std::exceptioni codziennie próbuje wymusić jej użycie w naszej bazie kodów. Domyślam się, że próbuje mnie ukarać za używanie kodu i bibliotek zewnętrznych, których sam nie napisał.
ereOn
17
@ereOn Wydaje mi się, że twój współpracownik potrzebuje szkolenia. W każdym razie prawdopodobnie unikałbym korzystania z bibliotek napisanych przez niego.
2
Szablony i wiedza, jakie wyjątki zostaną rzucone, idą w parze jak masło orzechowe i martwe gekony. Coś tak prostego, co std::vector<>może zrzucić dowolny wyjątek z dowolnego powodu.
David Thornley,
3
Powiedz nam dokładnie, skąd wiesz, jaki wyjątek zostanie zgłoszony przez naprawę błędu jutra w dalszej części drzewa połączeń?
mattnz
11

Myślę, że twój współpracownik wymieszał kilka dobrych rad - powinieneś radzić sobie ze znanymi wyjątkami w catchbloku, kiedy ich nie wyrzucasz.

To znaczy:

try
{
  // Stuff
}
catch (...)
{
  // General stuff
}

Jest źle, ponieważ po cichu ukryje każdy błąd.

Jednak:

try
{
  // Stuff
}
catch (exception_type_we_can_handle&)
{
  // Deal with the known exception
}

Jest w porządku - wiemy, z czym mamy do czynienia i nie musimy narażać go na kod wywołujący.

Również:

try
{
  // Stuff
}
catch (...)
{
  // Rollback transactions, log errors, etc
  throw;
}

Jest w porządku, nawet najlepsza praktyka, kod zajmujący się ogólnymi błędami powinien zawierać kod, który je powoduje. Lepiej niż polegać na odbiorcy, aby wiedzieć, że transakcja wymaga wycofania się lub cokolwiek innego.

Keith
źródło
9

Każdej odpowiedzi twierdzącej lub nie powinno towarzyszyć uzasadnienie, dlaczego tak jest.

Mówienie, że jest to błędne tylko dlatego, że tak mnie nauczono, jest po prostu ślepym fanatyzmem.

Pisanie tego samego //Some cleanup; throwkilka razy, tak jak w twoim przykładzie, jest błędne, ponieważ jest to powielanie kodu i jest to obciążenie związane z utrzymaniem. Lepsze jest napisanie tego tylko raz.

Napisanie w catch(...)celu wyciszenia wszystkich wyjątków jest złe, ponieważ powinieneś obsługiwać tylko wyjątki, które wiesz, jak sobie z tym poradzić, a dzięki temu symbolowi wieloznacznemu możesz katować więcej, niż się spodziewasz, a to może wyciszyć ważne błędy.

Ale jeśli ponownie wrzucisz po a catch(...), to drugie uzasadnienie nie ma już zastosowania, ponieważ tak naprawdę nie zajmujesz się wyjątkiem, więc nie ma powodu, dla którego powinno się to zniechęcać.

W rzeczywistości zrobiłem to w celu zalogowania wrażliwych funkcji bez żadnego problemu:

void DoSomethingImportant()
{
    try
    {
        Log("Going to do something important");
        DoIt();
    }
    catch (std::exception &e)
    {
        Log("Error doing something important: %s", e.what());
        throw;
    }
    catch (...)
    {
        Log("Unexpected error doing something important");
        throw;
    }
    Log("Success doing something important");
}
pestofagiczny
źródło
2
Miejmy nadzieję, że Log(...)nie można rzucić.
Deduplicator
2

Generalnie zgadzam się z nastrojem zamieszczonych tutaj postów, naprawdę nie podoba mi się wzór wychwytywania określonych wyjątków - myślę, że składnia tego jest wciąż w powijakach i nie jest jeszcze w stanie poradzić sobie z niepotrzebnym kodem.

Ale ponieważ wszyscy to mówią, przyjmuję do wiadomości fakt, że chociaż używam ich oszczędnie, często patrzyłem na jedno z moich stwierdzeń „catch (Exception e)” i powiedziałem „Cholera, chciałbym zadzwonić poza tym szczególnym wyjątkiem ”, ponieważ kiedy przychodzisz później, często miło jest wiedzieć, jaki był zamiar i co klient rzuci na pierwszy rzut oka.

Nie usprawiedliwiam postawy „Zawsze używaj x”, po prostu mówię, że czasami miło jest widzieć je na liście i jestem pewien, że niektórzy ludzie uważają, że jest to właściwa droga.

Bill K.
źródło