8

Скажите, пожалуйста, пробрасывание исключений на верх это нормальная практика?

Просто сейчас в проекте своем учебном попробовал это сделать (точнее сделал это во все узких места, особенно там, где идет работа с БД), и мне показалось это удобным в плане поиска ошибок и отладке.

Пример (на логику не обращайте внимания, интересует только проброс исключений):

       // Получение какого-либо значения из базы данных в виде object
        private object GetData_AsObject()
        {
            try
            {
                //тут идет получение данных из бд
            }
            catch (Exception e)
            {
                throw new Exception("Получение данных из " + tableName + "." + column);
            }
        }

        // Получение какого-либо значения из базы данных в виде string 
        public string GetData_AsString()
        {
            try
            {
                if (GetData_AsObject() != null)
                    return GetData_AsObject().ToString();
                else
                    return "";
            }
            catch (Exception e)
            {
                throw new Exception(e.Message + " as string");
            }
        }

        // Получение какого-либо значения из базы данных в виде int 
        public int GetData_AsInt()
        {
            try
            {
                if (GetData_AsObject() != null)
                    return int.Parse(GetData_AsObject().ToString());
                else
                    return 0;
            }
            catch (Exception e)
            {
                throw new Exception(e.Message + " as int");
            }
        }

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

Но в коде получается очень много конструкций try-catch. Не считается ли это захламлением?

3
  • (ответ от нуба) О c# знаю только то, что это язык программирование. Недавно по похожему вопросу [статью][1] читал (там про node.js) и там приведены несколько советов, что делать если произошла ошибка (по теме с абзаца "Handling operational errors"). [1]: joyent.com/developers/node/design/errors
    – alvoro
    16 мая 2014 в 14:49
  • Отличный вопрос.
    – VladD
    16 мая 2014 в 15:05
  • 1
    @VladD, спасибо. Просто если раньше старался просто написать код, чтобы хоть как-то работало, то сейчас начинаю задумываться над тем, как я это делаю. Вот такие вопросы и начали появляться...
    – teanЫЧ
    16 мая 2014 в 17:48

3 ответа 3

6

Да, это вполне нормальная практика. Чаще всего код не должен заниматься обработкой исключений - у него обычно на это нет соответствующих полномочий. Иными словами, это не его дело. Он должен либо выполнить некие действия, либо, получив исключение, пробросить его вызывающему коду, которому можно доверить эту самую обработку (что само по себе является отдельной задачей, а согласно Single responsibility, первому из принципов SOLID, объект не должен брать на себя более одной обязанности) Бывают, конечно, ситуации, когда исключение является одним из видов "штатной ситуации", и сгенерировавший его код способен корректно его обработать, но это довольно нечастые случаи (и честно говоря, примеры таких ситуаций у меня в голове как-то сразу не всплывают). Более того, тут возникает вопрос - стоит ли в такую ситуацию рассматривать как исключение.

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

З.Ы. несмотря на ваши слова про "на логику не обращайте внимания", я всё не удержусь и добавлю: искренне надеюсь, следующий кусок не из реального кода?

if (GetData_AsObject() != null)
    return GetData_AsObject().ToString();
else
    return "";

З.Ы. вы же вроде задавали несколько месяцев тому назад очень похожий вопрос

10
  • нет)) поэтому и попросил не обращать внимания. наспех написал лишь бы что-то было А так я стараюсь разделить логику от интерфейса пользователя. И все эти исключения поднимаются наверх к визуальной части приложения и уже там показываются, если ранее не обработались. Да, там тоже вопрос про исключения, но тогда я не знал про то, что их можно поднимать вверх.
    – teanЫЧ
    16 мая 2014 в 14:43
  • это радует. А то очень уж опасные вещи вы в нем творите >И все эти исключения поднимаются наверх к визуальной части приложения и уже там показываются, если ранее не обработались так обрабатывайте их, зачем показывать пользователю внутренности вашего приложения?
    – DreamChild
    16 мая 2014 в 14:46
  • Я их обрабатываю. Просто если вылетает ошибка, при которой нормальная работа приложения не может гарантироваться, надо же ее показать?
    – teanЫЧ
    16 мая 2014 в 14:49
  • >Просто если вылетает ошибка, при которой нормальная работа приложения не может гарантироваться, надо же ее показать вам надо показывать не ошибку, а некую человекопонятную информацию об этой ошибке. И завершать программу впоследствии
    – DreamChild
    16 мая 2014 в 14:52
  • Вот поэтому я и делаю проброс с описанием исключения. Чтобы пользователь потом мог объяснить, в чем проблема, и я сразу мог понять, где копать. Другой вопрос - стоит ли так подробно все расписывать, не достаточно ли описания ошибки, которое получено на верхнем уровне?
    – teanЫЧ
    16 мая 2014 в 14:59
3

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

Исключение стоит ловить в нескольких случаях:

  1. Если код, который его ловит, может восстановиться после исключения - корректно продолжнить работу). Например, создать базу данных, если подключение к ней упало с ошибкой "базы данных не существует". Или повторить команду, если первая попытка упала с ошибкой соединения. Исключение ловится и выше не перебрасывается.

  2. Если код хочет залоггировать факт исключения на границе слоя. Исключение ловится, логгируется, и перебрасывается через throw;.

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

  4. В ситуации из вопроса - Если код может добавить дополнительную информацию к оригинальному исключению. Например, дописать tableName и column, как сделано в вопросе. Исключение ловится, бросается новое исключение, с обязательной передачей оригинального исключения как параметра innerException:

private object GetData_AsObject()
{
    try
    {
        //тут идет получение данных из бд
    }
    catch (Exception e)
    {
        // передаем e вторым параметром в конструктор Exception
        throw new Exception("Получение данных из " + tableName + "." + column, e);
    }
}

А "пробрасывать" исключения так, как это сделано в коде из вопроса - нельзя, т.к. при этом полностью теряются детали оригинального исключения, и сообщение об ошибке приходит к разработчику в виде "где-то что-то пошло не так, а где именно и что именно - пусть гадает!"

1

Я считаю нет смысла пробрасывать Exception. На верхнем уровне сделать try-catch и выводить не e.Message, а e.ToString() - в нем будет и тип исключения и все вложенные функции до исключения, и даже возможно строка кода. А "пробрасывая" так вы всю дополнительную информацию теряете.

3
  • Ага, и внешний код должен знать все типы внутренних исключений? Или ловить catch (Exception)?
    – VladD
    3 дек 2017 в 7:59
  • @VladD это нормальная практика для веб-бэкенда. Если код от исключения не может восстановиться или добавить дополнительную инфу - не ловить исключение. Сверху обычно стоит глобальный трайкетч, который пишет в лог и выдает клиенту 500 ошибку.
    – user177221
    3 дек 2017 в 9:37
  • @VladD вообще странно, в вопросе описан наихудший подход к обработке - ловля с потерей стектрейса и деталей. Причем для него наверху тоже придется писать catch(Exception). И это названо пробрасыванием. А в принятом заплюсованном ответе написано "да, это нормально". И все это только потому, что топикастер под пробрасыванием подразумевал "поймать и бросить новое исключение без деталей"
    – user177221
    3 дек 2017 в 9:40

Ваш ответ

Нажимая на кнопку «Отправить ответ», вы соглашаетесь с нашими условиями использования и подтверждаете, что прочитали и поняли наши политику конфиденциальности и нормы поведения.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.