Злоупотребление try-catch или как не надо ловить эксэпшены.
От: ie Россия http://ziez.blogspot.com/
Дата: 27.09.05 10:49
Оценка: 125 (16) +6 :))
Накипело, решил излить душу...

Итак, с чего все началось. Пришел в новый проект, проект большой, длится на протяжении уже не первого года и теперь вошел в очередную стадию. Вот собственно на эту стадию я и подоспел. Все хорошо: архитектура продумана хорошо, разработчики вроде профессионалы, однако есть у них чрезмерная любовь к 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, подумайте, а что же лучше написать после него.
Превратим окружающую нас среду в воскресенье.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.