Dlaczego puste bloki połowowe są złym pomysłem? [Zamknięte]

194

Właśnie widziałem pytanie o try-catch , które ludzie (w tym Jon Skeet) twierdzą, że puste bloki catch to naprawdę zły pomysł? Dlaczego to Czy nie ma takiej sytuacji, w której pusty haczyk nie byłby złą decyzją projektową?

Chodzi mi na przykład, że czasami chcesz skądś uzyskać dodatkowe informacje (usługa internetowa, baza danych) i naprawdę nie obchodzi Cię, czy dostaniesz te informacje, czy nie. Więc próbujesz to zdobyć, a jeśli coś się stanie, to dobrze, dodam tylko „catch (wyjątek zignorowany) {}” i to wszystko

Samuel Carrijo
źródło
53
Napiszę komentarz wyjaśniający, dlaczego powinien być pusty.
Mehrdad Afshari,
5
... lub przynajmniej zalogować, że został złapany.
matt b
2
@ocdecio: unikaj go dla kodu czyszczenia , nie unikaj go w ogóle.
John Saunders,
1
@ocdecio: Innym przypadkiem, w którym należy unikać użycia try..catch, jest złapanie wyjątków dla znanych typów awarii. np. wyjątki konwersji numerycznej
MPritchard,
1
@ocdecio - try..finally jest lepszy niż try..empty catch, ponieważ błąd utrzymuje się na stosie - aby zostać obsłużonym przez framework lub zabić program i wyświetlić się użytkownikowi - oba wyniki są lepsze niż „ cicha awaria ".
Nate

Odpowiedzi:

298

Zwykle puste try-catch jest złym pomysłem, ponieważ cicho połykasz warunek błędu, a następnie kontynuujesz wykonywanie. Czasami może to być słuszne, ale często jest to znak, że deweloper widział wyjątek, nie wiedział, co z tym zrobić, i dlatego użył pustego haczyka, aby wyciszyć problem.

Jest to programowy odpowiednik nałożenia czarnej taśmy na lampkę ostrzegawczą silnika.

Uważam, że sposób postępowania z wyjątkami zależy od warstwy oprogramowania, w której pracujesz: wyjątki w lasach tropikalnych .

Ned Batchelder
źródło
4
W tych naprawdę rzadkich okolicznościach, w których naprawdę nie potrzebuję wyjątku, a rejestrowanie jest z jakiegoś powodu niedostępne, muszę skomentować, że jest to celowe - i dlaczego nie jest potrzebne, i powtarzam, że nadal wolałbym je zalogować, jeśli było to wykonalne w tym środowisku. Zasadniczo sprawia, że ​​komentarz WIĘCEJ działa, niż rejestrowanie, gdyby byłaby to opcja w takich okolicznościach. Na szczęście mam tylko 2 klientów, dla których jest to problem, i to tylko podczas wysyłania niekrytycznych wiadomości e-mail z ich stron internetowych.
John Rudy,
37
Uwielbiam również analogię - i jak wszystkie zasady, są wyjątki. Na przykład idealnie jest używać czarnej taśmy na migającym zegarze na magnetowidzie, jeśli nigdy nie zamierzasz nagrywać w określonym czasie (dla wszystkich starych ludzi, którzy pamiętają, czym jest magnetowid).
Michael Burr,
3
Jak każde odstępstwo od przyjętej praktyki, rób to w razie potrzeby i udokumentuj, aby następny facet wiedział, co zrobiłeś i dlaczego.
David Thornley,
5
@Jason: przynajmniej powinieneś zamieścić szczegółowy komentarz wyjaśniający, dlaczego uciszasz wyjątki.
Ned Batchelder,
1
Ta odpowiedź zakłada dwie rzeczy: 1. Zgłoszony wyjątek jest rzeczywiście znaczący i że ten, kto napisał kod, który go rzuca, wiedział, co robi; 2. Że wyjątek jest rzeczywiście „warunkiem błędu” i nie jest nadużywany jako przepływ kontrolny (chociaż jest to bardziej konkretny przypadek mojego pierwszego punktu).
Boris B.
38

Zasadniczo są złym pomysłem, ponieważ jest to naprawdę rzadki stan, w którym awaria (stan wyjątkowy, bardziej ogólnie) jest właściwie spotykana bez odpowiedzi BEZ. Ponadto puste catchbloki są powszechnym narzędziem używanym przez osoby korzystające z mechanizmu wyjątków do sprawdzania błędów, które powinny wykonywać zapobiegawczo.

Powiedzenie, że zawsze jest źle, jest nieprawdą ... to prawda o bardzo niewielu. Mogą zaistnieć okoliczności, w których albo nie przejmujesz się, że wystąpił błąd, albo jego obecność w jakiś sposób wskazuje, że i tak nie możesz nic z tym zrobić (na przykład podczas zapisywania poprzedniego błędu w pliku dziennika tekstowego i dostajesz IOException, co oznacza, że ​​i tak nie możesz zapisać nowego błędu).

Adam Robinson
źródło
11

Nie rozciągałbym się na tyle, by powiedzieć, że ten, kto używa pustych bloków catch, jest złym programistą i nie wie, co robi ...

W razie potrzeby używam pustych bloków catch. Czasami programista biblioteki, którą spożywam, nie wie, co robi i rzuca wyjątki nawet w sytuacjach, gdy nikt tego nie potrzebuje.

Rozważmy na przykład bibliotekę serwerów http. Nie przejmowałbym się tym, że serwer zgłasza wyjątek, ponieważ klient się rozłączył i index.htmlnie można go wysłać.

lubos hasko
źródło
6
Z pewnością jesteś nadmiernie uogólniony. To , że nie potrzebujesz, nie oznacza, że ​​nikt tego nie potrzebuje. Nawet jeśli absolutnie nic nie da się zrobić w odpowiedzi, jest ktoś, kto musi zbierać statystyki dotyczące porzuconych połączeń. Więc nieuprzejmie jest zakładać, że „programista biblioteki nie wie, co robi”.
Ben Voigt,
8

Istnieją rzadkie przypadki, w których można to uzasadnić. W Pythonie często widzisz tego rodzaju konstrukcję:

try:
    result = foo()
except ValueError:
    result = None

Więc może być OK (w zależności od aplikacji) wykonanie:

result = bar()
if result == None:
    try:
        result = foo()
    except ValueError:
        pass # Python pass is equivalent to { } in curly-brace languages
 # Now result == None if bar() returned None *and* foo() failed

W ostatnim projekcie .NET musiałem napisać kod, aby wyliczyć biblioteki DLL wtyczek, aby znaleźć klasy implementujące określony interfejs. Odpowiedni bit kodu (w VB.NET, przepraszam) to:

    For Each dllFile As String In dllFiles
        Try
            ' Try to load the DLL as a .NET Assembly
            Dim dll As Assembly = Assembly.LoadFile(dllFile)
            ' Loop through the classes in the DLL
            For Each cls As Type In dll.GetExportedTypes()
                ' Does this class implement the interface?
                If interfaceType.IsAssignableFrom(cls) Then

                    ' ... more code here ...

                End If
            Next
        Catch ex As Exception
            ' Unable to load the Assembly or enumerate types -- just ignore
        End Try
    Next

Chociaż nawet w tym przypadku przyznam, że zarejestrowanie gdzieś niepowodzenia byłoby prawdopodobnie poprawą.

Daniel Pryden
źródło
Odniosłem się do log4net jako jednego z tych przypadków, zanim zobaczyłem twoją odpowiedź.
Scott Lawrence
7

Puste bloki catch są zwykle umieszczane, ponieważ programista tak naprawdę nie wie, co robią. W mojej organizacji pusty blok catch musi zawierać komentarz wyjaśniający, dlaczego nic nie robić z wyjątkiem, jest dobrym pomysłem.

W pokrewnej notatce większość ludzi nie wie, że po bloku {} można zastosować catch {} lub wreszcie {}, wymagany jest tylko jeden.

Justin
źródło
1
+1 Podkreślę „lub” w ostatnim zdaniu, aby zwiększyć
siłę
Ten drugi akapit może być jednak zależny od języka, prawda?
David Z
3
chyba że mówisz o IE6 lub IE7 ;-) ... gdzie wymagany jest Haczyk lub w końcu się nie wykonuje. (zostało to naprawione w IE8 btw)
scunliffe
Bardzo prawdziwe. Może warto podkreślić języki. Wierzę, że .net jest w porządku
MPritchard
7

Wyjątki należy zgłaszać tylko wtedy, gdy naprawdę istnieje wyjątek - coś, co dzieje się poza normą. Pusty blok catch w zasadzie mówi „dzieje się coś złego, ale mnie to nie obchodzi”. To jest zły pomysł.

Jeśli nie chcesz obsłużyć wyjątku, pozwól mu propagować w górę, aż osiągnie kod, który może go obsłużyć. Jeśli nic nie jest w stanie obsłużyć wyjątku, powinno ono usunąć aplikację.

Reed Copsey
źródło
3
Czasami wiesz, że to nie wpłynie na nic. Następnie zrób to i skomentuj to, aby następny facet nie pomyślał, że spieprzyłeś go, ponieważ jesteś niekompetentny.
David Thornley,
Tak, jest czas i miejsce na wszystko. Ogólnie jednak uważam, że pusty blok catch, który jest dobry, stanowi wyjątek od reguły - jest to rzadki przypadek, w którym naprawdę chcesz zignorować wyjątek (zwykle IMO, jest to wynikiem złego użycia wyjątków).
Reed Copsey,
2
@David Thornley: Skąd wiesz, że nie wpłynie to na nic, jeśli przynajmniej nie sprawdzisz wyjątku w celu ustalenia, czy jest to oczekiwany i bezsensowny błąd, czy taki, który zamiast tego powinien być propagowany w górę?
Dave Sherohman,
2
@Dave: Chociaż catch (Exception) {}jest to zły pomysł, catch (SpecificExceptionType) {}może być w porządku. Programista DID sprawdza wyjątek, używając informacji o typie w klauzuli catch.
Ben Voigt,
7

Myślę, że jest w porządku, jeśli złapiesz określony typ wyjątku, o którym wiesz, że zostanie zgłoszony tylko z jednego konkretnego powodu, i oczekujesz tego wyjątku i naprawdę nie musisz nic z tym robić.

Ale nawet w takim przypadku komunikat debugowania może być w porządku.

balpha
źródło
6

Za Josh Bloch - Punkt 65: Nie ignoruj Wyjątki z Effective Java :

  1. Pusty blok catch pokonuje cel wyjątków
  2. Przynajmniej blok catch powinien zawierać komentarz wyjaśniający, dlaczego należy zignorować wyjątek.
KrishPrabakar
źródło
3

Pusty blok catch zasadniczo mówi: „Nie chcę wiedzieć, jakie błędy są zgłaszane, po prostu je zignoruję”.

Jest podobny do VB6 On Error Resume Next, z tym wyjątkiem, że wszystko w bloku try po zgłoszeniu wyjątku zostanie pominięte.

Co nie pomaga, gdy coś się następnie psuje.

Władca
źródło
Powiedziałbym, że „Po błędzie Wznów dalej” jest gorszy - mimo wszystko spróbuje przejść do następnego wiersza. Pusty haczyk przeskoczy, by przejść przez nawias zamykający instrukcji try
MPritchard,
Słuszna uwaga. Powinienem chyba to zauważyć w mojej odpowiedzi.
Powerlord,
1
Nie jest to do końca prawdą, jeśli masz pustą próbę ... złap z określonym typem wyjątku, wówczas zignoruje tylko ten typ wyjątku, i może to być uzasadnione ...
Meta-Knight
Ale jeśli wiesz, że zgłaszany jest określony typ wyjątku, wydaje się, że możesz mieć wystarczającą ilość informacji, aby napisać swoją logikę w sposób, który pozwoli uniknąć użycia metody try-catch do rozwiązania tej sytuacji.
Scott Lawrence
@Scott: Niektóre języki (np. Java) sprawdzają wyjątki ... wyjątki, dla których musisz pisać bloki catch.
Powerlord,
3

Jest to powiązane z „Nie używaj wyjątków do kontrolowania przebiegu programu” oraz „Używaj wyjątków tylko w wyjątkowych okolicznościach”. Jeśli tak się stanie, wyjątki powinny występować tylko wtedy, gdy występuje problem. A jeśli jest jakiś problem, nie chcesz zawieść po cichu. W rzadkich anomaliach, w których nie jest konieczne rozwiązanie problemu, należy przynajmniej zarejestrować wyjątek, na wypadek, gdyby anomalia przestała być anomalią. Jedyną gorszą rzeczą niż porażka jest cicha porażka.

Imagist
źródło
2

Myślę, że całkowicie pusty blok catch jest złym pomysłem, ponieważ nie można wnioskować, że ignorowanie wyjątku było zamierzonym zachowaniem kodu. Połknięcie wyjątku i zwrócenie wartości false, null lub innej wartości w niektórych przypadkach niekoniecznie jest złe. .NET ma wiele metod „wypróbowania”, które zachowują się w ten sposób. Zasadniczo, jeśli połkniesz wyjątek, dodaj komentarz i instrukcję dziennika, jeśli aplikacja obsługuje rejestrowanie.

komplekscipher
źródło
1

Ponieważ jeśli zostanie zgłoszony wyjątek , nigdy go nie zobaczysz - cicha porażka jest najgorszą możliwą opcją - dostaniesz błędne zachowanie i nie będziesz wiedział, co się dzieje. Przynajmniej umieść tam komunikat dziennika! Nawet jeśli jest to coś, co „nigdy się nie wydarzy”!

Nate
źródło
1

Puste bloki catch oznaczają, że programista nie wie, co zrobić z wyjątkiem. Pomijają wyjątek od ewentualnego bulgotania i są obsługiwane poprawnie przez inny blok try. Zawsze staraj się zrobić coś, z wyjątkiem tego, co łapiesz.

Dan
źródło
1

Moim zdaniem najbardziej denerwujące są puste instrukcje catch, kiedy zrobił to inny programista. Mam na myśli to, że kiedy trzeba debugować kod od kogoś innego, puste instrukcje catch utrudniają takie przedsięwzięcie, niż musi być. Instrukcje catch IMHO zawsze powinny wyświetlać jakiś komunikat o błędzie - nawet jeśli błąd nie zostanie obsłużony, powinien go przynajmniej wykryć (alt. Tylko w trybie debugowania)

AndersK
źródło
1

Prawdopodobnie nigdy nie jest to właściwe, ponieważ w ciszy omijasz każdy możliwy wyjątek. Jeśli spodziewasz się określonego wyjątku, powinieneś go przetestować, wrzucić, jeśli nie jest to wyjątek.

try
{
    // Do some processing.
}
catch (FileNotFound fnf)
{
    HandleFileNotFound(fnf);
}
catch (Exception e)
{
    if (!IsGenericButExpected(e))
        throw;
}

public bool IsGenericButExpected(Exception exception)
{
    var expected = false;
    if (exception.Message == "some expected message")
    {
        // Handle gracefully ... ie. log or something.
        expected = true;
    }

    return expected;
}
Xanadont
źródło
1
To nie jest tak naprawdę standardowy wzorzec, który doradzasz ludziom, aby go tam używał. Większość ludzi po prostu wychwytuje wyjątki tego, czego oczekują, a nie te, których nie są. Widzę pewne okoliczności, w których wasze podejście byłoby słuszne, po prostu uważajcie na posty na takim forum, na którym ludzie czytają takie rzeczy, ponieważ przede wszystkim nie rozumieją;)
MPritchard
Moim zamiarem nie było to, że IsKnownException () sprawdza typ wyjątku (tak, powinieneś to zrobić za pomocą różnych bloków catch), a raczej sprawdza, czy jest to oczekiwany wyjątek. Będę edytować, aby było bardziej jasne.
xanadont,
Początkowo myślałem o COMExceptions. Możesz przetestować określony kod błędu i obsłużyć oczekiwane kody. Robię to często podczas programowania za pomocą ArcOjbects.
xanadont,
2
-1 Podejmowanie decyzji w kodzie na podstawie komunikatu o wyjątku jest naprawdę złym pomysłem. Dokładna wiadomość jest rzadko udokumentowana i może ulec zmianie w dowolnym momencie; to szczegół implementacji. Lub komunikat może być oparty na lokalizowalnym ciągu zasobów. W każdym razie jest przeznaczony tylko do czytania przez człowieka.
Wim Coenen,
Eek. Wyjątek. Wiadomość może być zlokalizowana, a zatem nie jest to, czego oczekujesz. To jest po prostu złe.
frankhommers
1

Zasadniczo powinieneś wychwytywać tylko wyjątki, z którymi możesz sobie poradzić. Oznacza to, że przy wychwytywaniu wyjątków należy być jak najbardziej konkretnym. Łapanie wszystkich wyjątków rzadko jest dobrym pomysłem, a ignorowanie wszystkich wyjątków jest prawie zawsze bardzo złym pomysłem.

Mogę tylko wymyślić kilka przypadków, w których pusty blok catch ma jakiś sensowny cel. Jeśli jakikolwiek szczególny wyjątek, który łapiesz, zostanie „obsłużony” przez ponowną próbę wykonania akcji, nie będzie potrzeby nic robić w bloku catch. Jednak nadal dobrym pomysłem byłoby odnotowanie faktu, że wystąpił wyjątek.

Kolejny przykład: CLR 2.0 zmienił sposób traktowania nieobsługiwanych wyjątków w wątku finalizatora. Przed wersją 2.0 proces mógł przetrwać w tym scenariuszu. W bieżącym CLR proces zostaje zakończony w przypadku nieobsługiwanego wyjątku w wątku finalizatora.

Pamiętaj, że powinieneś wdrożyć finalizator tylko wtedy, gdy naprawdę go potrzebujesz, a nawet wtedy powinieneś zrobić jak najmniej w finalizatorze. Ale jeśli cokolwiek musi zrobić twój finalizator, może rzucić wyjątek, musisz wybierać między mniejszym a drugim złem. Czy chcesz zamknąć aplikację z powodu nieobsługiwanego wyjątku? A może chcesz kontynuować w mniej lub bardziej nieokreślonym stanie? Przynajmniej teoretycznie ten ostatni może być w niektórych przypadkach mniejszym złem. W takim przypadku pusty blok catch uniemożliwiłby zakończenie procesu.

Brian Rasmussen
źródło
1
Chodzi mi na przykład, że czasami chcesz skądś uzyskać dodatkowe informacje (usługa internetowa, baza danych) i naprawdę nie obchodzi Cię, czy dostaniesz te informacje, czy nie. Więc próbujesz to zdobyć, a jeśli coś się stanie, to dobrze, dodam tylko „catch (wyjątek zignorowany) {}” i to wszystko

Idąc za przykładem, jest to zły pomysł, ponieważ wychwytujesz i ignorujesz wszystkie wyjątki. Gdybyś tylko EInfoFromIrrelevantSourceNotAvailablełapał i ignorował to, byłoby dobrze, ale nie jesteś. Ignorujesz również ENetworkIsDown, co może, ale nie musi być ważne. Ignorujesz ENetworkCardHasMeltedi EFPUHasDecidedThatOnePlusOneIsSeventeen, które są prawie na pewno ważne.

Pusty blok catch nie stanowi problemu, jeśli skonfigurowano go tak, aby przechwytywał (i ignorował) wyjątki niektórych typów, o których wiadomo, że są nieistotne. Sytuacje, w których dobrym pomysłem jest tłumienie i dyskretne ignorowanie wszystkich wyjątków, bez przerywania ich sprawdzania w pierwszej kolejności, aby sprawdzić, czy są spodziewane / normalne / nieistotne czy nie, są niezwykle rzadkie.

Dave Sherohman
źródło
1

Są sytuacje, w których możesz ich użyć, ale powinny one być bardzo rzadkie. Sytuacje, w których mogę użyć jednego z nich, obejmują:

  • rejestrowanie wyjątków; w zależności od kontekstu możesz zamiast tego wysłać nieobsługiwany wyjątek lub wiadomość.

  • zapętlanie sytuacji technicznych, takich jak renderowanie lub przetwarzanie dźwięku lub wywołanie zwrotne listbox, gdzie samo zachowanie zademonstruje problem, zgłoszenie wyjątku po prostu przeszkodzi, a zarejestrowanie wyjątku prawdopodobnie spowoduje po prostu 1000 komunikatów „nie udało się XXX” .

  • programy, które nie mogą zawieść, chociaż nadal powinny przynajmniej coś rejestrować.

dla większości aplikacji WinForm stwierdziłem, że wystarczy mieć jedną instrukcję try dla każdego wejścia użytkownika. Korzystam z następujących metod: (AlertBox to tylko szybkie opakowanie programu MessageBox.Show)

  public static bool TryAction(Action pAction)
  {
     try { pAction(); return true; }
     catch (Exception exception)
     {
        LogException(exception);
        return false;
     }
  }

  public static bool TryActionQuietly(Action pAction)
  {
     try { pAction(); return true; }
     catch(Exception exception)
     {
        LogExceptionQuietly(exception);
        return false;
     }
  }

  public static void LogException(Exception pException)
  {
     try
     {
        AlertBox(pException, true);
        LogExceptionQuietly(pException);
     }
     catch { }
  }

  public static void LogExceptionQuietly(Exception pException)
  {
     try { Debug.WriteLine("Exception: {0}", pException.Message); } catch { }
  }

Następnie każdy moduł obsługi zdarzeń może zrobić coś takiego:

  private void mCloseToolStripMenuItem_Click(object pSender, EventArgs pEventArgs)
  {
     EditorDefines.TryAction(Dispose);
  }

lub

  private void MainForm_Paint(object pSender, PaintEventArgs pEventArgs)
  {
     EditorDefines.TryActionQuietly(() => Render(pEventArgs));
  }

Teoretycznie możesz mieć TryActionSilently, co może być lepsze do renderowania połączeń, aby wyjątek nie generował nieskończonej ilości wiadomości.

Dave Cousineau
źródło
1

Jeśli nie wiesz, co robić w bloku catch, możesz po prostu zarejestrować ten wyjątek, ale nie pozostawiać go pustego.

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }
Andzej Maciusovic
źródło
Dlaczego zostało to zanegowane?
Vino
0

Nigdy nie powinieneś mieć pustego bloku catch. To jak ukrywanie znanego błędu. Przynajmniej powinieneś zapisać wyjątek do pliku dziennika, aby przejrzeć go później, jeśli będziesz naciskał na czas.

Czadit
źródło