Почему пустые блоки catch — плохая идея?[закрыто]

StackOverflow https://stackoverflow.com/questions/1234343

  •  22-07-2019
  •  | 
  •  

Вопрос

Я только что видел вопрос по try-catch, какие люди (включая Джона Скита) считают, что пустые блоки захвата — это действительно плохая идея?Почему это?Нет ли ситуации, когда пустая защелка не является неправильным дизайнерским решением?

Я имею в виду, например, что иногда вы хотите получить откуда-то дополнительную информацию (веб-сервис, базу данных), и вам действительно все равно, получите вы эту информацию или нет.Итак, вы пытаетесь его получить, и если что-то произойдет, ничего страшного, я просто добавлю «catch (Exception ignore) {}» и все

Это было полезно?

Решение

Обычно пустой try-catch - плохая идея, потому что вы молча проглатываете состояние ошибки и затем продолжаете выполнение. Иногда это может быть правильным, но часто это признак того, что разработчик увидел исключение, не знал, что с этим делать, и поэтому использовал пустой улов, чтобы заглушить проблему.

Это программный эквивалент наложения черной ленты на сигнальную лампу двигателя.

Я считаю, что то, как вы справляетесь с исключениями, зависит от того, на каком уровне программного обеспечения вы работаете: Исключения в тропическом лесу .

Другие советы

Они - плохая идея в целом , потому что это действительно редкое условие, когда сбой (исключительное условие, в более общем смысле) должным образом встречается без какого-либо ответа. Кроме того, пустые блоки catch являются распространенным инструментом, используемым людьми, которые используют механизм исключений для проверки ошибок, что они должны делать упреждающе.

Сказать, что это всегда плохо, не соответствует действительности ... это правда очень мало. Могут быть обстоятельства, когда либо вас не волнует, что произошла ошибка, либо наличие ошибки как-то указывает на то, что вы ничего не можете с этим поделать (например, при записи предыдущей ошибки в текстовый файл журнала и вы получаете IOException , что означает, что вы все равно не можете выписать новую ошибку).

Я бы не стал растягиваться, если бы сказал, что тот, кто использует пустые блоки catch, плохой программист и не знает, что делает ...

Я использую пустые блоки catch при необходимости. Иногда программист библиотеки, которого я потребляю, не знает, что он делает, и выдает исключения даже в ситуациях, когда это никому не нужно.

Например, рассмотрим некоторую библиотеку http-сервера, мне было бы все равно, если сервер выдает исключение, потому что клиент отключился, и index.html не удалось отправить.

Есть редкие случаи, когда это может быть оправдано.В Python часто можно увидеть такую ​​конструкцию:

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

Так что может быть нормально (в зависимости от вашего приложения):

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

В недавнем проекте .NET мне пришлось написать код для перечисления DLL плагинов и поиска классов, реализующих определенный интерфейс.Соответствующий фрагмент кода (в VB.NET, извините):

    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

Хотя даже в этом случае я бы признал, что регистрация сбоя где-то, вероятно, была бы улучшением.

Обычно вставляются пустые блоки catch, потому что кодировщик на самом деле не знает, что они делают. В моей организации пустой блок catch должен содержать комментарий о том, почему не следует делать ничего за исключением.

В соответствующей заметке большинство людей не знают, что за блоком try {} может следовать либо catch {}, либо finally {}, требуется только один.

Исключения следует выдавать только в том случае, если есть действительно исключение - что-то происходящее за пределами нормы. Пустой блок catch в основном говорит «что-то плохое происходит, но мне все равно». Это плохая идея.

Если вы не хотите обрабатывать исключение, пусть оно распространяется вверх, пока не достигнет некоторого кода, который может его обработать. Если ничто не может обработать исключение, оно должно закрыть приложение.

Я думаю, что все в порядке, если вы поймаете определенный тип исключения, который, как вы знаете, будет вызван только по одной конкретной причине, и вы ожидаете, что это исключение и действительно не нужно ничего с этим делать.

Но даже в этом случае отладочное сообщение может быть в порядке.

За Джош Блох - . Пункт 65: Не игнорируйте исключения из Эффективная Java :

<Ол>
  • Пустой блок catch игнорирует цель исключений
  • По крайней мере, блок catch должен содержать комментарий, объясняющий, почему целесообразно игнорировать исключение.
  • Пустой блок catch по сути говорит: "Я не хочу знать, какие ошибки выдают, я просто буду их игнорировать".

    Это похоже на VB6 On Error Resume Next , за исключением того, что все, что находится в блоке try после исключения, будет пропущено.

    Что не помогает, когда что-то ломается.

    Это идет рука об руку с " не использовать исключения для управления потоком программ. " и " использовать исключения только для исключительных обстоятельств. " Если это сделано, исключения должны возникать только тогда, когда есть проблема. И если есть проблема, вы не хотите молча провалиться. В редких аномалиях, когда нет необходимости решать проблему, вы должны, по крайней мере, регистрировать исключение, на тот случай, если аномалия больше не станет аномалией. Единственное, что хуже, чем провал, это молчаливый провал.

    Я думаю, что абсолютно пустой блок catch - плохая идея, потому что нет никакого способа сделать вывод, что игнорирование исключения было предполагаемым поведением кода. В некоторых случаях не обязательно плохо проглатывать исключение и возвращать false или null или какое-либо другое значение. .NET Framework имеет много «попробовать» методы, которые ведут себя таким образом. Как правило, если вы проглотили исключение, добавьте комментарий и оператор журнала, если приложение поддерживает ведение журнала.

    Потому что, если выдается исключение , вы никогда его не увидите - молчаливый сбой - наихудший из возможных вариантов - вы получите ошибочное поведение и не поймете, где оно происходит. По крайней мере, поместите туда лог-сообщение! Даже если это то, что «никогда не случится»!

    Пустые блоки catch указывают на то, что программист не знает, что делать с исключением. Они подавляют исключение из-за возможного появления пузырей и их правильной обработки другим блоком try. Всегда пытайтесь что-то делать с исключением, которое вы ловите.

    Меня больше всего раздражают пустые операторы catch, когда это делает какой-то другой программист. Я имею в виду, что когда вам нужно отладить код у кого-то еще, любые пустые операторы catch делают такое начинание более трудным, чем это должно быть. IMHO операторы catch всегда должны показывать какое-то сообщение об ошибке - даже если ошибка не обрабатывается, она должна по крайней мере ее обнаружить (альтернативно, только в режиме отладки)

    Вероятно, это никогда не правильно, потому что вы молча пропускаете каждое возможное исключение. Если есть конкретное исключение, которое вы ожидаете, то вы должны проверить его, пересмотреть, если это не ваше исключение.

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

    Как правило, вы должны перехватывать только те исключения, которые действительно можете обработать. Это означает быть как можно более конкретным при отлове исключений. Поймать все исключения редко бывает хорошей идеей, а игнорировать все исключения - почти всегда очень плохой идеей.

    Я могу вспомнить лишь несколько случаев, когда пустой блок catch имеет какое-то значимое назначение. Если какое-то конкретное исключение, которое вы ловите, является " обработанным " просто попытавшись повторить действие, не нужно будет что-либо делать в блоке catch. Однако было бы неплохо зарегистрировать факт возникновения исключения.

    Другой пример: CLR 2.0 изменил способ обработки необработанных исключений в потоке финализатора. До версии 2.0 процессу было позволено пережить этот сценарий. В текущем CLR процесс завершается в случае необработанного исключения в потоке финализатора.

    Имейте в виду, что вы должны реализовывать финализатор только в том случае, если он вам действительно нужен, и даже в этом случае вы должны делать как можно меньше в финализаторе. Но если любая работа, которую должен выполнить ваш финализатор, может вызвать исключение, вам нужно выбрать между меньшим из двух зол. Вы хотите закрыть приложение из-за необработанного исключения? Или вы хотите перейти в более или менее неопределенное состояние? По крайней мере, в теории последнее может быть меньшим из двух зол в некоторых случаях. В этом случае пустой блок catch не позволит завершить процесс.

    Я имею в виду, например, что иногда вы хотите получить некоторую дополнительную информацию откуда-то (веб-сервис, база данных), и вам действительно все равно, получите вы эту информацию или нет. Таким образом, вы пытаетесь получить его, и если что-то случится, это нормально, я просто добавлю " catch (Exception игнорируется) {} " и это все

    Так что, следуя вашему примеру, в этом случае это плохая идея, потому что вы перехватываете и игнорируете все исключения. Если бы вы ловили только EInfoFromIrrelevantSourceNotAvailable и игнорировали его, это было бы хорошо, но это не так. Вы также игнорируете ENetworkIsDown , что может быть или не быть важным. Вы игнорируете ENetworkCardHasMelted и EFPUHasDecidedThatOnePlusOneIsSeventeen , которые почти наверняка важны.

    Пустой блок catch не является проблемой, если он настроен на перехват (и игнорирование) исключений определенных типов, которые, как вы знаете, не важны. Ситуации, в которых целесообразно подавлять и беззвучно игнорировать все исключения, не останавливаясь для их предварительного изучения, чтобы выяснить, ожидаемые они / нормальные / неактуальные или нет, чрезвычайно редки.

    Есть ситуации, когда вы можете их использовать, но они должны быть очень редкими.Ситуации, в которых я мог бы использовать его, включают:

    • регистрация исключений;в зависимости от контекста вам может потребоваться отправить необработанное исключение или сообщение.

    • зацикливание технических ситуаций, таких как рендеринг или обработка звука или обратный вызов списка, где само поведение будет демонстрировать проблему, выдача исключения будет только мешать, а регистрация исключения, вероятно, просто приведет к 1000 сообщениям «не удалось выполнить XXX» .

    • программы, которые не могу потерпят неудачу, хотя они все равно должны хотя бы что-то регистрировать.

    Я обнаружил, что для большинства приложений Winforms достаточно иметь один оператор try для каждого пользовательского ввода.Я использую следующие методы:(AlertBox — это просто быстрая оболочка 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 { }
      }
    

    Тогда каждый обработчик событий может сделать что-то вроде:

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

    или

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

    Теоретически вы могли бы использовать TryActionSilency, который может быть лучше для рендеринга вызовов, чтобы исключение не создавало бесконечное количество сообщений.

    У вас никогда не должно быть пустого блока catch. Это похоже на сокрытие ошибки, о которой вы знаете. По крайней мере, вы должны записать исключение в файл журнала для последующего просмотра, если вам не хватает времени.

    Если вы не знаете, что делать в блоке catch, вы можете просто зарегистрировать это исключение, но не оставляйте его пустым.

            try
            {
                string a = "125";
                int b = int.Parse(a);
            }
            catch (Exception ex)
            {
                Log.LogError(ex);
            }
    
    Лицензировано под: CC-BY-SA с атрибуция
    Не связан с StackOverflow
    scroll top