Бросать исключение или возвращать коды ошибок/успеха? Является ли исключением то, что метод не может выполнить свою задачу?

Question

[Алексей Потапенко]

Исключения являются для меня самой непонятной темой в программировании(пока что). Даже прочтение кучи информации не помогло мне получить чёткое представление о том как ими ПРАВИЛЬНО пользоваться.
В поле "Суть вопроса" сложно передать суть вопроса? Поэтому нужно прочитать всё что ниже :)
Простите за много букв.

Допустим есть метод transferMoney($amount, User $recipient) и мы считаем, что любой user input является ожидаемым(даже строка с буквами). А он может быть любым из-за того, что пользователь может отправить запрос с любыми данными в теле прямиком на сервер, также некорректные данные может отправить недоработанное клиентское приложение. Проверить отправлен ли вообще параметр amount на сервер и является ли он числом можно где-нибудь выше метода transferMoney(). Но узнать является ли сумма(amount) корректной по-хорошему можно только в браузере/GUI или в Domain слое в самом методе transferMoney(). А поскольку пользователь может обойти и браузер и GUI, то это может сделать только transferMoney(). В transferMoney() нужно извлечь данные пользователя(который пересылает деньги) из БД и проверить есть ли у него такая сумма. Получается в методе transferMoney() есть одна причина неудачи, а что если добавится ещё одна причина, например, пользователю запрещено временно пересылать деньги? Таким образом получается, что из метода в случае неудачи нужно возвратить причину неудачи, этих причин уже может быть две, но при этом они являются вполне ожидаемыми если смотреть на общий контекст(мы ожидаем некорректные данные). Поскольку они ожидаемые, то бросать исключения не логично. В случае если бы была одна причина неудачи, то можно было бы просто вернуть false. Но поскольку их две, то получается, что нужно возвращать коды ошибок(или код успеха в случае успеха) либо возвращать что-то на подобии [false, $error] и [true, null](я подобное видел в очень популярной книге про PHP, только там вместо null была запись из БД).

Но есть и другой подход к проектированию(вроде как): метод transferMoney() является частью ядра приложения, а в Clean Architecture ясно сказано, что ядро это основа вокруг которой строится приложение. То есть, если я правильно понимаю, то transferMoney() не должен подстраиваться под слой повыше, в нём не должно быть никаких предположений, что пользователь введёт что-то правильно или неправильно, он просто должен ожидать только валидные данные, если они неправильные, то это исключительная ситуация. То есть transferMoney() выбросит исключение, которое подымется до Controller'a, и на основе которого Controller отправит 400 Bad Request и причину неудачи. Я правильно размышляю? Или всё таки первый подход к проблеме лучше(первый абзац)?
Я в интернете нашел вот такую вот мысль: "A good rule of thumb is that if a method does not do what its name suggests, it should be considered a method-level failure, resulting in an exception".

Хочу заметить, что я еще не сталкивался с потребностью реализации такого метода, меня сейчас интересует только работа с ошибками.

Answer 1

[Adamos]

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

Comments

[Алексей Потапенко]

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

А когда произошло исключение, после которого есть смысл продолжать работу, то его лучше обработать максимально близко к месту выброса? Да? Вот так вот как-то:

function getData() {
    try {
          return file_get_contents('http://webservice......');
    } catch (ErrorException $ex) {
         try {
               return file_get_contents('http://web_service......');
         } catch (ErrorException $ex) {
               throw new SpecificRuntimeException('bla bla bla');
         }  
    }
}

Если один ресурс не работает, то пробуем второй, если второй не пашет, то бросаем более конкретное исключение, чтобы Controller его словил и как-то по-особенному его обработал, например вместо неполученных данных заглушки какие-то отправить или надпись "Не удалось загрузить данные". Верно?

[Adamos]

Алексей Потапенко, а что вы собрались ловить от file_get_contents? Это сишная функция, она не выбросит никаких исключений, молча вернет false - и все.
Соберите свои урлы в массив и перебирайте его в цикле. Если ни один не вернет осмысленного - тогда решайте, как дальше жить.

[Алексей Потапенко]

Adamos, ок, я понял, спасибо

Answer 2

[index0h]

Бросать исключение или возвращать коды ошибок/успеха?

Бросать исключение.

Является ли исключением то, что метод не может выполнить свою задачу?

Конечно же.

Но узнать является ли сумма(amount) корректной по-хорошему можно только в браузере/GUI или в Domain слое в самом методе transferMoney().

К фронту не должно быть доверия, данные обязательно нужно проверять.
Аргументы метода лучше проверять И на тип И на граничные значения. Например ваш amount по идее должен быть float + больше, или равен 0. Если amount таковым не является - бросайте исключение. Логика выше должна была это отсечь, еще на этапе валидации данных запроса.

В transferMoney() нужно извлечь данные пользователя(который пересылает деньги) из БД и проверить есть ли у него такая сумма.

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

Получается в методе transferMoney() есть одна причина неудачи, а что если добавится ещё одна причина, например, пользователю запрещено временно пересылать деньги?

Тот же комментарий, что и про баланс.

Поскольку они ожидаемые, то бросать исключения не логично.

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

В случае если бы была одна причина неудачи, то можно было бы просто вернуть false.

Очень хреновая практика. Как вы определите в вызывающем коде, что пошло не так? Что "false"? Баланса не хватает, нельзя делать покупки, или Меркурий в ретрограде?

возвращать что-то на подобии [false, $error] и [true, null]

Для php - это очень кривой подход. По двум причинам:
1. Вы расширяете интерфейс метода, просто так.
2. Вы нагружаете вызывающий код дополнительными обвязками проверок, опять же просто так.
В этом нет смысла так как есть механизм try-catch, который отлично решает это задачу.
Вот пример, допустим ваш transferMoney($amount, User $recipient) должен возвращать объект транзакции, пусть его сигнатура будет:
transferMoney(float $amount, User $recipient): Transaction
Вызывающий код знает, что он обязан в конце получить транзакцию, а если что-то пойдет не так - будет исключение, нет смысла в дополнительных проверках, а что если первый элемент массива false, а что если он true, но второй элемент не null, а что если второй элемент не тот, что ожидалось, а что если первый элемент - строка, и т.д.

transferMoney() не должен подстраиваться под слой повыше, в нём не должно быть никаких предположений, что пользователь введёт что-то правильно или неправильно, он просто должен ожидать только валидные данные, если они неправильные, то это исключительная ситуация.

Верно

То есть transferMoney() выбросит исключение, которое подымется до Controller'a, и на основе которого Controller отправит 400 Bad Request и причину неудачи.

Угу

Или всё таки первый подход к проблеме лучше(первый абзац)?

Первый подход - это путь боли, ошибок и отчаянья.

Вот вам еще чтива: https://github.com/index0h/php-conventions

Comments

[Алексей Потапенко]

Спасибо за ответ

Answer 3

[Daria Motorina]

А поскольку пользователь может обойти и браузер и GUI

Пользователь не должен уметь обходить, он должен иметь gui и/или api)

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

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

Comments

[Adamos]

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

[Алексей Потапенко]

То есть лучше провести валидацию суммы и проверку прав перед вызовом transferData()? А сам transferData() будет также проводить валидацию, но это уже будет последним рубежом защиты, где будут выбрасываться критические исключения сигнализирующие о косяках в коде?

[Daria Motorina]

Алексей Потапенко, по идее да, но исключенмя должны касаться ошибок бизнес-логики, косяки кода должны быть устранены заранее (т.е. если они вылезли - это очень большой косяк qa, дева и код-ревьюера, за который ловят по шапке)

[Алексей Потапенко]

Daria Motorina, ну я под косяками имел в виду, что если до метода, например, данные доходят в некорректном состоянии, то метод бросает LogicException(к примеру InvalidArgumentException), то есть разработчик или не добавил проверку либо забыл добавить, из-за которого до метода данные дошли в виде, в котором они не должны были доходить. И её нужно исправлять. Но вообще такие ошибки должны отсекаться еще на стадии тестирования. Верно?

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

[Daria Motorina]

Алексей Потапенко, верно, правильно)

[Алексей Потапенко]

Daria Motorina, спасибо за ответы

Answer 4

[xfg]

Исключения и есть современный способ реакции программы на ошибки, которые приводят к бессмысленности дальнейшего её выполнения. Соответственно, если у пользователя недостаточно средств для перевода, дальнейшее выполнение программы не имеет смысла, необходимо прервать её выполнение выбросив исключение. Когда этого механизма не существовало использовали коды ошибок.

Вы также всегда имеете возможность посмотреть популярные open-source решения и убедиться какой точки зрения придерживается сообщество.

Comments

[Алексей Потапенко]

Спасибо за ответ

Answer 5

[Евгений Шерман]

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

Контроллер - управляет приложением
Формы - выполняют валидацию данных
Модель - основная логика, решение задачи бизнеса.

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

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

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