Параметр для контроля, выдавать ли исключение или возвращать ноль - хорошая практика?

24

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

Уже ведутся дискуссии о том, какой из них является лучшим выбором в этом случае, поэтому давайте не будем здесь останавливаться на этом. Смотрите, например, Возвращение магического значения, выбросить исключение или вернуть ложь при ошибке?

Давайте вместо этого предположим, что есть веская причина, почему мы хотим поддерживать оба пути.

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

Итак, что лучше?

A: Один метод с $exception_on_failureпараметром.

/**
 * @param int $id
 * @param bool $exception_on_failure
 *
 * @return Item|null
 *   The item, or null if not found and $exception_on_failure is false.
 * @throws NoSuchItemException
 *   Thrown if item not found, and $exception_on_failure is true.
 */
function loadItem(int $id, bool $exception_on_failure): ?Item;

Б: Два разных метода.

/**
 * @param int $id
 *
 * @return Item|null
 *   The item, or null if not found.
 */
function loadItemOrNull(int $id): ?Item;

/**
 * @param int $id
 *
 * @return Item
 *   The item, if found (exception otherwise).
 *
 * @throws NoSuchItemException
 *   Thrown if item not found.
 */
function loadItem(int $id): Item;

РЕДАКТИРОВАТЬ: C: Что-то еще?

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

Заметки

На случай, если кому-то интересно: примеры на PHP. Но я думаю, что вопрос применим ко всем языкам, если они чем-то похожи на PHP или Java.

Дон Кихот
источник
5
Хотя я должен работать на PHP и довольно опытный, это просто плохой язык, и его стандартная библиотека повсюду. Прочитайте eev.ee/blog/2012/04/09/php-a-fractal-of-bad-design для справки. Поэтому неудивительно, что некоторые разработчики PHP принимают плохие привычки. Но я еще не видел этот специфический дизайнерский запах.
Полигном
Я упускаю важный момент в данных ответах: когда вы будете использовать один или другой. Вы должны использовать метод исключения в случае, если элемент, который не был найден, будет неожиданным и считается ошибкой (время для паники). Вы бы использовали метод Try ... в случае, если отсутствующий элемент не является маловероятным и, конечно, не ошибкой, а просто возможностью, которую вы включаете в свой обычный контроль потока.
Мартин Маат
Связанный: stackoverflow.com/questions/34439782/…
Мартин Маат
1
@Polygnome Вы правы, стандартные библиотечные функции повсюду, много плохого кода существует, и существует проблема «один процесс на запрос». Но это становится лучше с каждой версией. Типы и объектная модель делают все или большинство вещей, которые можно ожидать от нее. Я хочу видеть дженерики, ко- и контравариантность, типы обратного вызова, которые определяют сигнатуру и т. Д. Многое из этого обсуждается или находится в процессе реализации. Я думаю, что общее направление хорошее, хотя стандартные особенности библиотеки не исчезнут легко.
Donquixote
4
Одно предложение: вместо того loadItemOrNull(id), подумайте, loadItemOr(id, defaultItem)имеет ли смысл для вас. Если элемент является строкой или числом, это часто происходит.
user949300

Ответы:

35

Вы правы: два метода намного лучше , что, по нескольким причинам:

  1. В Java сигнатура метода, который потенциально генерирует исключение, будет включать это исключение; другой метод не будет. Это делает особенно ясным, что ожидать от одного и другого.

  2. В таких языках, как C #, где сигнатура метода ничего не говорит об исключениях, публичные методы по-прежнему должны документироваться, и такая документация включает исключения. Документирование одного метода не будет легким.

    Ваш пример идеален: комментарии во втором фрагменте кода выглядят намного яснее, и я бы даже коротко сказал «Элемент, если найден (исключение в противном случае)». Вплоть до «Элемент.» - наличие потенциального исключения и описание, которое вы дали ему, говорит само за себя.

  3. В случае одного метода есть несколько случаев, когда вы хотите переключить значение логического параметра во время выполнения , и если вы это сделаете, это будет означать, что вызывающая сторона должна будет обрабатывать оба случая ( nullответ и исключение) ), делая код гораздо сложнее, чем нужно. Поскольку выбор сделан не во время выполнения, а при написании кода, два метода имеют смысл.

  4. Некоторые платформы, такие как .NET Framework, создали соглашения для этой ситуации, и они решают ее двумя способами, как вы и предлагали. Единственное отличие состоит в том, что они используют шаблон, объясненный Юаном в его ответе , поэтому int.Parse(string): intвыдает исключение, а int.TryParse(string, out int): boolне делает. Это соглашение об именах является очень сильным в сообществе .NET и должно соблюдаться всякий раз, когда код соответствует ситуации, которую вы описываете.

Арсений Мурзенко
источник
1
Спасибо, это ответ, который я искал! Цель состояла в том, чтобы я начал обсуждение этого для CMS Drupal, drupal.org/project/drupal/issues/2932772 , и я не хочу, чтобы это касалось моих личных предпочтений, но подкрепите это отзывами других.
Donquixote
4
На самом деле давняя традиция, по крайней мере, для .NET - НЕ использовать два метода, а вместо этого выбрать правильный выбор для правильной задачи. Исключение составляют исключительные обстоятельства, а не обработка ожидаемого потока управления. Не удалось разобрать строку в целое число? Не очень неожиданное или исключительное обстоятельство. int.Parse()считается действительно плохим дизайнерским решением, именно поэтому команда .NET пошла дальше и внедрила TryParse.
Voo
1
Предоставить два метода вместо того, чтобы думать о том, с какими вариантами использования мы имеем дело, - самый простой выход: не думайте о том, что вы делаете, вместо этого возложите ответственность на всех пользователей вашего API. Конечно, это работает, но это не является отличительной чертой хорошего дизайна API (или любого
Voo
@ Voo Действительная точка. Действительно, предоставление двух методов делает выбор вызывающего. Возможно, для некоторых значений параметров элемент должен существовать, а для других значений параметров несуществующий элемент считается обычным случаем. Возможно, компонент используется в одном приложении, где элементы всегда должны существовать, и в другом приложении, где несуществующие элементы считаются нормальными. Возможно, звонящий звонит ->itemExists($id)перед звонком ->loadItem($id). Или, возможно, это просто выбор, сделанный другими людьми в прошлом, и мы должны принять это как должное.
Donquixote
1
Причина 1b: Некоторые языки (Haskell, Swift) требуют, чтобы в подписи присутствовала обнуляемость. В частности, Haskell имеет совершенно другой тип для значений NULL ( data Maybe a = Just a | Nothing).
Джон Дворжак
9

Интересным вариантом является язык Swift. В Swift вы объявляете функцию как «throwing», то есть ей разрешено выбрасывать ошибки (похожие, но не совсем такие, как, скажем, исключения Java). Вызывающая сторона может вызывать эту функцию четырьмя способами:

а. В операторе try / catch перехват и обработка исключения.

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

с. Обозначение вызова функции try!означает «Я уверен, что этот вызов не будет сброшен». Если вызываемая функция действительно выбрасывает, приложение гарантированно завершится сбоем.

д. Обозначение вызова функции try?означает «Я знаю, что функция может выдать, но мне все равно, какая именно ошибка выдается». Это изменяет возвращаемое значение функции с типа " T" на тип " optional<T>", и выброшенное исключение превращается в возвращающий ноль.

(a) и (d) - случаи, о которых вы спрашиваете. Что если функция может вернуть nil и может выдавать исключения? В этом случае тип возвращаемого значения будет " optional<R>" для некоторого типа R, и (d) изменит это на " optional<optional<R>>", что немного загадочно и трудно понять, но вполне нормально. И функция Swift может возвращать optional<Void>, так что (d) может использоваться для броска функций, возвращающих Void.

gnasher729
источник
2

Мне нравится bool TryMethodName(out returnValue)подход.

Это дает вам окончательное указание на то, успешно ли выполнен метод, и совпадает ли наименование, заключая метод выброса исключения в блок try catch.

Если вы просто возвращаете ноль, вы не знаете, если это не удалось или возвращаемое значение было законно нулевым.

например:

//throws exceptions when error occurs
Item LoadItem(int id);

//returns false when error occurs
bool TryLoadItem(int id, out Item item)

пример использования (вне означает, что передача по ссылке не инициализирована)

Item i;
if(TryLoadItem(3, i))
{
    Print(i.Name);
}
else
{
    Print("unable to load item :(");
}
Ewan
источник
Извините, вы потеряли меня здесь. Как будет bool TryMethodName(out returnValue)выглядеть ваш « подход» в исходном примере?
Donquixote
отредактировано, чтобы добавить пример
Ewan
1
Таким образом, вы используете параметр по ссылке вместо возвращаемого значения, так что у вас есть возвращаемое значение бесплатно для успешного завершения bool? Есть ли язык программирования, который поддерживает это ключевое слово "out"?
Donquixote
2
да, извините, не понял, что 'out' является специфическим для c #
Ewan
1
не всегда нет. Но что, если пункт 3 равен нулю? Вы не знали бы, была ли ошибка или нет
Ewan
2

Обычно логический параметр указывает, что функцию можно разделить на две части, и, как правило, вы всегда должны разбивать ее, а не передавать логическую.

Максимум
источник
1
Я бы не стал этого исключать, как правило, не добавив уточнения или нюанса. Удаление логического значения в качестве аргумента может заставить вас писать тупой if / else в каком-то другом месте и, таким образом, вы будете кодировать данные в своем написанном коде, а не в переменных.
Джерард
@ Джерард, можешь привести пример?
Макс.
@Max Когда у вас есть логическая переменная b: вместо вызова foo(…, b);вы должны писать if (b) then foo_x(…) else foo_y(…);и повторять другие аргументы или что-то вроде того, ((b) ? foo_x : foo_y)(…)если в языке есть троичный оператор и функции / методы первого класса. И это может быть даже повторено из нескольких разных мест в программе.
Блэкджек,
@ BlackJack хорошо, да, я могу согласиться с вами. Я подумал о примере, if (myGrandmaMadeCookies) eatThem() else makeFood()который да, я мог бы обернуть в другую функцию, как это checkIfShouldCook(myGrandmaMadeCookies). Интересно, связан ли упомянутый вами нюанс с тем, передаем ли мы логическое значение того, как мы хотим, чтобы функция работала, как в первоначальном вопросе, по сравнению с тем, как мы передаем некоторую информацию о нашей модели, как в Пример бабушки, который я только что привел.
Макс.
1
Нюанс, о котором говорится в моем комментарии, относится к «ты должен всегда». Может быть, перефразировать это как «если a, b и c применимы, то [...]». Упомянутое вами «правило» - отличная парадигма, но очень вариант для варианта использования.
Джерард
1

Чистый код советует избегать аргументов логических флагов, потому что их значение непрозрачно на сайте вызова:

loadItem(id, true) // does this throw or not? We need to check the docs ...

тогда как отдельные методы могут использовать выразительные имена:

loadItemOrNull(id); // meaning is obvious; no need to refer to docs
меритон - забастовка
источник
1

Если бы мне пришлось использовать A или B, я бы использовал B, два отдельных метода, по причинам, указанным в ответе Арсения Мурзенкоса .

Но есть еще один метод 1. В Java он вызывается Optional, но вы можете применить ту же концепцию к другим языкам, где вы можете определять свои собственные типы, если язык еще не предоставляет подобный класс.

Вы определяете свой метод так:

Optional<Item> loadItem(int id);

В вашем методе вы либо возвращаете, Optional.of(item)если нашли предмет, либо возвращаете, если не нашли Optional.empty(). Вы никогда не бросаете 2 и никогда не возвращаетесь null.

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

  • loadItem(1).get()скинет NoSuchElementExceptionили вернет найденный предмет.
  • Также есть loadItem(1).orElseThrow(() -> new MyException("No item 1"))возможность использовать пользовательское исключение, если возвращаемое значение Optionalпустое.
  • loadItem(1).orElse(defaultItem)возвращает либо найденный элемент, либо пропущенный defaultItem(что также может быть null) без исключения.
  • loadItem(1).map(Item::getName)будет снова возвращать Optionalс именем элементов, если присутствует.
  • Есть еще несколько методов, см. JavadocOptional .

Преимущество состоит в том, что теперь все зависит от клиентского кода, что должно произойти, если элемента нет, а вам просто нужно предоставить один метод .


1 Я думаю, что это что-то вроде ответаtry? in gnasher729s, но он мне не совсем понятен, так как я не знаю Swift, и кажется, что это языковая функция, поэтому она специфична для Swift.

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

siegi
источник
0

Как еще один момент, согласившись с использованием двух методов, это может быть очень легко реализовать. Я напишу свой пример на C #, так как я не знаю синтаксис PHP.

public Widget GetWidgetOrNull(int id) {
    // All the lines to get your item, returning null if not found
}

public Widget GetWidget(int id) {
    var widget = GetWidgetOrNull(id);

    if (widget == null) {
        throw new WidgetNotFoundException();
    }

    return widget;
}
krillgar
источник
0

Я предпочитаю два метода, таким образом, вы не только скажете мне, что вы возвращаете значение, но и какое именно значение.

public int GetValue(); // If you can't get the value, you'll throw me an exception
public int GetValueOrDefault(); // If you can't get the value, you'll return me 0, since it's the default for ints

TryDoSomething () тоже неплохой шаблон.

public bool TryParse(string value, out int parsedValue); // If you return me false, I won't bother checking parsedValue.

Я больше привык видеть первое в взаимодействиях с базой данных, а второе больше используется для разбора.

Как уже говорили другие, параметры флагов обычно являются запахом кода (запахи кода не означают, что вы делаете что-то не так, просто есть вероятность, что вы делаете это неправильно). Хотя вы называете его точно, иногда есть нюансы, которые затрудняют понимание того, как использовать этот флаг, и вы в конечном итоге заглядываете внутрь тела метода.

Браво Дев
источник
-1

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

Thing getThing(string x);
Thing tryGetThing (string x);

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

Thing getThingWithBrackets(string x)
{
  getThing("["+x+"]");
}
Thing tryGetThingWithBrackets (string x);
{
  return tryGetThing("["+x+"]");
}

Если есть аргумент о том, как обрабатывать ошибки, потребуется только одна оболочка:

Thing getThingWithBrackets(string x, bool returnNullIfFailure)
{
  return getThing("["+x+"]", returnNullIfFailure);
}

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

Thing getThingWithBrackets(string x)
{
   return getThingWithBrackets(x, false);
}
Thing tryGetThingWithBrackets(string x)
{
   return tryGetThingWithBrackets(x, true);
}

Можно хранить всю «бизнес-логику» в одном методе, который принимает аргумент, указывающий, что делать в случае сбоя.

Supercat
источник
Вы правы: декораторы и обертки, и даже обычная реализация, будут иметь некоторое дублирование кода при использовании двух методов.
Donquixote
Разумеется, помещать версии с и без исключения в отдельные пакеты и делать обертки общими?
Уилл Кроуфорд
-1

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

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

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

Пример из реальной жизни для меня: математическая библиотека. Если в вашей математической библиотеке есть Vectorкласс, который поддерживает получение единичного вектора в одном и том же направлении, вы должны иметь возможность делить на величину. Если величина равна 0, у вас есть ситуация деления на ноль. В разработке вы хотите поймать это . Вы действительно не хотите удивительного деления на 0, торчащие вокруг. Бросок исключения - очень популярное решение для этого. Это почти никогда не происходит (это действительно исключительное поведение), но когда это происходит, вы хотите знать это.

На встроенной платформе вы не хотите этих исключений. Было бы больше смысла сделать проверку if (this->mag() == 0) return Vector(0, 0, 0);. На самом деле, я вижу это в реальном коде.

Теперь подумайте с точки зрения бизнеса. Вы можете попытаться научить двум различным способам использования API:

// Development version - exceptions        // Embeded version - return 0s
Vector right = forward.cross(up);          Vector right = forward.cross(up);
Vector localUp = right.cross(forward);     Vector localUp = right.cross(forward);
Vector forwardHat = forward.unit();        Vector forwardHat = forward.tryUnit();
Vector rightHat = right.unit();            Vector rightHat = right.tryUnit();
Vector localUpHat = localUp.unit()         Vector localUpHat = localUp.tryUnit();

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

Если, с другой стороны, у вас есть флаг, который включает или отключает обработку исключений, обе группы могут изучить один и тот же стиль кодирования. Фактически, во многих случаях вы можете даже использовать код одной группы в проектах другой группы. В случае использования этого кода unit(), если вы действительно заботитесь о том, что происходит в случае деления на 0, вы должны были написать тест самостоятельно. Таким образом, если вы перемещаете его во встроенную систему, где вы просто получаете плохие результаты, такое поведение является «нормальным». Теперь, с точки зрения бизнеса, я обучаю своих программистов один раз, и они используют одни и те же API и одинаковые стили во всех частях моего бизнеса.

В подавляющем большинстве случаев вы хотите следовать советам других: используйте идиомы, подобные tryGetUnit()или похожие, для функций, которые не генерируют исключения, и вместо этого возвращайте значение часового типа, например, null. Если вы думаете, что пользователи могут захотеть использовать как обработку исключений, так и код обработки не исключений бок о бок в одной программе, придерживайтесь tryGetUnit()обозначений. Тем не менее, есть угловой случай, когда реальность бизнеса может сделать использование флага лучше. Если вы окажетесь в этом угловом деле, не бойтесь отбросить «правила» на второй план. Вот для чего нужны правила!

Корт Аммон - Восстановить Монику
источник