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

194

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

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

Самуэль Каррихо
источник
53
Я бы написал комментарий, объясняющий, почему он должен быть пустым.
Мердад Афшари
5
... или хотя бы лог, чтобы его поймали.
Мэтт б
2
@ocdecio: избегайте его для кода очистки , не избегайте его вообще.
Джон Сондерс
1
@ocdecio: Другой случай, чтобы избежать использования try..catch, - это когда вы ловите исключения для известных типов сбоев. например, исключения числового преобразования
MPritchard
1
@ocdecio - try..finally лучше, чем try..empty catch, потому что ошибка продолжается вверх по стеку - чтобы обработать ее либо фреймворком, либо уничтожить программу и отобразить ее пользователю - оба результата лучше, чем " тихая неудача ".
Nate

Ответы:

298

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

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

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

Нед Бэтчелдер
источник
4
В тех действительно редких обстоятельствах, когда мне действительно не нужно исключение, и регистрация по какой-то причине недоступна, я обязательно комментирую, что это преднамеренно - и почему это не нужно, и повторяю, что я все еще предпочел бы регистрировать это, если это было возможно в этой среде. По сути, я делаю комментарий БОЛЬШЕ работы, чем ведение журнала, если бы в таких обстоятельствах это было возможно. К счастью, у меня есть только 2 клиента, для которых это проблема, и это только при отправке некритических электронных писем с их веб-сайтов.
Джон Руди
37
Также люблю аналогии - и, как и все правила, есть исключения. Например, вполне нормально использовать черную ленту над мигающими часами на видеомагнитофоне, если вы никогда не собираетесь делать записи по времени (для всех вас, стариков, которые помнят, что такое видеомагнитофон).
Майкл Берр
3
Как и любое отклонение от принятой практики, сделайте это, если это необходимо, и запишите это, чтобы следующий парень знал, что вы сделали и почему.
Дэвид Торнли
5
@Jason: по крайней мере, вы должны включить подробный комментарий, объясняющий, почему вы отключаете исключения.
Нед Бэтчелдер
1
Этот ответ предполагает две вещи: 1. То, что выбрасываемое исключение действительно является значимым, и тот, кто написал вызывающий код, знал, что он делал; 2. То, что исключение действительно является «условием ошибки» и не используется как поток управления (хотя это более конкретный случай моей первой точки).
Борис Б.
38

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

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

Адам Робинсон
источник
11

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

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

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

Lubos Hasko
источник
6
Вы, конечно, чрезмерно обобщаете. То, что вам это не нужно, еще не значит, что никто этого не делает. Даже если в ответ ничего не поделаешь, где-то есть кто-то с требованием собирать статистику по потерянным соединениям. Поэтому довольно грубо полагать, что «программист библиотеки не знает, что делает».
Бен Фойгт
8

Есть редкие случаи, когда это может быть оправдано. В 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

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

Даниэль Приден
источник
Я упомянул log4net как один из тех примеров, прежде чем увидел ваш ответ.
Скотт Лоуренс
7

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

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

Джастин
источник
1
+1 Я бы подчеркнул «или» в этом последнем предложении для дополнительного воздействия
MPritchard
Этот второй абзац может зависеть от языка, правда?
Дэвид З
3
если вы , конечно , не говорим о IE6 или IE7 ;-) ... где улов IS требуется или , наконец , не выполняет. (это было исправлено в IE8)
scunliffe
источник: webbugtrack.blogspot.com/2007/11/…
scunliffe
Очень верно. Возможно, стоит выделить языки. Я считаю, что .net в порядке
MPritchard
7

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

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

Рид Копси
источник
3
Иногда вы знаете, что это ни на что не повлияет. Затем сделайте это и прокомментируйте это, чтобы следующий парень не просто подумал, что вы облажались, потому что вы некомпетентны.
Дэвид Торнли
Да, есть время и место для всего. В целом, однако, я думаю, что пустой блок catch, являющийся хорошим, является исключением из правила - это редкий случай, когда вы действительно хотите игнорировать исключение (обычно IMO, это результат плохого использования исключений).
Рид Копси
2
@ Дэвид Торнли: Откуда вы знаете, что это ни на что не повлияет, если вы по крайней мере не осмотрите исключение, чтобы определить, является ли это ожидаемой и бессмысленной ошибкой или ошибкой, которая должна распространяться вверх?
Дейв Шерохман
2
@Dave: Хотя catch (Exception) {}это плохая идея, catch (SpecificExceptionType) {}может быть, все в порядке. Программист DID проверяет исключение, используя информацию о типе в предложении catch.
Бен Фойгт
7

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

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

balpha
источник
6

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

  1. Пустой блок catch уничтожает цель исключений
  2. По крайней мере, блок catch должен содержать комментарий, объясняющий, почему целесообразно игнорировать исключение.
KrishPrabakar
источник
3

Пустой блок catch по сути говорит: «Я не хочу знать, какие ошибки выдаются, я просто буду их игнорировать».

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

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

Powerlord
источник
Я бы на самом деле сказал: «При ошибке продолжить далее» - это хуже - он просто попробует следующую строку, независимо от этого. Пустой улов будет прыгать, чтобы передать закрывающую скобку оператора try
MPritchard
Хорошая точка зрения. Наверное, я должен отметить это в своем ответе.
Powerlord
1
Это не совсем верно, если у вас есть пустой try ... catch с определенным типом исключения, то он будет игнорировать только этот тип исключения, и это может быть допустимо ...
Meta-Knight
Но если вы знаете, что генерируется исключение определенного типа, похоже, что у вас может быть достаточно информации, чтобы написать свою логику таким образом, чтобы избежать использования try-catch для разрешения ситуации.
Скотт Лоуренс
@Scott: Некоторые языки (например, Java) проверяют исключения ... исключения, для которых вы вынуждены писать блоки catch.
Powerlord
3

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

имажинистов
источник
2

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

complexcipher
источник
1

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

Nate
источник
1

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

Дэн
источник
1

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

AndersK
источник
1

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

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
источник
1
Это не совсем стандартный шаблон, который вы советуете людям использовать там. Большинство людей просто ловят исключения тех типов, которые они ожидают, а не те, которыми они не являются. Я вижу некоторые обстоятельства, в которых ваш подход был бы верным, просто будьте осторожны при размещении сообщений на подобных форумах, где люди, как правило, читают подобные вещи, потому что они не понимают с самого начала;)
MPritchard
Я не хотел, чтобы IsKnownException () проверял тип исключения (да, вы должны делать это с помощью различных блоков catch), а скорее проверял, является ли это ожидаемым исключением. Я отредактирую, чтобы сделать это более ясным.
xanadont
Я изначально думал о COMException. Вы можете проверить определенный код ошибки и обработать ожидаемые коды. Я делаю это часто при программировании с помощью ArcOjbects.
xanadont
2
-1 Принятие решений в коде на основе сообщения об исключении - очень плохая идея. Точное сообщение редко документируется и может измениться в любое время; это деталь реализации. Или сообщение может быть основано на локализуемой строке ресурса. В любом случае, он предназначен только для чтения человеком.
Вим Коенен
Ик. Исключение. Сообщение может быть локализовано и, следовательно, не то, что вы ожидаете. Это просто неправильно.
франхоммеры
1

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

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

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

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

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

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

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

Дейв Шерохман
источник
1

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

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

  • зацикливание технических ситуаций, таких как рендеринг или обработка звука, или обратный вызов списка, когда само поведение будет демонстрировать проблему, выбрасывание исключения будет только мешать, а регистрация исключения, вероятно, просто приведет к тысячам сообщений «fail to 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));
  }

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

Дейв Кузино
источник
1

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

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }
Анджей Мациусович
источник
Почему это было отвергнуто?
Вино
0

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

Chadit
источник