Злоупотребление try-catch или как не надо ловить эксэпшены. - .NET

Накипело, решил излить душу...

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

Сперва стоит отметить, что для логирования всех внештатных ситуаций (читай как — необработанных в исполняемых модулях исключений), существует обработчик содержащий такой код:

IModule module;
try
{
        ...
        module = ModuleFactory.CreateModule(moduleType, moduleContext, moduleParameters);
        module.Run();
        ...
}
catch (Exception ex)
{
        launcherContext.Log(LogType.Error, string.Format("Unhandled exception in module {0}...", moduleType), ex);
        return false;
}
finally
{
        module.Dispose();
}

В логах, помимо прочего, сохраняется StackTrace исключения. Как вы понимаете — весьма незаменимая вещь для устранения багов возникающих как во время тестирования, так и во время работы продакшн версии.
Все хорошо, но по всему коду встречаются такие предательские моменты:

try
{
        {SomeCodeThatThrowsException}
}
catch (SocketException sex)
{
        if (tries >= TRIES_COUNT)
        {
                moduleContext.Log(LogType.Debug, "Some error occurs during network operations. Error message: " + sex.Message);
                throw sex;
        }
        else
        {
                tries++;
        }
}

Не просто throw;, а именно throw sex;. На мой вопрос о смысле такого извращенния — никто не мог понять, что же именно в этом коде мне не нравится. Эх, долго мы общались, перед тем как поняли друг друга

И как они поддерживали эту систему столько времени

говорят, забирали с продакшна реальные данные и дебажили

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

static void Main(string[] args)
{
        try
        {
                FunctionThatThrowsException(); // line 13
        }
        catch (Exception ex)
        {
                Console.WriteLine(ex);
        }
}

Теперь рассмотрим 4 примера и проведем их небольшой анализ:

Пример 1.

private static void FunctionThatThrowsException()
{
    try
    {
        GenerateException(); 
    }
    catch (Exception ex)
    {
        /* тут некоторая обработка */
        throw ex; // line 29
    }
}

private static void GenerateException()
{
    throw new Exception("Some exception.");
}

Залогируется такое сообщение:

System.Exception: Some exception.
at TestExApp3.Program.FunctionThatThrowsException() in W:\...\TestExApp3\Program.cs:line 29
at TestExApp3.Program.Main(String[] args) in W:\...\TestExApp3\Program.cs:line 13

Что же мы можем вынести о первопричине такого сообщения? А лишь то, что где-то в блоке try метода FunctionThatThrowsException бросается исключение. В нашем примере очевино не составит труда понять, что исключене произошло в методе GenerateException, но в большинстве случаев, все не так тривиально.

Теперь рассмотрим пример с throw;

Пример 2.

private static void FunctionThatThrowsException()
{
    try 
    {
        GenerateException();
    }
    catch (Exception ex)
    {
        /* тут некоторая обработка */
        throw; // line 29
    }
}

private static void GenerateException()
{
    throw new Exception("Some exception."); // line 35
}

Имеем следующее:

System.Exception: Some exception.
at TestExApp3.Program.GenerateException() in W:\...\TestExApp3\Program.cs:line 35
at TestExApp3.Program.FunctionThatThrowsException() in W:\...\TestExApp3\Program.cs:line 29
at TestExApp3.Program.Main(String[] args) in W:\...\TestExApp3\Program.cs:line 13

Т.о. мы не теряем никакой информации о "источнике" сообщения и в ряде случаев мы можем достаточно быстро установить его причину. Однако, встречаются ситуации, в которых по-мимо всего того, что мы уже получили, было неплохо знать, а где же именно в блоке try метода FunctionThatThrowsException бросается исключение. Вот пример такой ситуации:

Пример 3.

private static void FunctionThatThrowsException()
{
    try
    {
        GenerateException(0);
        GenerateException(2);
    }
    catch (Exception ex)
    {
        /* тут некоторая обработка */
        throw; // line 31
    }
}
private static void GenerateException(int p)
{
    if (p > 0)
    {
        throw new Exception("Some exception."); // line 38
    }
}

Получаем:

System.Exception: Some exception.
at TestExApp3.Program.GenerateException(Int32 p) in W:\...\TestExApp3\Program.cs:line 38
at TestExApp3.Program.FunctionThatThrowsException() in W:\...\TestExApp3\Program.cs:line 31
at TestExApp3.Program.Main(String[] args) in W:\...\TestExApp3\Program.cs:line 13

Знай бы мы из StackTrace, что исключение выбрасывается во время исполнения GenerateException с параметром 2, мы бы избежали возни с выяснением этого (вполне возможно решающего) факта. А узнать можно заменив throw; на что-то типа throw new Exception("Some messega.", ex):

private static void FunctionThatThrowsException()
{
    try
    {
        GenerateException(0);
        GenerateException(2); // line 26
    }
    catch (Exception ex)
    {
        /* тут некоторая обработка */
        throw new Exception("Some exception in FunctionThatThrowsException(). See inner exception for more information.", ex); // line 31
    }
}
private static void GenerateException(int p)
{
    if (p > 0)
    {
        throw new Exception("Some exception."); // line 38
    }
}

Получаем:

System.Exception: Some exception in FunctionThatThrowsException(). See inner exc
eption for more information. ---> System.Exception: Some exception.
at TestExApp3.Program.GenerateException(Int32 p) in W:\...\TestExApp3\Program.cs:line 38
at TestExApp3.Program.FunctionThatThrowsException() in W:\...\TestExApp3\Program.cs:line 26
--- End of inner exception stack trace ---
at TestExApp3.Program.FunctionThatThrowsException() in W:\...\TestExApp3\Program.cs:line 31
at TestExApp3.Program.Main(String[] args) in W:\...\TestExApp3\Program.cs:line 13

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

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

Пример 4.
К нам совсем недавно в проект подкинули нового сотрудника-студента, меня попросили делать ревью его кода. И опять то же самое, да еще и в квадрате — мания какая-то:

    catch (Exception ex)
    {
        throw ex;
    }

Прям вот так, один в один. Нет даже той обработки — /* тут некоторая обработка */, ради которой весь catch и затеивался. Зачем спрашиваю, внятно объяснить не может. Отругал, объяснил, заставил где /* тут некоторая обработка */ нет, убирать try-catch совсем. В тот же день пришлось модифицировать код FTP клиента Melih.FTP.Client Каково же было мое удивление когда я встретил то же самое по всему(!) коду, например:

private void OpenActiveDataConnection(ref Socket servsock)
{
    try
    {
        string szReply;
        IPEndPoint ipeClient = (IPEndPoint)FTPSocket.LocalEndPoint;
        servsock.Bind(new IPEndPoint(IPAddress.Parse(ipeClient.Address.ToString()),0));
        IPEndPoint ipeServer = (IPEndPoint)servsock.LocalEndPoint;
        int iTransferPort = ipeServer.Port;
        int iPortPart1 = (iTransferPort&0xff00) >> 8;
        int iPortPart2 = iTransferPort&0x00ff;
        SendCommand("PORT "+ipeClient.Address.ToString().Replace('.',',')+","+iPortPart1.ToString()+","+iPortPart2.ToString());
        szReply = GetReply();
        if (!CheckReply(szReply,"200"))
            throw(new FTPClientException(RemoveReturnCode(szReply)));
        servsock.Listen(5);
        return;
    }
    catch (System.Exception ex)
    {
        throw(ex);
    }
}

ЗАЧЕМ!!!

Это просто мания какая-то.

В общем моя чаша тепрения переполнилась и появился этот вот пост.

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

Здравствуйте, ie, Вы писали:

ie>Накипело, решил излить душу...

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

ie>ЗАЧЕМ!!!

Это просто мания какая-то.

Полагаю причина проста. Многие программисты пришли в .net c java. А в java стек-трейс к объекту-исключению прикрепляется в момент создания объекта, а не в момент выброса. Вот по инерции и пишут.

Здравствуйте, ie, Вы писали:

ie>

ie>private void OpenActiveDataConnection(ref Socket servsock)
ie>{
ie>    try
ie>    {
ie>        string szReply;
ie>        IPEndPoint ipeClient = (IPEndPoint)FTPSocket.LocalEndPoint;
ie>        servsock.Bind(new IPEndPoint(IPAddress.Parse(ipeClient.Address.ToString()),0));
ie>        IPEndPoint ipeServer = (IPEndPoint)servsock.LocalEndPoint;
ie>        int iTransferPort = ipeServer.Port;
ie>        int iPortPart1 = (iTransferPort&0xff00) >> 8;
ie>        int iPortPart2 = iTransferPort&0x00ff;
ie>        SendCommand("PORT "+ipeClient.Address.ToString().Replace('.',',')+","+iPortPart1.ToString()+","+iPortPart2.ToString());
ie>        szReply = GetReply();
ie>        if (!CheckReply(szReply,"200"))
ie>            throw(new FTPClientException(RemoveReturnCode(szReply)));
ie>        servsock.Listen(5);
ie>        return;
ie>    }
ie>    catch (System.Exception ex)
ie>    {
ie>        throw(ex);
ie>    }
ie>}
ie>

ie>ЗАЧЕМ!!!

Это просто мания какая-то.

Я так делал все время особенно при поиске ошибок в чужом коде. На throw(ex); ставятся точки останова.

Во первых потому, что перехватчики обычно снабжаются сообщениями, которые еще и в ЕХЕ, а сам раскапываешь DLL.
и такие случаи тебе могут подсунуть:

throw new Exception("Some exception.")

так что, придется покопаться.

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

Так что по моему это нормально (зря на парня наехал) он же не писал так:

catch (System.Exception ex)
{
    /*пусто*/
}

Могу только присвистнуть. Не подскажете откуда студент? А то я тут недавно встретился с таким же точно кодом.
Вообще, exception порежо обрабатывать

(в смысле не допускать их возникновения).

--
VBStreets, Editor-in-Chief
http://blogs.gotdotnet.ru/personal/gaidar/

If you don't know where you are going, any road will take you there...

данное сообщение получено с www.gotdotnet.ru
ссылка на оригинальное сообщение

Здравствуйте, hexen, Вы писали:

H>Я так делал все время особенно при поиске ошибок в чужом коде. На throw(ex); ставятся точки останова.

А на "throw;" нельзя поставить точку останова?

Здравствуйте, dshe, Вы писали:

D>Здравствуйте, hexen, Вы писали:

H>>Я так делал все время особенно при поиске ошибок в чужом коде. На throw(ex); ставятся точки останова.

D>А на "throw;" нельзя поставить точку останова?

т.е.

catch(Exception ex)
{
    throw;
}

вместо

catch(Exception ex)
{
    throw ex;
}

можно, кому как нравиться.

Здравствуйте, hexen, Вы писали:

H>Здравствуйте, dshe, Вы писали:

D>>Здравствуйте, hexen, Вы писали:

H>>>Я так делал все время особенно при поиске ошибок в чужом коде. На throw(ex); ставятся точки останова.

D>>А на "throw;" нельзя поставить точку останова?

H>т.е.

H>

H>catch(Exception ex)
H>{
H>    throw;
H>}
H>

H>вместо

H>

H>catch(Exception ex)
H>{
H>    throw ex;
H>}
H>

H>можно, кому как нравиться.

Ну так ie как раз и ратует за первый вариант поскольку во втором теряется stack trace.

Здравствуйте, dshe, Вы писали:

D>Здравствуйте, hexen, Вы писали:

H>>Я так делал все время особенно при поиске ошибок в чужом коде. На throw(ex); ставятся точки останова.

D>А на "throw;" нельзя поставить точку останова?

Спасибо, коллега!

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, dshe, Вы писали:

ie>>ЗАЧЕМ!!!

Это просто мания какая-то.

D>Полагаю причина проста. Многие программисты пришли в .net c java. А в java стек-трейс к объекту-исключению прикрепляется в момент создания объекта, а не в момент выброса. Вот по инерции и пишут.

Да, ты, видимо, прав. Почти все в свое время перелезли с явы.

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, hexen, Вы писали:

ie>>ЗАЧЕМ!!!

Это просто мания какая-то.

H>Я так делал все время особенно при поиске ошибок в чужом коде. На throw(ex); ставятся точки останова.

H>Во первых потому, что перехватчики обычно снабжаются сообщениями, которые еще и в ЕХЕ, а сам раскапываешь DLL.
H>и такие случаи тебе могут подсунуть:

H>

H>throw new Exception("Some exception.")
H>

H>так что, придется покопаться.

H>Если знаешь точку входа в DLL , можешь загрузить только ее проект и подключиться к процессу, поставить перехватчик, поставить точку останова.

Хорошо когда так, как ты пишешь. Мы имеем дело с системой, которую поотлаживать в дебагере затруднительно, хотя бы по причине невозможности доступа к продакшн серверу, а ошибки на почту сыплются, где уж тут точка останова. И как уже написал dshe, ну поставь throw; если уж это необходимо.

H>Так что по моему это нормально (зря на парня наехал) он же не писал так:

H>

H>catch (System.Exception ex)
H>{
H>    /*пусто*/
H>}
H>

За такое бы сразу уволил

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, gaidar, Вы писали:

G>Могу только присвистнуть. Не подскажете откуда студент? А то я тут недавно встретился с таким же точно кодом.
G>Вообще, exception порежо обрабатывать

(в смысле не допускать их возникновения).

Студент из НГУ, кафедра ФТИ (если не ошибаюсь). Парень-то талантливый, но пока такие порой ляпы делает. Недавно вот функцию GetPositionOfStringInSubstring написал (ну не знал, что String.IndexOf существует), так над реализацией весь отдел смеялся.

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, ie, Вы писали:

ie>Накипело, решил излить душу...

В глобальных обработчиках я просматриваю до null

ex.GetBaseException()

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

throw new Exception("Какая то ошибка");

, а не

throw new Exception("Какая то ошибка", ex);

)

... << RSDN@Home 1.1.4 stable SR1 rev. 568>>

Здравствуйте, SEDEGOFF, Вы писали:

SED>В глобальных обработчиках я просматриваю до null

ex.GetBaseException()

На самом деле просматривать до null будет проблемно, ибо

Return Value
The first exception thrown in a chain of exceptions. If the InnerException property of the current exception is a null reference (Nothing in Visual Basic), this property returns the current exception.

Возможно ты имел ввиду InnerException.

SED>и все — ничего пока не потерял. Единственный недостаток, это если при порождении исключения не будет указано основание (будет написано

throw new Exception("Какая то ошибка");

, а не

throw new Exception("Какая то ошибка", ex);

)

Здравствуйте, dshe, Вы писали:

D>Ну так ie как раз и ратует за первый вариант поскольку во втором теряется stack trace.

Спасибо. Возьму на заметку.

Здравствуйте, ie, Вы писали:

ie>Накипело, решил излить душу...

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

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

Помнится, у Рихтера в "Программировании на платформе MS .NET Framework" это все доходчиво описано в главе про исключения. Жаль, не все читают классиков

... << RSDN@Home 1.1.4 beta 7 rev. 447>>

Здравствуйте, ie, Вы писали:

Я вообще не понимаю любителей натыкать try/catch-ей на всякий случай. Сам стараюсь вообще по реже пользоваться этой конструкцией и уж если влепляю ее, то точно обдумано.

... << RSDN@Home 1.2.0 alpha rev. 618>>

Здравствуйте, hexen, Вы писали:

H>

H>catch(Exception ex)
H>{
H>    throw;
H>}
H>

H>вместо

H>

H>catch(Exception ex)
H>{
H>    throw ex;
H>}
H>

H>можно, кому как нравиться.

Этот пост из серии "гляжу в книгу, вижу фигу". Ты его сообщение внимательно читал?

ЗЫ

Я фигею дорогая редакция...

... << RSDN@Home 1.2.0 alpha rev. 618>>

От:	ie	http://ziez.blogspot.com/
Дата:	27.09.05 10:49
Оценка:	125 (16) +6

	От:	dshe
	Дата:	27.09.05 11:16
	Оценка:	1 (1)

От:	hexen	https://github.com/maliutin
Дата:	27.09.05 12:52
Оценка:

	От:	Аноним
	Дата:	27.09.05 13:17
	Оценка:

	От:	dshe
	Дата:	27.09.05 13:34
	Оценка:	+2

От:	SEDEGOFF	www.srcsoft.com
Дата:	28.09.05 02:55
Оценка:

От:	VladD2	www.nemerle.org
Дата:	29.09.05 00:00
Оценка: