Писать ли в этом случае catch(Exception)?
От: 0K Ниоткуда  
Дата: 11.08.10 21:01
Оценка:
Пользователь указывает html (пусть в виде строки). Нужно проверить на корректность. Специального метода нет, но есть библиотека которая его парсит. Какой вариант проверки правильный:

Вариант 1


if (string.IsNullOrEmpty(html)
{
   Console.WriteLine("Поле HTML не может быть пустым");
   return;
}

HtmlDocument doc = new HtmlDocument();

try
{
    doc.LoadHtml(html);
}
catch (ArgumentException)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}
catch (FormatException)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}
catch (IndexOutOfRangeException)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}
catch (InvalidOperationException)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}
catch (Exception) // Нужно ли это писать? Ведь не рекомендуют писать общие исключения. Но хрен знает как работает эта библиотека и какие исключения она выбросит. А мы ведь в таком случае ничего не теряем...
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}


Вариант 2

HtmlDocument doc = new HtmlDocument();

try
{
    doc.LoadHtml(html);
}
catch (Exception)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}
Re: Писать ли в этом случае catch(Exception)?
От: mrjeka Россия  
Дата: 11.08.10 21:31
Оценка: +3
Здравствуйте, 0K, Вы писали:

0K>Пользователь указывает html (пусть в виде строки). Нужно проверить на корректность. Специального метода нет, но есть библиотека которая его парсит. Какой вариант проверки правильный:


0K>Вариант 1



0K>
0K>if (string.IsNullOrEmpty(html)
0K>{
0K>   Console.WriteLine("Поле HTML не может быть пустым");
0K>   return;
0K>}

0K>HtmlDocument doc = new HtmlDocument();

0K>try
0K>{
0K>    doc.LoadHtml(html);
0K>}
0K>catch (ArgumentException)
0K>{
0K>    Console.WriteLine("Не удалось распарсить HTML");
0K>    return;
0K>}
0K>catch (FormatException)
0K>{
0K>    Console.WriteLine("Не удалось распарсить HTML");
0K>    return;
0K>}
0K>catch (IndexOutOfRangeException)
0K>{
0K>    Console.WriteLine("Не удалось распарсить HTML");
0K>    return;
0K>}
0K>catch (InvalidOperationException)
0K>{
0K>    Console.WriteLine("Не удалось распарсить HTML");
0K>    return;
0K>}
0K>catch (Exception) // Нужно ли это писать? Ведь не рекомендуют писать общие исключения. Но хрен знает как работает эта библиотека и какие исключения она выбросит. А мы ведь в таком случае ничего не теряем...
0K>{
0K>    Console.WriteLine("Не удалось распарсить HTML");
0K>    return;
0K>}
0K>


0K>Вариант 2


0K>
0K>HtmlDocument doc = new HtmlDocument();

0K>try
0K>{
0K>    doc.LoadHtml(html);
0K>}
0K>catch (Exception)
0K>{
0K>    Console.WriteLine("Не удалось распарсить HTML");
0K>    return;
0K>}
0K>



В первом варианте нет смысла, т.к. он выдает одно и тоже сообщение. Такую обработку как в первом варианте имеет смысл писать, если на каждый тип исключения будет своя обработка. Если такого не предполагается, то вполне корректен второй вариант.
Re[2]: Писать ли в этом случае catch(Exception)?
От: 0K Ниоткуда  
Дата: 11.08.10 21:38
Оценка: :)))
Здравствуйте, mrjeka, Вы писали:

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


А вы хоть знаете что это не по стандарту? Вас любой FxCop остановит и заставить написать правильно. И MSDN откройте, почитайте. Общие исключения перехватывать нельзя.

Но чем это грозит в данном конкретном случае? Вот если не писать общие -- очень чревато: программа упадет. А если написать? Ведь все будет нормально. Даже если метод LoadHtml и выдаст экзотическое исключение -- что мы теряем? Не мы же его писали и не нам его править? Если не будет корректно работать -- применим другую библиотеку.

Ведь все сторонние библиотеки могут выдавать экзотические исключения (неадекватные). Мир мы не перестроим -- не будем переписывать все библиотеки чтобы они работали по стандарту. Единственный вариант -- перестроить стандарты: ОФИЦИАЛЬНО разрешить в таких случаях перехватывать все исключения.
Re[3]: Писать ли в этом случае catch(Exception)?
От: mrjeka Россия  
Дата: 11.08.10 22:09
Оценка:
Здравствуйте, 0K, Вы писали:

0K>А вы хоть знаете что это не по стандарту? Вас любой FxCop остановит и заставить написать правильно. И MSDN откройте, почитайте. Общие исключения перехватывать нельзя.


Не по стандарту не означает же что запрещено. Смоделируем ситуацию: программа должна подавать звуковой сигнал вне зависимости от типа исключения. Имеет ли смысл писать кучу кэтчей в этой ситуации?
Re: Писать ли в этом случае catch(Exception)?
От: MozgC США http://nightcoder.livejournal.com
Дата: 11.08.10 22:16
Оценка: 63 (5) -1
Вариант 1 содержит много дублирования, что неудивительно, т.к. в большинстве случаев обработки исключения код, который мы пишем, относительно стандартный, независимо от реальной причины исключения. Мы должны восстановить состояние каким оно было до возникновения исключения, залогировать ошибку и убедиться, что мы можем продолжать работу. В варианте 1 мы видим, что для различных исключений проделываются практически одинаковые действия в блоках catch. В таком случае мы можем упростить код, обернув API сторонней библиотеки и генериуя единый тип исключения:

Вариант 3

class HtmlDocumentWrapper
{
  private HtmlDocument _htmlDocument = new HtmlDocument();

  public void LoadHtml(SomeType html)
  {
    try
    {
      _htmlDocument.LoadHtml(html);
    }
    catch(ArgumentException ex)
    {
      throw new HtmlDocumentException(ex.Message, ex);
    }
    catch(FormatException ex)
    {
      throw new HtmlDocumentException(ex.Message, ex);
    }
    catch(IndexOutOfRangeException ex)
    {
      throw new HtmlDocumentException(ex.Message, ex);
    }
    catch(InvalidOperationException ex)
    {
      throw new HtmlDocumentException(ex.Message, ex);
    }
  }
}

...

HtmlDocumentWrapper doc = new HtmlDocumentWrapper();

try
{
    doc.LoadHtml(html);
}
catch (HtmlDocumentException ex)
{
  // try to recover state, then log exception and show message to user or rethrow exception
  ...
}

Такие классы-обертки могут быть очень полезными. И вообще, оборачивание сторонних API является очень хороший практикой. Когда мы оборачиваем API сторонней библиотеки, мы минимизируем зависимость от нее. В будущем мы сможем переключиться на другую библиотеку, избежав больших переделок (придется изменить только код класса-обертки, а код в местах использования класса-обертки не изменится.
Еще одно преимущество оборачивания состоит в том, что мы не будем привязаны к дизайну API конкретной библиотеки. Мы можем создать свой, более удобный API, как, например, в варианте 3, мы создали единый тип исключения, и получилось, что вызывающий код стал намного проще и чище.

Ловить System.Exception все же не стоит, поверьте. Вместо этого нужно позволить исключение пройти выше по стеку, и на верхнем уровне должен быть обработчик (это может быть обработчик Application.ThreadException или AppDomain.UnhandledException) по крайней мере залогирует исключение. Завершать программу при этом может быть не обязательно. Если вы считаете, что можно продолжать работу, то пусть обработчик на верхнем уровне просто залогирует исключение и покажет сообщение пользователю. Так что если возникнет какое-то непредусмотренное исключение — вы увидите его в логе и потом решите как лучше поступить.

Вы не совсем верно понимаете правило, что не нужно ловить System.Exception. На самом деле оно звучит так: Не нужно ловить System.Exception кроме как в обработчиках верхнего уровня (удостовериться можно здесь и во Framework Design Guidelines на стр. 234).
Re: Писать ли в этом случае catch(Exception)?
От: midcyber
Дата: 11.08.10 23:09
Оценка:
Здравствуйте, 0K, Вы писали:

Грубо говоря, catch(Exception) писать не нужно, и даже вредно (в случае Java то же самое).

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

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

Я так понимаю, ты все это подводишь под тему checked exceptions в Java.
Так вот, мое мнение по этому вопросу
1) API в Java довольно грязное, все же исключения должны использоваться как сигнал об исключительной ситуации, а не обработки штатных вещей.
2) Checked exceptions не всегда несут полезную нагрузку, и точно также склонны забиваться заглушками. Ну вот например, ноль пользы они приводят в таком коде:

void playSound(int index)
{
  currentSound.stop();

  try
  {
    Thread.sleep(15);
  }
  catch(InterruptedException) 
  { 
  }

  currentSound = loadSound(index);

  try
  {
    Thread.sleep(5);
  }
  catch(InterruptedException) 
  { 
  }

  currentSound.play();
}
в п
Re[2]: Писать ли в этом случае catch(Exception)?
От: 0K Ниоткуда  
Дата: 11.08.10 23:42
Оценка:
Здравствуйте, midcyber, Вы писали:

M>Да, внешне будут "рюшечки" — никогда не падающая программа.

M>Возможно, показывающая пользователю пустое окно вместо данных. Возможно, некорректные данные, что еще хуже.

А как в данном случае тип исключения может повлиять на появление пустого окна или некорректные данные? Вы видите хоть один вариант.
Re[3]: Писать ли в этом случае catch(Exception)?
От: samius Япония http://sams-tricks.blogspot.com
Дата: 12.08.10 02:42
Оценка:
Здравствуйте, 0K, Вы писали:

0K>А вы хоть знаете что это не по стандарту? Вас любой FxCop остановит и заставить написать правильно. И MSDN откройте, почитайте. Общие исключения перехватывать нельзя.


Обидно что тот же FxCop не остановил авторов обсуждаемой библиотеки HtmlAgilityPack от подобных трюков.

0K>Но чем это грозит в данном конкретном случае? Вот если не писать общие -- очень чревато: программа упадет. А если написать? Ведь все будет нормально. Даже если метод LoadHtml и выдаст экзотическое исключение -- что мы теряем? Не мы же его писали и не нам его править? Если не будет корректно работать -- применим другую библиотеку.


Загвоздка в том, что HtmlAgilityPack одна из самых всеядных библиотек, которая почти ничего не кидает сама. Т.е. она там что-то парсит, прокидывает только то что получила от низлежащего IO, в качестве результата выдает то что смогла распарсить. Ну а не смогла — все равно что-нибудь да отдаст. Требований к исходному HTML почти никаких нет.

0K>Ведь все сторонние библиотеки могут выдавать экзотические исключения (неадекватные). Мир мы не перестроим -- не будем переписывать все библиотеки чтобы они работали по стандарту. Единственный вариант -- перестроить стандарты: ОФИЦИАЛЬНО разрешить в таких случаях перехватывать все исключения.

Вопрос в том, где и с какой целью.
Re[2]: Писать ли в этом случае catch(Exception)?
От: samius Япония http://sams-tricks.blogspot.com
Дата: 12.08.10 03:16
Оценка: 17 (2) +1
Здравствуйте, MozgC, Вы писали:

MC>Такие классы-обертки могут быть очень полезными. И вообще, оборачивание сторонних API является очень хороший практикой. Когда мы оборачиваем API сторонней библиотеки, мы минимизируем зависимость от нее. В будущем мы сможем переключиться на другую библиотеку, избежав больших переделок (придется изменить только код класса-обертки, а код в местах использования класса-обертки не изменится.

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

Это очень хорошая теория. К сожалению, часто она далека от практики. Возьмем те же библиотеки для разбора Html... Чтобы обернуть API одной из них чтобы абстрагироваться от этой библиотеки, понадобится значительное время. Для того чтобы сделать универсальный API с целью подменять одну библиотеку на другую — потребуется значительно больше времени, если это вообще будет возможно. Если подразумевать 3 и более библиотек — то это будет отдельный проект на несколько человеколет, если оборачивать качественно. Скажем, абстрагировать API HtmlAgilityPack до той степени что бы можно было легко пересесть на Linq2Xml — unreal.

Потому для абстрагирования от библиотеки сгодится немного другой подход, а именно — вынести работу со спецбиблиотекой в стратегии. Допустим, нам от HtmlAgilityPack нужно лишь немногое — выдрать все значения <img href=???>. На входе HTML, на выходе список строк. Для абстрагирования будет достаточно создать интерфейс с методом GetImgHrefs и реализовать его в классе, работающим с HtmlAgilityPack. Там же мы можем обработать исключения как хотим.
Если рассматривать задачи сложнее, например когда требуется дерево представления Html, то лучше не использовать сторонние деревья, а сделать свое, и использовать сторонние библиотеки лишь для построения своего дерева, т.е. опять таки стратегий, чтобы сосредоточить код использования сторонней библиотеки в одном-нескольких классах, а не размазывать его по всей программе. Когда придется сменить библиотеку, можно будет подменить стратегию, и реализовать требуемые вещи заново на другой библиотеке.

Но это опять теория. Тот же HtmlAgilityPack умеет достаточно много, чтобы повторять его функциональность в своих структурах данных. Например, он поддерживает XPath, позволяет модифицировать дерево, легко выдает обновленный Html и т.п. Построение своих структур данных со схожей функциональностью — расточительность. Это будет все равно что писать код, абстрагирующийся от конкретного ORM, что бы когда-нибудь безболезненно пересесть на другой, заранее не известный.

Лишь хотел поправить, что в каждом конкретном случае следует рассматривать и другие варианты, кроме оборачивания API сторонней библиотеки.
Re[2]: Писать ли в этом случае catch(Exception)?
От: samius Япония http://sams-tricks.blogspot.com
Дата: 12.08.10 03:25
Оценка:
Здравствуйте, midcyber, Вы писали:

M>В первом варианте, ты обрабатываешь предоставленные "официально" API исключения.


Это самодеятельность. Официально там ничего не предоставлено. Ни FormatException, ни IndexOutOfRange, вообще ничего...
Re[3]: Писать ли в этом случае catch(Exception)?
От: 0K Ниоткуда  
Дата: 12.08.10 03:32
Оценка:
Здравствуйте, samius, Вы писали:

M>>В первом варианте, ты обрабатываешь предоставленные "официально" API исключения.


S>Это самодеятельность. Официально там ничего не предоставлено. Ни FormatException, ни IndexOutOfRange, вообще ничего...


А откда вообще вы взяли HtmlAgilityPack? Я за нее ничего не говорил.
Re[4]: Писать ли в этом случае catch(Exception)?
От: samius Япония http://sams-tricks.blogspot.com
Дата: 12.08.10 03:34
Оценка:
Здравствуйте, 0K, Вы писали:

0K>Здравствуйте, samius, Вы писали:


M>>>В первом варианте, ты обрабатываешь предоставленные "официально" API исключения.


S>>Это самодеятельность. Официально там ничего не предоставлено. Ни FormatException, ни IndexOutOfRange, вообще ничего...


0K>А откда вообще вы взяли HtmlAgilityPack? Я за нее ничего не говорил.


Предположил по наименованию класса документа, метода загрузки из строки, отсутствию исключений. Угадал ведь?

З.Ы. лучше на "ты"
Re[5]: Писать ли в этом случае catch(Exception)?
От: 0K Ниоткуда  
Дата: 12.08.10 03:44
Оценка:
Здравствуйте, samius, Вы писали:

S>Предположил по наименованию класса документа, метода загрузки из строки, отсутствию исключений. Угадал ведь?


Я ее недавно использовал. Выбор был между SgmlReader, HTMLDocument2 и AgilityPack. Выбрал последний, т.к. он нейтивный и относительно безглючиный. Но на форуме я его сознательно не подразумевал (даже на знаю совпадает ли сигнатура методов), возможно подсознательно написал.
Re[6]: Писать ли в этом случае catch(Exception)?
От: samius Япония http://sams-tricks.blogspot.com
Дата: 12.08.10 03:48
Оценка:
Здравствуйте, 0K, Вы писали:

0K>Здравствуйте, samius, Вы писали:


S>>Предположил по наименованию класса документа, метода загрузки из строки, отсутствию исключений. Угадал ведь?


0K>Я ее недавно использовал. Выбор был между SgmlReader, HTMLDocument2 и AgilityPack. Выбрал последний, т.к. он нейтивный и относительно безглючиный. Но на форуме я его сознательно не подразумевал (даже на знаю совпадает ли сигнатура методов), возможно подсознательно написал.


Что такое HTMLDocument2? Может это MSHTML?

Хм, а ведь у SgmlReader и HtmlAgilityPack разные назначения. В чем заключается задача-то?
Re[7]: Offtop
От: 0K Ниоткуда  
Дата: 12.08.10 03:52
Оценка:
Здравствуйте, samius, Вы писали:

Отклолняемся от темы

S>Что такое HTMLDocument2? Может это MSHTML?


Он самый.

S>Хм, а ведь у SgmlReader и HtmlAgilityPack разные назначения. В чем заключается задача-то?


Назначение у них -- помощь в парсинге HTML. Только SgmlReader преобразоывавает все в валидный Xml -- но очень криво с можнеством ошибок.
Re[8]: Offtop
От: samius Япония http://sams-tricks.blogspot.com
Дата: 12.08.10 03:56
Оценка:
Здравствуйте, 0K, Вы писали:

S>>Хм, а ведь у SgmlReader и HtmlAgilityPack разные назначения. В чем заключается задача-то?


0K>Назначение у них -- помощь в парсинге HTML. Только SgmlReader преобразоывавает все в валидный Xml -- но очень криво с можнеством ошибок.


Да, сталкивался. Есть еще GPL-ный выпрямитель Html2Xhtml с одноименным враппером под дотнет. После него можно даже читать в XDocument и XmlDocument. Но AgilityPack все равно повсеяднее будет, чем такая связка.
Re: Писать ли в этом случае catch(Exception)?
От: Undying Россия  
Дата: 12.08.10 04:27
Оценка:
Здравствуйте, 0K, Вы писали:

0K>Пользователь указывает html (пусть в виде строки). Нужно проверить на корректность. Специального метода нет, но есть библиотека которая его парсит. Какой вариант проверки правильный:


Принципа при обработке исключений два:

1) Специальная обработка определенного типа исключения имеет смысл только тогда, когда есть понимание вероятной причины этого исключения и соответственно есть осмысленная стратегия исправления/минимизации последствий этой ошибки.

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

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


Соответственно применяем эти принципы к вашим вариантам:

Смотрим первый вариант:

catch (ArgumentException)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}


У вас есть понимание вероятной причины ArgumentException? Как вижу нет, соответственно нет и осмысленной стратегии исправления последствий этой ошибки. Так в чем тогда смысл этого кода?

Смотрим второй вариант:

catch (Exception)
{
    Console.WriteLine("Не удалось распарсить HTML");
    return;
}


Ни по одному виду ошибки у нас нет понимания ее вероятной причины, соответственно нет и осмысленных стратегий для их специальной обработки, этот факт мы честно отразили в коде. Этот вариант нормальный.
Re[3]: Писать ли в этом случае catch(Exception)?
От: Undying Россия  
Дата: 12.08.10 04:27
Оценка: +1
Здравствуйте, 0K, Вы писали:

0K>А вы хоть знаете что это не по стандарту? Вас любой FxCop остановит и заставить написать правильно. И MSDN откройте, почитайте. Общие исключения перехватывать нельзя.


А думать своей головой не пробовал? Без ссылок на авторитеты?
Re[2]: Писать ли в этом случае catch(Exception)?
От: midcyber
Дата: 12.08.10 04:44
Оценка: +1
Здравствуйте, Undying, Вы писали:

U>Ни по одному виду ошибки у нас нет понимания ее вероятной причины, соответственно нет и осмысленных стратегий для их специальной обработки, этот факт мы честно отразили в коде. Этот вариант нормальный.


Так и зачем их вообще тут перехватывать, если нет осмысленных стратегий?
Почему не передать выше, пусть разбирается тот, у кого они есть
Re[3]: Писать ли в этом случае catch(Exception)?
От: Undying Россия  
Дата: 12.08.10 04:55
Оценка:
Здравствуйте, midcyber, Вы писали:

M>Так и зачем их вообще тут перехватывать, если нет осмысленных стратегий?

M>Почему не передать выше, пусть разбирается тот, у кого они есть

В приведенном коде в общем catch'е есть осмысленная стратегия — это логгирование в виде Console.WriteLine("Не удалось распарсить HTML"). Вышестоящий код не знает, что ошибка состоит в том, что "Не удалось распарсить HTML", соответственно без catch (Exception ex) тут никак не обойтись.
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.