Исключения вместо if-ов: почему нет?
В большинстве случаев, мы читаем код чаще, чем пишем. Большинство практик программирования нацелены на упрощение понимания кода: чем проще код, тем меньше багов он содержит и тем проще его поддержка.
Использование исключений для контроля хода выполнения программы маскирует намерения программиста, поэтому это считается плохой практикой.
public void ProcessItem(Item item)
{
if (_knownItems.Contains(item))
{
// Do something
throw new SuccessException();
}
else
{
throw new FailureException();
}
}
Метод ProcessItem усложняет понимание кода, т.к. невозможно сказать какие возможны результаты его выполнения просто глядя на его сигнатуру. Подобный код нарушает принцип наимешего удивления, т.к. бросает исключение даже в случае положительного исхода.
В данном конкретном случае решение очевидно — нужно заменить выбрасывание исключения на возврат булевского значения. Давайте рассмотрим более сложные примеры.
Исключения для валидации входящих данных
Возможно, наиболее часто встречающаяся практика в контексте исключений — это использование их в случае получения неверных входящих данных.
public class EmployeeController : Controller
{
[HttpPost]
public ActionResult CreateEmployee(string name, int departmentId)
{
try
{
ValidateName(name);
Department department = GetDepartment(departmentId);
// Rest of the method
}
catch (ValidationException ex)
{
// Return view with error
}
}
private void ValidateName(string name)
{
if (string.IsNullOrWhiteSpace(name))
throw new ValidationException(“Name cannot be empty”);
if (name.Length > 100)
throw new ValidationException(“Name length cannot exceed 100 characters”);
}
private Department GetDepartment(int departmentId)
{
using (EmployeeContext context = new EmployeeContext())
{
Department department = context.Departments
.SingleOrDefault(x => x.Id == departmentId);
if (department == null)
throw new ValidationException(“Department with such Id does not exist”);
return department;
}
}
}
Очевидно, подобный подход имеет некоторые плюсы: он позволяет нам быстро «вернуться» из любого метода прямо в catch блок метода CreateEmployee.
Теперь давайте посмотрим на следующий пример:
public static Employee FindAndProcessEmployee(IList<Employee> employees, string taskName)
{
Employee found = null;
foreach (Employee employee in employees)
{
foreach (Task task in employee.Tasks)
{
if (task.Name == taskName)
{
found = employee;
goto M1;
}
}
}
// Some code
M1:
found.IsProcessed = true;
return found;
}
Что общего имеют эти два сэмпла? Оба они позволяют прервать текущий поток выполнения и быстро перейти к определенной точке в коде. Единственная проблема в таком коде — он существенно ухудшает читаемость. Оба подхода затрудняют понимание кода, именно поэтому использование исключений для контроля потока выполнения программы часто уравнивают с использованием goto.
При использовании исключений сложно понять где именно они ловятся. Вы можете обернуть код, выбрасывающий исключение, в try/catch блок в том же методе, а можете поместить try/catch блок на несколько уровней по стеку выше. Вы никогда не можете знать наверняка сделано ли это намерено или нет:
public Employee CreateEmployee(string name, int departmentId)
{
// Это баг или метод специально был помещен сюда без try/catch блока?
ValidateName(name);
// Rest of the method
}
Единственный способ узнать это — проанализировать весь стек. Исключения, использующиеся для валидации, делают код намного менее читаемым, т.к. они недостаточно ясно показывают намерения разработчика. На такой код невозможно просто посмотреть и сказать что может пойти не так и как необходимо отреагировать на это.
Есть ли способ лучше? Конечно:
[HttpPost]
public ActionResult CreateEmployee(string name, int departmentId)
{
if (!IsNameValid(name))
{
// Return view with error
}
if (!IsDepartmentValid(departmentId))
{
// Return view with another error
}
Employee employee = new Employee(name, departmentId);
// Rest of the method
}
Указание всех проверок явным образом делает ваши намерения намного более понятными. Эта версия метода проста и очевидна.
Исключения для исключительных ситуаций
Так когда же использовать исключения? Главная цель исключений — сюрприз! — обозначить исключительную ситуацию в приложении. Исключительная ситуация — это ситуация, в которой вы не знаете что делать и наилучшим выходом для вас является прекращение выполнения текущией операции (возможно, с предварительным логированием деталей ошибки).
Примерами исключительных ситуаций могут являться проблемы с подключением к БД, отсутствие необходимых конфигурационных файлов и т.п. Ошибки валидации не являются исключительной ситуацией, т.к. метод, проверяющий входящие данные, по определению ожидает что они могут быть неверными.
Другим примером корректного использования исключений является валидация контрактов (code contract). Вы, как автор класса, ожидаете, что клиенты этого класса будут соблюдать его контракты. Ситуация, при которой контракт метода не соблюден, является исключительной и заслуживает выбрасывания исключения.
Как работать с исключениями, брошенными другими библиотеками?
Является ли ситуация исключительной, зависит от контекста. Разработчик сторонней библиотеки может не знать как иметь дело с проблемами подключения к БД, т.к. он не знает в каком контексте будет использоваться его библиотека.
В случае подобной проблемы, разработчик библиотеки не имеет возможности что-либо с ней сделать, поэтому бросание исключения будет подходящим решением. Вы можете взять Entity Framework или NHibernate в качестве примера: они ожидают, что БД всегда доступна и если это не так, бросают исключение.
С другой стороны, разработчик, использующий библиотеку, может ожидать, что база данных уходит в офлайн время от времени и разрабатывать свое приложение с учетом этого факта. В случае отказа БД, клиентское приложение может попытаться повторить ту же операцию или отобразить сообщение пользователю с предложением повторить операцию позже.
Таким образом, ситуация может быть исключительной с точки зрения нижележащего кода и ожидаемой с точки зрения клиентского кода. Как в этом случае работать с исключениями, брошенными такой библиотекой?
Подобные исключения должны ловиться как можно ближе к коду, их выбрасыющему. Если это не так, ваш код будет иметь те же недостатки, что и пример кода с goto: невозможно будет понять где это исключение обрабатывается без анализа всего стека вызова.
public void CreateCustomer(string name)
{
Customer customer = new Customer(name);
bool result = SaveCustomer(customer);
if (!result)
{
MessageBox.Show(“Error connecting to the database. Please try again later.”);
}
}
private bool SaveCustomer(Customer customer)
{
try
{
using (MyContext context = new MyContext())
{
context.Customers.Add(customer);
context.SaveChanges();
}
return true;
}
catch (DbUpdateException ex)
{
return false;
}
}
Как можно видеть на примере выше, метод SaveCustomer ожидает проблемы с базой данных и намеренно отлавливает все ошибки, связанные с этим. Он возвращает булевский флаг, который затем обрабатывается кодом выше по стеку.
Метод SaveCustomer имеет понятную сигнатуру, говорящую нам, что в процессе сохранения клиента могут быть проблемы, что эти проблемы ожидаемы и что вы должны проверить возвращаемое значение чтобы убедиться, что все в порядке.
Стоит отметить широко известную практику, применимую в данном случае: не нужно оборачивать подобный код в generic обработчик. Generic обработчик утверждает, что любые исключения являются ожидаемыми, что по сути неправда.
Если вы действительно ожидаете какие-либо исключения, вы делаете это для очень ограниченного их числа, для которого вы точно знаете, что вы можете их обработать. Помещение generic обработчика приводит к тому, что вы проглатываете не ожидаемые вами исключения, приводя приложение в неконсистентное состояние.
Единственная ситуация, в которой generic обработчики применимы — это размещение их на самом высоком уровне по стеку приложения для отлова всех исключений, не пойманных кодом ниже, для того, чтобы залогировать их. Подобные исключения не следует пытаться обработать, все что можно сделать — это закрыть приложение (в случае со stateful приложением) или прекратить текущую операцию (в случае со stateless приложением).
Исключения и fail-fast принцип
Как часто вы встречаете код подобный этому?
public bool CreateCustomer(int managerId, string addressString, string departmentName)
{
try
{
Manager manager = GetManager(managerId);
Address address = CreateAddress(addressString);
Department department = GetDepartment(departmentName);
CreateCustomerCore(manager, address, department);
return true;
}
catch (Exception ex)
{
_logger.Log(ex);
return false;
}
}
Это пример некорректного использования generic обработчика исключений. Код выше подразумевает, что все исключения, приходящие из тела метода, являются признаком ошибки в процессе создания кастомера. В чем проблема подобного кода?
Помимо схожести с «goto» семантикой, обсуждаемой выше, пробема состоит в том, что исключение, приходящее в catch блок может не быть известным нам исключением. Исключение может быть как ArgumentException, которое мы ожидаем, так и ContractViolationException. В последнем случае, мы прячем баг, притворяясь, что нам известно как обрабатывать подобное исключение.
Итогда подобный подход применяется разработчиками, которые хотят защитить свое приложение от неожиданных падений. На самом же деле подобный подход только маскирует проблемы, делая более сложным процесс их вылавливания.
Наилучший способ работы с неожидаемыми исключениями — прекратить текущую операцию полностью и предотвратить распространение неконсистентного состояния по приложению.
Заключение
- Бросайте исключение тогда и только тогда, когда надо задекларировать исключительную ситуацию в коде.
- Используйте возвращаемые значения при валидации входящих данных.
- Если вы знаете, как обрабатывать исключения, бросаемые библиотекой, делайте это как можно ближе к коду, их бросающему.
- Если вы имеете дело с неожидаемым исключением, прекращайте текущую операцию полностью. Не притворяйтесь, что знаете как иметь дело с такими исключениями.
Ссылка на оригинал статьи: Exceptions for flow control in C#
Комментарии (35)
mayorovp
28.07.2015 09:48Проблема затронута правильная — отсутствие информации об исключениях в сигнатуре метода. Вот только решение — отказаться от почти всех исключений — странное. Ведь если проблема в отсутствии информации, то ее можно туда добавить!
Можно добавить информацию об исключении в xmldoc. А можно — начать название метода с префикса ThrowIf — тогда факт возможного «выпадения» исключения будет виден еще лучше.
DjoNIK
28.07.2015 10:06В Java, вроде, пока не пропишешь в сигнатуре, не можешь наружу пробросить исключение. Но там тоже от этого какие-то неудобства, если не ошибаюсь.
А ThrowIf мне видится половинчатым решением. Что если внутренний метод бросает исключение, но мы пока не знаем как его обрабатывать, тоже помечать ThrowIf? Но это будет вводить в заблуждение.
urrri
28.07.2015 11:18+1В Java есть два типа исключений:
— декларируемые, которые нужно декларировать по всей иерархии вызовов
— RuntimeException, которые появились из-за диких проблем с первыми. Они работают так же, как исключения в С#DjoNIK
28.07.2015 11:23А какие именно эти дикие проблемы?погуглил. Одна из основных, как я понял, лежит в плоскости подавления исключений где-то по середине, дабы ублажить компилятор.urrri
28.07.2015 11:24+1Основной проблемой первых считается то, что изменение имплементации функции может потребовать изменение её сигнатуры, что не совсем верно
EvilsInterrupt
28.07.2015 11:51+1// Это баг или метод специально был помещен сюда без try/catch блока?
ValidateName(name);
Какой-то искусственный пример. Вполне возможно в корпоративной документации есть мануал о том как сообщать об ошибках и возможно там есть строчки вида «Мы всегда бросаем исключения». Если члены команды между собою договорились то и проблемы нет! А новичку всегда нужно давать читать «курс молодого бойца»DjoNIK
28.07.2015 12:02+1Проблема документации в поддержании актуальности документов и синхронизации, в случае изменения, со всеми членами команды. Человеческий фактор довольно быстро превращает красивую модель с использованием документации в еще одну проблему (как на той самой картинке).
Да и наименьшего удивления приведен не просто так — код по возможности должен быть самоописываемым. Да, это очень тяжело реализовать в реальных условиях, но жить с этим гораздо легче.EvilsInterrupt
28.07.2015 12:22Проблема документации в поддержании актуальности документов и синхронизации
Проблем с поддержанием нет.
Приведу пример: Как только мне придет в голову светлая мысль и я закомичу код в котором в качестве отступа будет табуляция вместо 4 пробелов в компании, где работаю, Заверяю Вас мои коллеги мне сразу же дадут знать о том как надо и как не надо писать код.
Правила нужны не для того чтобы туда записать и потом их поддерживать. Правила нужны для того чтобы избавлять команду от проблем! Если это правило действительно приносит серьезную пользу множеству людей и причем существенную, то правило быстро будет ВСЕГДА применяться на практике. Человеку просто не дадут сделать неправильно.
>>Да и наименьшего удивления приведен не просто так
Это имеет смысл, если Вы пришли в сторонний проект, к примеру в Open-Source. Тогда да, лучше никого не удивлять и явно говорить о том что Вы хотите выразить. Но в компаниях лучше выстраивать правила с другими членами команды. Код будет проще. Понятнее.MacIn
28.07.2015 15:05Забавно, когда заходишь почитать комментарии и видишь те же самые доводы и те же самые контр-аргументы, 1 в 1. В прошлый раз я отстаивал вашу позицию.
EvilsInterrupt
28.07.2015 18:03В прошлый раз я отстаивал вашу позицию.
Ваши слова звучат так, как будто бы Ваша позиция поменялась. Это так? Если да, то почему? ;)MacIn
29.07.2015 16:04Нет, все то же. Просто забавно, когда аргументы повторяются слово в слово. И каждый раз — как вновь.
feedbee
28.07.2015 12:38+6Вот то, что я рассказал бы про исключения тому, кто пока не умеет ими пользоваться.
- Исключения нужно использовать тогда и только тогда, когда возникает развитие событий, не предусмотренное нормальным ходом работы приложения — исключительной ситуации. При этом причина может быть как статической (например, логическая ошибка в коде), так и динамическая (например, недоступность ресурсов).
- Исключения нужно кидать максимально точно (узко) типизированными.
- Исключения замечательны для решения своей задачи — прерывания процесса с информированием о возникшей проблеме — причине прерывания, потому что они всплывают по стеку до нужного места. Для других задач они не подходят.
- Обрабатывать исключения нужно там, где их одновременно возможно и уместно обработать.
- В прикладном ПО большинство бросаемых на практике исключений не обрабатываются, перехватываются в самой высокой точке стека и попадают в лог, а пользователь получает ошибку 500 «Что-то пошло не так».
Mixim333
28.07.2015 18:56По поводу 2 пункта полностью согласен, сам для своего кода в 90% случаев создаю свои классы-исключения.
Сейчас на сервере крутится одно мое приложение, которое использует сторонний сервис для получения данных (отправил запрос — получил ответ — распарсил — сохранил в БД — обработал — разослал — примерно такая логика). У меня сделана очень-очень жесткая валидация того, что мне возвращает этот сервис — на каждый «некорректный» символ мой код бросает исключение (7 одного класса методов могут выкинуть исключение), которые логируются + сам сервер может иметь высокую нагрузку и не справляться (также логируется, но с другим кодом) — обращаюсь к авторам этого сервиса: «Слушайте, почему на 30% запросов Вы мне возвращаете не валидные ответы?» — «Не может такого быть, у нас все работает как часы! Давай данные для примера» — «XXX — по ним примерно 10% ошибок» — «А, правда, мы этого не учли, исправим. Давай следующие» — «YYY — еще 10%» — «А ну вот здесь вот все понятно, там формат другой, поправим...»… в итоге, 100% исключений на «некорректный» символ оказались правильными, а если учесть, что объем данных около 1000000 строк в неделю, 30% из которых не валидны, то не плохо получается.
Если же кидать не исключения, а реализовывать методы вроде TryXXX, которые возвращают true\false, то у меня получилось бы такая куча ветвлений, что просто ужас.
Сам стараюсь сильно не увлекаться генерацией исключений, использую их очень осторожно.
Angelina_Joulie
28.07.2015 19:14Ага, а потом утром придя на работу, а служба поддержки пользователей встречает тебя с вилами:
— Нам всё утро звонили пользователи и просили объяснить им «Что именно пошло не так, и что им с этим делать?»
Я каждый день работаю с разработчиками, и всё больше и больше убеждаюсь, что вопросы диагностики и работы с исключениями разработчиками спускаются на тормозах. Почему? Это вопрос философский, чем практический.
При работе с исключениями, я бы выделила две особенности:
а. Исключения для разработчика
б. Исключения для пользователя
Во-первых, сообщения для разработчика нужно снабжать необходимой _полезной_ информацией. Что бы те получив исключение UserNotFoundException долго не мучились пытаясь хоть как-то понять, какой именно пользователь не был найден. И всегда помнить, что исключение может произойти тогда, когда ваше приложение развёрнуто в 100500 серверах по такому же количеству ДЦ и подключится отладчиком, или включить подробную (verbose) детализацию логов у вас может не получится.
Ко второй категории, исключений я бы отнесла те, что имеют все больше шансов быть отображёнными пользователю. (Это ведь исключения. Никто не может дать 100% гарантии.) Такие исключения, нужно не только чётко типизировать, но ещё и давать им идентификационные номера, классы, описание, и всё, что будет вам полезно услышать от пользователя, в момент его обращения к вам с проблемой.
N.B. А многие любят писать такое:
try
{
}
catch(Exception ex)
{
log.Write(...., ex);
}
По этому мы на собеседования выносим вопросы:
а. А как себя поведёт этот код, если случилось StackOverflowException? SEHException? OutOfMemoryException?
P.S. Спасибо разработчикам C# 6.0 за фильтрацию исключений
Exception Filters
alan008
28.07.2015 20:12Исключения нужно использовать тогда и только тогда, когда возникает развитие событий, не предусмотренное нормальным ходом работы
Увы, в любой строке кода может произойти «не нормальный ход работы». Деление на ноль, целочисленное переполнение, ошибка выделения памяти, ошибка отсутствия прав доступа к ресурсу (файлу и т.п.), невозможность выполнения операции (например, передача данных по закрытому сокету и т.п.). Т.е. всегда нужно писать код с мыслью в голове, что из любой строки кода может прийти исключение. И пусть даже его обработка будет заключаться в «перехвате в самой высокой точке стека», все равно это гораздо лучше, чем где-то в глубине загасить исключение и делать вид, что всё хорошо. Иначе получится не программа, а «труп прибитый гвоздями к стене» (с) Джоэл Спольски.
dkukushkin
28.07.2015 12:59+1Тема обработки исключений — одна из важнейших в девелоперстве. А вот статья слабенькая, особенно для перевода.
Даже нет предмета для обсуждений.
Хороший вопрос для обсуждения: нужны ли исключения бизнес-процесса. К примеру, если при попытке перевода средств оказалось что на счету недостаточно денег — выбрасывать исключение с деталями ошибки (нет денег/счет заблокирован и пр.) или возвращать класс, содержащий информацию об ошибке?feedbee
29.07.2015 11:14Ответ псевдокодом (субъективно):
function transferMoney(value, fromWalet, toWalet) { lock(fromWalet); // throws CantAquireLockException after timeout try { if (!fromWalet.hasFunds(value)) { throw new InsufficientFundsException(fromWalet, value); } try { SomeTransactionalSystem.begin(); fromWalet.addFunds(-value); toWalet.addFunds(value); SomeTransactionalSystem.commit(); } catch (Exception e) { SomeTransactionalSystem.rollback(); throw e; } } catch (Exception e) { unlock(fromWalet); throw e; } unlock(fromWalet); }
Таким образом transferMoney становится полностью самостоятельной процедурой, которая делает свою работу, ничего не возвращает и кидает исключения, когда не может ее сделать. Она не нуждается в блокировках на уровне выше. При этом никто не мешает на уроне выше повторить проверку fromWalet.hasFunds(value), что бы не доводить до исключения. Просто это будет вне блокировки и при параллельной работе с одним fromWalet может возникнуть ситуация, когда InsufficientFundsException все же вылетит.
или возвращать класс, содержащий информацию об ошибке?
Возвращать что угодно, содержащее сообщение об ошибке, должен только метод на подобии getError или getLastError. Есть 2 варианта — проверка (валидация) (никаких исключений и возврат true/false) и выполнение (в случае ошибки — исключение). В данном случае я бы предпочел сочетание обоих, т.е. сначала проверить fromWalet.funds < value, потом вызвать transferMoney. Проверка прямо скажет true/false
lair
29.07.2015 12:01Этот код будет феерически работать на больших нагрузках. Распределенные транзакции (а кто сказал, что у вас счета физически на одной машине) — это очень дорого, блокировки каждого счета — тоже.
feedbee
29.07.2015 12:07Этот код будет отлично работать на больших нагрузках. Потому что по одному счету не будет даже 100 обращений в секунду (если будет, то это становится центральным моментом проектирования и код нужно писать иначе, а здесь в примере — это погрешность — это код для 98% биллингов, но не для высокочастотного трейдинга). А заявление о том, что что-то дорого само по себе — ни о чем. Дорого или дешево может быть только относительно.
А спорить о транзакциях и блокировках с вами я больше не буду — это долго и бессмысленно. Проходили, знаем.lair
29.07.2015 12:11А заявление о том, что что-то дорого само по себе — ни о чем. Дорого или дешево может быть только относительно.
Это (распределенные транзакции и блокировки) дороже (с точки зрения ресурсов), чем неблокирующие системы.
trurl123
28.07.2015 13:36+3Интересный способ пиара собственного блога. Написать на английском, а потом разместить как перевод.
antarx
28.07.2015 17:49Эта статья — отличная иллюстрация, почему в Go нет и не нужны исключения: их заменяет явный возврат ошибки с их явной обработкой, и паники в случае действительно исключительных ситуаций, требующих раскручивания стека; причём при работе с библиотеками так же удобнее работать с возвращаемыми оттуда ошибками, а не исключениями.
vkhorikov Автор
28.07.2015 18:25+1Это кстати не только в Go такая тенденция. В функциональных языках тоже заметно стремление использовать контейнеры с результатом выполнения операции вместо выбрасывания исключения. Either монада как пример
Dywar
28.07.2015 22:00+1Хороший свод правил на msdn.microsoft.com Лучшие методики обработки исключений
perfectdaemon
29.07.2015 12:30Из которого выносится простое правило: если событие происходит редко, следует использовать исключения, если часто — программные проверки. Собственно, это более широкая трактовка о том, что исключения нужны в исключительных ситуациях.
JuniorIL
29.07.2015 12:43Ну а если покритиковать подход Scala, обернуть исключение в монаду, тогда мы можем знать, что попытались выполнить операцию, но возможно у нас не получилось. Кто ее распутывать будет, тот и обработает это исключение. Минус тут вижу, что в отличии от нормальной функции, тут throw уподобляется return, т.е быстрый goto в конец метода.
lawliet29
По поводу исключений ещё один интересный вопрос — производительность. try-catch вместо if — это плохо не только потому, что ухудшает читабельность кода, а ещё и потому, что это просто медленнее.
DjoNIK
А вот это заявление дискуссионное. В каком-нибудь enterprise приложении боятся бросить разовое исключение из-за потери производительности как минимум опрометчиво. Да и производительность аппаратуры сейчас позволяет пренебречь этим фактором в угоду скорости разработки.
alan008
Это медленнее можно заметить, только если вы будете выполнять миллионы if'ов или бросать миллионы исключений в секунду. В остальных «реальных» ситуациях этой разницей можно пренебречь. С подобным же энтузиазмом можно говорить про замедление времени на вызов одной функции из другой или про замедление вызова виртуального метода по сравнению с обычным. А потом смотришь — в коде сплошной boxing/unboxing, а они исключение боятся бросить. Ответ тут один — профилируйте код, и вам откроется очень много интересного (то, что тормозит совсем не там, где вы думаете).
MacIn
Нет, дело не в «бросании», а в самой постановке обработчика, если это, например, Windows SEH обработка. Но да — заметно только в циклах.
mayorovp
Постановка обработчика SEH — это всего лишь две операции со стеком. Снятие обработчика — еще две.
Дорогой является именно процедура возбуждения исключения, включающая в себя системный вызов и еще кучу нетривиальных действий.
MacIn
Да, но их могло и не быть. Комментарий, с которого началась ветка, содержит
Заметьте, сказано не о создании исключения, а о блоке. Мой комментарий написан именно в этом контексте — «if против try..catch», а не «try..catch затратнее/менее затратен, чем throw»
Это если исключение «честное» и проходит через ядро.
mayorovp
Исключать цену throw из цены try/catch сравнивая try/catch и if — некорректно. Ведь в гипотетической программе, написанной с использованием первой конструкции, конструкция throw также будет присутствовать — ведь без нее try/catch не сможет заменить if, в альтернативной же программе, построенной на if-ах, конструкции throw быть не может.
MacIn
Я не исключал, вы что-то путаете. Это была ремарка про «нормальное» выполнение без исключений и сопутствующие накладные расходы. А так — каждому случаю — свой инструмент.