Dlaczego puste bloki to zły pomysł? [zamknięte]

Są złym pomysłem ogólnie ponieważ jest to naprawdę rzadki stan, w którym niepowodzenie (stan wyjątkowy, bardziej ogólnikowo) jest właściwie spełnione bez żadnej odpowiedzi. Co więcej, puste catch bloki są powszechnym narzędziem używanym przez ludzi, którzy używają silnika WYJĄTKÓW do sprawdzania błędów, które powinny być wykonywane prewencyjnie.

Nie rozciągałbym rzeczy tak daleko, aby powiedzieć, że kto używa pustych bloków catch jest złym programistą i nie wie, co robi...

W razie potrzeby używam pustych klocków. Czasami programista biblioteki, którą konsumuję, nie wie, co robi i wyrzuca wyjątki nawet w sytuacjach, gdy nikt tego nie potrzebuje.

Na przykład, rozważ jakąś bibliotekę serwerów http, nie obchodzi mnie, czy serwer wyrzuca wyjątek, ponieważ klient się rozłączył i index.html nie może być wysłane.

Istnieją rzadkie przypadki, w których można to uzasadnić. W Pythonie często spotyka się tego typu konstrukcje:

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

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

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 niedawnym projekcie. NET musiałem napisać kod, aby wyliczyć biblioteki DLL wtyczek, aby znaleźć klasy, które implementują określony interfejs. Odpowiedni bit kodu (w VB.NET, sorry) jest:

    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

Puste bloki catch są zwykle umieszczane, ponieważ koder nie wie, co robi. W mojej organizacji pusty blok połowu musi zawierać komentarz, dlaczego nie Robienie nic z wyjątkiem jest dobrym pomysłem.

W związku z tym, większość ludzi nie wie, że blok try{} może być śledzony z catch{} lub finally {}, tylko jeden jest wymagany.

Wyjątki powinny być rzucane tylko wtedy, gdy naprawdę istnieje wyjątek - coś, co dzieje się poza normą. Pusta blokada głosi: "dzieje się coś złego, ale mam to gdzieś". To zły pomysł.

Jeśli nie chcesz obsługiwać wyjątku, pozwól mu rozprzestrzeniać się w górę, aż osiągnie jakiś kod, który może go obsłużyć. Jeśli nic nie może obsłużyć wyjątku, powinien usunąć aplikację.

Myślę, że jest to w porządku, jeśli złapiesz konkretny typ wyjątku, o którym wiesz, że zostanie wychowany 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.

Per Josh Bloch - Pozycja 65: nie ignoruj WYJĄTKÓW z skuteczna 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ą wyrzucane, po prostu je zignoruję."

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

Idzie to w parze z: "nie używaj WYJĄTKÓW do kontrolowania przepływu programu.", oraz " stosować wyjątki tylko w wyjątkowych okolicznościach."Jeśli tak się stanie, wyjątki powinny występować tylko wtedy, gdy wystąpi problem. A jeśli jest problem, nie chcesz zawieść po cichu. W rzadkich anomaliach, gdzie nie jest konieczne rozwiązywanie problemu, powinieneś przynajmniej zarejestrować wyjątek, na wypadek, gdyby anomalia przestała być anomalią. Jedyną rzeczą gorszą od porażki jest porażka cicho.

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

Ponieważ jeśli wyjątek zostanie wyrzucony, nigdy go nie zobaczysz - zawodzenie po cichu jest najgorszą możliwą opcją - otrzymasz błędne zachowanie i nie będziesz wiedział, gdzie to się dzieje. Przynajmniej umieść tam wiadomość! Nawet jeśli jest to coś ,co "nigdy się nie wydarzy"!

Puste bloki catch oznaczają, że programista nie wie, co robić z wyjątkiem. Tłumią wyjątek od prawdopodobnie bulgotania i są obsługiwane poprawnie przez inny blok próbny. Zawsze staraj się coś zrobić, z wyjątkiem tego, że się łapiesz.

Najbardziej irytujące z pustymi poleceniami catch jest to, kiedy zrobił to jakiś inny programista. Chodzi mi o to, że gdy musisz debugować Kod od kogoś innego, puste deklaracje catch utrudniają takie przedsięwzięcie. IMHO instrukcje catch powinny zawsze pokazywać jakiś komunikat o błędzie - nawet jeśli błąd nie jest obsługiwany, powinien go przynajmniej wykryć (alt. tylko w trybie debugowania)

To chyba nigdy nie jest słuszne, ponieważ po cichu mijasz każdy możliwy wyjątek. Jeśli spodziewasz się konkretnego wyjątku, powinieneś go przetestować, jeśli nie jest to Twój 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;
}

Ogólnie rzecz biorąc, powinieneś łapać tylko te wyjątki, z którymi możesz sobie poradzić. Oznacza to, że przy wyłapywaniu wyjątków należy być jak najbardziej precyzyjnym. Łapanie wszystkich wyjątków rzadko jest dobrym pomysłem, a ignorowanie wszystkich wyjątków prawie zawsze jest bardzo złym pomysłem.

Myślę tylko o kilku przypadkach, w których pusta blokada ma jakiś sensowny cel. Jeśli jakikolwiek konkretny wyjątek, który wyłapujesz, jest "obsługiwany" po prostu reattempting akcji, nie będzie potrzeby robienia niczego w / align = "left" / Jednak nadal dobrym pomysłem byłoby zarejestrowanie faktu, że wystąpił wyjątek.

Inny przykład: CLR 2.0 zmienił sposób traktowania nieobsługiwanych wyjątków w wątku finalizera. Przed 2.0 proces mógł przetrwać ten scenariusz. W bieżącym CLR Proces jest zakończony w przypadku nieobsługiwanego wyjątku w wątku finalizera.

Pamiętaj, że powinieneś zaimplementować finalizer tylko wtedy, gdy naprawdę go potrzebujesz i nawet wtedy powinieneś zrobić jak najmniej w finalizatorze. Ale jeśli jakaś praca, którą musi wykonać Twój finalizator, może spowodować wyjątek, musisz wybrać pomiędzy mniejszym złem. Czy chcesz wyłączyć aplikację z powodu nieobsługiwanego wyjątku? A może chcesz postępować w mniej lub bardziej nieokreślonym stanie? Przynajmniej teoretycznie to ostatnie może być w niektórych przypadkach mniejszym złem. W takim przypadku pusty blok catch uniemożliwiłby zakończenie procesu.

Mam na myśli, na przykład, czasami chcesz uzyskać jakieś dodatkowe informacje skądś (webservice, baza danych) i naprawdę nie obchodzi cię, czy dostaniesz te informacje, czy nie. Więc spróbuj to dostać, a jeśli coś się stanie, to jest ok, dodam tylko " catch (Exception ignored) {}" i to wszystko

Więc idąc za swoim przykładem, to zły pomysł w takim przypadku, ponieważ wyłapujesz i ignorujesz wszystkie wyjątki. Gdybyś łapał tylko EInfoFromIrrelevantSourceNotAvailable i ignorował to byłoby w porządku, ale nie jesteś. Ignorujesz również ENetworkIsDown, co może być ważne lub nie. Ignorujesz ENetworkCardHasMelted i EFPUHasDecidedThatOnePlusOneIsSeventeen, które są prawie na pewno ważne.

Pusty blok catch nie jest problemem, jeśli jest skonfigurowany tak, aby wyłapywać (i ignorować) wyjątki pewnych typów, o których wiesz, że są nieistotne. Sytuacje, w których dobrym pomysłem jest tłumić i po cichu ignorować wszystkie wyjątki, bez zatrzymywania się, aby najpierw je zbadać, aby zobaczyć, czy są oczekiwane / normalne/nieistotne, czy nie, są niezwykle rzadkie.

Nigdy nie powinieneś mieć pustego bloku. To jak ukrywanie błędu, o którym wiesz. Przynajmniej powinieneś napisać wyjątek od pliku dziennika, aby przejrzeć go później, jeśli zostaniesz naciśnięty na czas.

Są sytuacje, w których możesz ich używać, ale powinny być one bardzo rzadkie. Sytuacje, w których mogę go użyć to:

• Rejestrowanie WYJĄTKÓW; w zależności od kontekstu możesz chcieć zamiast tego opublikować 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, rzucenie wyjątku po prostu wejdzie w drogę i zarejestrowanie wyjątku prawdopodobnie spowoduje to 1000 wiadomości "failed to XXX".

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

Dla większości aplikacji winforms, stwierdziłem, że wystarczy mieć jedną instrukcję try dla każdego wejścia użytkownika. Używam następujących metod: (AlertBox to tylko szybki MessageBox.Pokaż wrapper)

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

Wtedy każdy opiekun zdarzenia może coś zrobić like:

  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żna było spróbować, 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 catch block, możesz po prostu zalogować ten wyjątek, ale nie zostawiaj go pustego.

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }