Это плохая практика, чтобы поймать неспецифическое исключение, такое как System.Exception? Зачем?
В настоящее время я делаю обзор кода, и следующий код заставил меня прыгнуть. Я вижу несколько проблем с этим кодом. Согласен ли ты со мной? Если да, то как мне объяснить моему коллеге, что это неправильно (упрямый тип...)?
- Поймать универсальное исключение (Exception ex)
- Использование "if (ex - что-то)" вместо использования другого блока 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;
}
}
8 ответов
Мантра это:
- Вы должны ловить исключения, только если вы можете правильно их обработать
Таким образом:
- Вы не должны ловить общие исключения.
В вашем случае, да, вы должны просто поймать эти исключения и сделать что-то полезное (возможно, не просто съесть их - вы могли бы 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
}
Это в значительной степени эквивалентно тому, что делает ваш код. Вероятно, ваш разработчик сделал это таким образом, чтобы избежать дублирования блоков "ошибка лога и съесть".
В настоящее время я делаю обзор кода, и следующий код заставил меня прыгнуть. Я вижу несколько проблем с этим кодом. Согласен ли ты со мной?
Не совсем, см. Ниже.
- Поймать универсальное исключение (Exception ex)
В целом, перехватывать общее исключение на самом деле нормально, если вы перебрасываете его (с помощью throw;), когда приходите к выводу, что вы не можете его обработать. Код делает это, поэтому здесь нет никаких проблем.
- Использование "if (ex - что-то)" вместо использования другого блока catch
Чистый эффект блока catch заключается в том, что только SoapException, HttpException и т. Д. Фактически обрабатываются, а все остальные исключения распространяются вверх по стеку вызовов. Я предполагаю, что с точки зрения функциональности это то, что должен делать код, так что здесь нет никаких проблем.
Тем не менее, из эстетики и читабельности POV я бы предпочел несколько блоков catch для "if (ex is SoapException || ..)". Как только вы реорганизуете общий код обработки в метод, несколько блоков catch становятся лишь немного более типичными и их легче читать большинству разработчиков. Кроме того, последний бросок легко пропускается.
- Мы едим SoapException, HttpException и WebException. Но если веб-службы не удалось, там не так много, чтобы сделать.
Здесь, возможно, таится самая большая проблема кода, но трудно дать совет, не зная больше о природе приложения. Если вызов веб-службы делает что-то, от чего вы зависите позже, вероятно, неправильно регистрировать и есть исключения. Как правило, вы позволяете исключению распространяться на вызывающую сторону (обычно после помещения его, например, в исключение XyzWebServiceDownException), возможно, даже после нескольких повторных попыток вызова веб-службы (чтобы быть более устойчивым при ложных проблемах в сети).
Проблема с перехватом и повторным вызовом одного и того же исключения заключается в том, что, хотя.NET делает все возможное, чтобы сохранить трассировку стека нетронутой, он всегда заканчивается модификацией, которая может усложнить отслеживание того, откуда на самом деле пришло исключение (например, исключение). номер строки, скорее всего, будет выглядеть как строка оператора re-throw, а не строка, в которой первоначально возникло исключение). Здесь есть много информации о разнице между уловом / повторным броском, фильтрацией и не отловом.
Когда есть подобная повторяющаяся логика, вам действительно нужен фильтр исключений, чтобы вы могли перехватывать только те типы исключений, которые вас интересуют. В VB.NET есть эта функциональность, но, к сожалению, в C# нет. Гипотетический синтаксис может выглядеть так:
try
{
// Call to a WebService
}
catch (Exception ex) if (ex is SoapException || ex is HttpException || /* etc. */)
{
// Log Error and eat it
}
Однако, поскольку вы не можете сделать это, вместо этого я использую лямбда-выражение для общего кода (вы можете использовать delegate в C# 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);
}
Я хотел бы добавить сюда, потому что обработка исключений почти во всех кодах java / C#, которые я видел, просто некорректна. Т.е. вы получаете очень сложную для отладки ошибку для игнорируемых исключений, или, что не менее плохо, вы получаете неясное исключение, которое ничего вам не говорит, потому что слепое следование "отлову (исключение) - это плохо", а все просто хуже.
Во-первых, следует понимать, что исключение - это способ облегчения возврата информации об ошибках между уровнями кода. Теперь ошибка 1: слой - это не просто стековый фрейм, а слой - это код с четко определенной ответственностью. Если вы просто закодировали интерфейсы и подразумеваете только потому, что у вас есть вещи, которые лучше исправить.
Если слои хорошо спроектированы и имеют определенные обязанности, то информация об ошибке имеет разное значение, поскольку она всплывает. Это ключ к тому, что делать, универсального правила не существует.
Таким образом, это означает, что когда возникает исключение, у вас есть 2 варианта, но вам нужно понять, где в слое вы находитесь:
A) Если вы находитесь в середине уровня, и вы просто внутренняя, обычно частная, вспомогательная функция, и что-то идет не так: DONT WORRY, пусть вызывающая сторона получит исключение. Это совершенно нормально, потому что у вас нет бизнес-контекста и 1) вы не игнорируете ошибку и 2) вызывающая сторона является частью вашего уровня и должна была знать, что это может произойти, но теперь вы можете не иметь контекст для обработки этого ниже.
или же...
Б) Вы верхняя граница слоя, фасад к внутренностям. Тогда, если вы получите исключение, по умолчанию должно быть CATCH ALL и остановка любых определенных исключений от перехода на верхний уровень, который не будет иметь смысла для вызывающей стороны, или, что еще хуже, вы можете измениться, и вызывающая сторона будет зависеть от реализации деталь и то и другое сломается.
Сила приложения - это уровень развязки между слоями. Здесь вы остановите все как общее правило и сбросите ошибку с общим исключением, переводя информацию в более значимую ошибку для верхнего уровня.
ПРАВИЛО: Все точки входа в слой должны быть защищены CATCH ALL, а все ошибки переведены или обработаны. Теперь это происходит только в 1% случаев, в основном вам просто нужно (или можно) вернуть ошибку в правильной абстракции.
Нет, я уверен, что это очень трудно понять. реальный пример ->
У меня есть пакет, который запускает некоторые симуляции. Эти симуляции в текстовых скриптах. есть пакет, который компилирует эти сценарии, и есть универсальный пакет utils, который просто читает текстовые файлы и, конечно, базовый java RTL. UML-зависимость is->
Симулятор-> Компилятор->utilsTextLoader-> Файл Java
1) Если что-то ломается в загрузчике утилит в одном приватном файле, и я получаю FileNotFound, Permissions или что-то еще, просто позвольте этому пройти. Больше ничего не поделаешь.
2) На границе, в первоначально вызванной функции utilsTextLoader вы будете следовать приведенному выше правилу и CATCH_ALL. Компилятору нет дела до того, что произойдет, просто сейчас нужно, загружен файл или нет. Таким образом, в подвохе повторно выведите новое исключение и переведите FileNotFound или что-либо еще в "Не удалось прочитать файл XXXX".
3) Компилятор теперь будет знать, что источник не был загружен. Это ВСЕ, что нужно знать. Поэтому, если я позже изменю utilsTestLoader для загрузки из сети, компилятор не изменится. Если вы отпустите FileNotFound и позже измените его, вы окажете влияние на компилятор даром.
4) Цикл повторяется: фактическая функция, которая вызвала нижний уровень для файла, ничего не сделает при получении исключения. Так что это позволяет ему подняться.
5) Как исключение попадает в слой между симулятором и компилятором, компилятор снова CATCHES_ALL, скрывая любые детали и просто выдает более конкретную ошибку: "Не удалось скомпилировать скрипт XXX"
6) Наконец, повторите цикл еще раз, функция симулятора, которая вызывает компилятор, просто отпускает.
7) Окончательная граница для пользователя. Пользователь это СЛОЙ и все относится. У основного есть попытка, которая делает catches_ALL и, наконец, просто создает красивое диалоговое окно или страницу и "выдает" переведенную ошибку пользователю.
Так видит пользователь.
Симулятор: фатальная ошибка не может запустить симулятор
-Compiler: не удалось скомпилировать скрипт FOO1
--TextLoader: не удалось прочитать файл foo1.scp
--- trl: FileNotFound
Сравнить с:
a) Компилятор: исключение NullPointer <- общий случай и потерянная ночь отладки опечатки имени файла
б) Загрузчик: файл не найден <- Я упоминал, что загрузчик загружает сотни скриптов??
или же
в) ничего не происходит, потому что все было проигнорировано!!!
Конечно, это предполагает, что при каждом перебрасывании вы не забыли установить причину исключения.
Ну, мои 2ct. Эти простые правила спасли мне жизнь много раз...
-Ale
Иногда это единственный способ справиться со сценариями "поймать каждое исключение", фактически не перехватывая каждое исключение.
Я думаю, что иногда, скажем, низкоуровневый фреймворк / код времени выполнения должен убедиться, что ни одно исключение никогда не ускользнет. К сожалению, также нет способа, которым код фреймворка может знать, какие исключения вызываются кодом, выполняемым потоком.
В этом случае возможный блок catch может выглядеть так:
try
{
// User code called here
}
catch (Exception ex)
{
if (ExceptionIsFatal(ex))
throw;
Log(ex);
}
Здесь есть три важных момента:
- Это не что-то для каждой ситуации. В обзорах кода мы очень требовательны к местам, где это на самом деле необходимо и, следовательно, разрешено.
- Метод ExceptionIsFatal() гарантирует, что мы не будем использовать исключения, которые никогда не следует проглатывать (ExecutionEngineException, OutOfMemoryException, ThreadAbortException и т. Д.)
- То, что проглочено, тщательно регистрируется (журнал событий, log4net, YMMV)
Как правило, я за то, чтобы позволить неперехваченным исключениям просто "сбить" приложение, завершив работу CLR. Однако, особенно в серверных приложениях, это иногда не имеет смысла. Если один поток сталкивается с проблемой, которая считается нефатальной, нет причин прерывать весь процесс, убивая все остальные выполняющиеся запросы (например, WCF обрабатывает некоторые случаи таким образом).
Обычно вы все равно должны ловить универсальные исключения в глобальном обработчике (который является идеальным местом для регистрации неожиданных исключений), но в противном случае, как было сказано ранее, вы должны ловить только определенные типы исключений в других местах, если вы планируете что-то с ними делать. Блоки catch должны явно искать эти типы исключений, а не код, который вы опубликовали.
Принцип только для того, чтобы поймать исключение, которое вы можете обработать например, если вы знаете, как обращаться с findnotfound, вы ловите исключение файла notfoundexception, в противном случае НЕ перехватываете его и позволяете выбросить на верхний уровень.
Я не думаю, что это так уж плохо в этом случае, но я также делаю что-то похожее в моем коде с исключениями, которые могут быть безопасно проигнорированы, когда их перехватят, а остальные перезапустят. Как отмечается в ответе Майкла, наличие каждого улова в виде отдельного блока может вызвать некоторые проблемы с читабельностью, которые можно избежать, следуя этому маршруту.
Что касается указания на это вашему коллеге, я думаю, вам будет трудно убедить их в том, что это неправильный способ ведения дел - даже в большей степени, если они упрямы - из-за потенциальных проблем с читаемостью, если вы будете делать что-то иначе., Поскольку эта версия по-прежнему вызывает общие исключения, которые не могут быть обработаны, она соответствует духу практики. Однако если код соответствует следующему:
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;
}
}
Тогда, я думаю, у вас будет немного больше поводов для беспокойства, поскольку нет смысла выполнять работу для конкретного исключения, проверяя его в блоке общего исключения.