Подтвердить что ты не робот

Является ли это плохой практикой, чтобы поймать неспецифическое исключение, такое как System.Exception? Зачем?

В настоящее время я делаю обзор кода, и следующий код заставил меня прыгнуть. Я вижу несколько проблем с этим кодом. Согласен ли ты со мной? Если да, то как объяснить моему коллеге, что это неправильно (упрямый тип...)?

  • Поймать общее исключение (Exception ex)
  • Использование "if (ex-something)" вместо наличия другого блока catch
  • Мы едим SoapException, HttpException и WebException. Но если веб-служба не удалась, делать нечего.

код:

try
{
    // Call to a WebService
}
catch (Exception ex)
{
    if (ex is SoapException || ex is HttpException || ex is WebException)
    {
        // Log Error and eat it.
    }
    else
    {
        throw;
    }
}
4b9b3361

Ответ 1

Мантра:

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

Таким образом:

  • Вы не должны ловить общие исключения.

В вашем случае, да, вы должны просто поймать эти исключения и сделать что-то полезное (возможно, не просто съесть их - вы можете throw после того, как вы их запишите).

Ваш кодер использует throw (не throw ex), который хорош.

Вот как вы можете поймать несколько конкретных исключений:

try
{
    // Call to a WebService
}
catch (SoapException ex)
{
    // Log Error and eat it
}
catch (HttpException ex)
{
    // Log Error and eat it
}
catch (WebException ex)
{
    // Log Error and eat it
}

Это в значительной степени эквивалентно тому, что делает ваш код. Ваш разработчик, возможно, сделал это таким образом, чтобы избежать дублирования блоков "log error and eat it".

Ответ 2

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

Не полностью, см. ниже.

  • Поймать общее исключение (Exception ex)

В общем, вылавливать общее исключение на самом деле нормально, пока вы его ретронизуете (с throw;), когда приходите к выводу, что вы не можете его обработать. Код делает это, поэтому здесь нет непосредственной проблемы.

  • Использование "if (ex-something)" вместо наличия другого блока catch

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

Однако из эстетики и читаемости POV я бы предпочел несколько блоков catch для "if (ex is SoapException ||..)". После того, как вы реорганизуете общий код обработки в метод, несколько блоков catch только немного больше печатаются и их легче читать для большинства разработчиков. Кроме того, окончательный бросок легко упускается из виду.

  • Мы едим SoapException, HttpException и WebException. Но если веб-служба не удалась, делать нечего.

Здесь возможно скрывается самая большая проблема кода, но трудно дать совет, не зная больше о характере приложения. Если вызов веб-службы делает то, на что вы зависите позже, вероятно, неправильно регистрировать и есть исключения. Как правило, вы разрешаете исключение распространяться на вызывающего абонента (обычно после его переноса в, например, исключение XyzWebServiceDownException), возможно, даже после повторного вызова вызова webservice несколько раз (чтобы быть более надежным при возникновении ложных сетевых проблем).

Ответ 3

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

Если существует такая повторяющаяся логика, то вам действительно нужен фильтр исключений, поэтому вы только поймаете типы исключений, которые вас интересуют. У VB.NET есть эта функция, но, к сожалению, С# этого не делает. Гипотетический синтаксис может выглядеть так:

try
{
    // Call to a WebService
}
catch (Exception ex) if (ex is SoapException || ex is HttpException || /* etc. */)
{
    // Log Error and eat it
}

Как вы не можете этого сделать, вместо этого я предпочитаю использовать лямбда-выражение для общего кода (вы можете использовать delegate в С# 2.0), например

Action<Exception> logAndEat = ex => 
    {  
        // Log Error and eat it
    };

try
{
    // Call to a WebService
}
catch (SoapException ex)
{
    logAndEat(ex);
}
catch (HttpException ex)
{
    logAndEat(ex);
}
catch (WebException ex)
{
    logAndEat(ex);
}

Ответ 4

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

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

В этом случае возможный блок catch может выглядеть следующим образом:

try
{
   // User code called here
}
catch (Exception ex)
{
   if (ExceptionIsFatal(ex))
     throw;

   Log(ex);
}

Однако здесь есть три важных момента:

  • Это не что-то для каждой ситуации. В обзорах кода мы очень разборчивы в тех местах, где это действительно необходимо и, следовательно, разрешено.
  • Метод ExceptionIsFatal() гарантирует, что мы не едим исключения, которые никогда нельзя проглатывать (ExecutionEngineException, OutOfMemoryException, ThreadAbortException и т.д.).
  • То, что проглочено, тщательно регистрируется (журнал событий, log4net, YMMV).

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

Ответ 5

Я хотел бы добавить здесь, потому что обработка исключений почти во всех java/С# -кодах, которые я видел, просто некорректна. То есть вы в конечном итоге очень сложно отлаживать ошибку для игнорируемых исключений или, в равной степени, плохо, вы получаете неясное исключение, которое ничего не говорит вам, потому что слепое следование за "catching (Exception) плохо", и все просто хуже.

Во-первых, поймите, что исключение - это способ облегчить возврат информации об ошибках через уровни кода. Теперь ошибка 1: слой - это не просто кадр стека, а слой - это код, который имеет четко определенную ответственность. Если вы только что закодировали интерфейсы и импланты только потому, что вам лучше исправить ошибки.

Если слои хорошо разработаны и имеют определенные обязанности, тогда информация об ошибке имеет другое значение, поскольку она пузырится вверх. < -это ключ к тому, что делать, нет универсального правила.

Итак, это означает, что при возникновении исключения у вас есть 2 варианта, но вам нужно понять, где находится ваш уровень:

A) Если вы находитесь в середине слоя, и вы просто внутренняя, обычно частная, вспомогательная функция, и что-то плохое: DONT WORRY, пусть вызывающий получает исключение. Это совершенно нормально, потому что у вас нет бизнес-контекста и 1) Вы не игнорируете ошибку и 2) Вызывающий является частью вашего слоя и должен знать, что это может произойти, но теперь вы не можете использовать контекст ниже.

или...

B) Вы - верхняя граница слоя, фасад - к внутренним. Затем, если вы получите исключение, по умолчанию должно быть CATCH ALL и прекратить какие-либо исключения из перехода на верхний уровень, который не имеет смысла для вызывающего абонента, или, что еще хуже, вы можете изменить, и вызывающая сторона будет иметь зависимость от реализации детали и оба будут ломаться.

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

ПРАВИЛО: Все точки входа на слой должны быть защищены CATCH ALL и все ошибки, переведенные или обработанные. Теперь этот "handeled" происходит только в 1% случаев, в основном вам просто нужно (или может) вернуть ошибку в правильной абстракции.

Нет. Я уверен, что это очень сложно понять. real Пример →

У меня есть пакет, который запускает некоторые симуляции. Эти симуляции представлены в текстовых сценариях. есть пакет, который компилирует эти сценарии, и есть общий пакет utils, который просто читает текстовые файлы и, конечно же, базовый RTL. Зависимость UML →

Симулятор- > Компилятор- > utilsTextLoader- > Файл Java

1) Если что-то ломается в загрузчике utils внутри одного частного, и я получаю FileNotFound, Разрешения или что-то еще, ну просто пусть это пройдет. Больше ничего не поделаешь.

2) На границе, в первоначально утилите utilsTextLoader вы будете следовать приведенному выше правилу и CATCH_ALL. Компилятор не заботится о том, что произойдет, просто нужно ли загрузить файл или нет. Таким образом, в catch, re выкидывает новое исключение и переводит FileNotFound или что-то еще: "Не удалось прочитать файл XXXX".

3) Теперь компилятор знает, что источник не загружен. Это ВСЕ, что нужно знать. Поэтому, если позже я изменю utilsTestLoader для загрузки из сети, компилятор не изменится. Если вы отпустите FileNotFound и позже измените, вы ничего не скажете на компиляторе.

4) Цикл повторяется: фактическая функция, называемая нижним уровнем для файла, ничего не сделает после получения исключения. Таким образом, он позволяет ему расти.

5) Поскольку исключение попадает на уровень между симулятором и компилятором, компилятор снова CATCHES_ALL, скрывая любую деталь и просто выдает более конкретную ошибку: "Не удалось скомпилировать script XXX"

6) Наконец, повторите цикл еще раз, функция симулятора, называемая компилятором, просто отпустит.

7) Граница окончаний для пользователя. Пользователь является СЛОЕМ, и все применяется. У главного есть попытка catches_ALL и, наконец, просто создает приятное диалоговое окно или страницу и "выдает" переведенную ошибку пользователю.

Итак, пользователь видит.


Симулятор: Неустранимая ошибка не запускается симулятор

-Compiler: Не удалось скомпилировать script FOO1

- TextLoader: Не удалось прочитать файл foo1.scp

--- trl: FileNotFound


Сравнить с:

a) Компилятор: NullPointer Exception < -common case и потерянная ночь, отлаживающая имя файла typo

b) Loader: File not found < - Я упоминал, что загрузчик загружает сотни скриптов?

или

c) Ничего не происходит, потому что все проигнорировано!!!

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

Ну, мои 2cts. Эти простые правила много раз спасли мою жизнь...

-Ale

Ответ 6

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

Ответ 7

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

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

try
{
  // Do some work
}
catch (Exception ex)
{
  if (ex is SoapException)
  {
    // SoapException specific recovery actions
  }
  else if (ex is HttpException)
  {
    // HttpException specific recovery actions
  }
  else if (ex is WebException)
  {
    // WebException specific recoery actions
  }
  else
  {
    throw;
  }
}

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

Ответ 8

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