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

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

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 .

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).

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ć.

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ą.

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.

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ę.

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.

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.

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.

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.

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.

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”!

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.

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)

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;
}

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.

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.

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.

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);
        }

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.