Должен ли сервис генерировать исключение или возвращаться, если для удаления не задано

12

У меня есть кусок кода, который может быть представлен как:

public class ItemService {

    public void DeleteItems(IEnumerable<Item> items)
    {
        // Save us from possible NullReferenceException below.
        if(items == null)
            return;

        foreach(var item in items)
        {
            // For the purpose of this example, lets say I have to iterate over them.
            // Go to database and delete them.
        }
    }
}

Теперь мне интересно, если это правильный подход или я должен бросить исключение. Я могу избежать исключения, потому что возвращение будет таким же, как итерация по пустой коллекции, то есть никакой важный код не выполняется в любом случае, но, с другой стороны, я, возможно, скрываю проблемы где-то в коде, потому что почему кто-то захочет вызвать DeleteItemsс nullпараметром? Это может указывать на наличие проблемы где-то еще в коде.

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

FCin
источник
2
Это только мое личное мнение, но я всегда находил наиболее разумным, чтобы метод генерировал исключение, когда он совершает что-то непреднамеренное (исключительное). В вашем случае я бы сгенерировал исключение InvalidOperationException, если бы кто-то попытался удалить элементы null / 0.
Falgantil
1
@gnat Мой вопрос конкретно об услугах, из-за того, как они используются и какова их цель. Этот «дубликат» говорит только об общих случаях, а основной ответ даже противоречит самому себе в контексте моего точного метода.
FCin
2
Возможно ли перечисляемое быть пустым вместо нуля? Вы бы справились с этим по-другому?
JAD
13
@Falgantil Я вижу, что null заслуживает исключения, но я считаю абсурдом выбрасывать исключение в пустой список.
Кевин

Ответы:

58

Это два разных вопроса.

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

Должны ли вы принять пустую коллекцию? На мой взгляд: ДА, абсолютно. Гораздо больше усилий ограничить все вызывающие объекты непустыми коллекциями, чем сделать математически правильную вещь - даже если это удивляет некоторых разработчиков, которые сомневаются в концепции нуля.

Килиан Фот
источник
9
В любом случае, если вы собираетесь выбросить, то бросать не так уж и много полезного, AhaINoticedYouPassingInNullExceptionкогда среда выполнения уже предоставляет вам возможность бросать NullReferenceException, без написания какого-либо кода. Существует некоторая утилита (например, ваш инструмент покрытия кода скажет вам, есть ли у вас модульный тест для прохождения нулевого значения в первом случае, но не во втором), но ни одно исключение не должно быть попытано / поймано при каждом обращении к службе, потому что обычно это ошибка программиста.
Стив Джессоп
2
... вы должны немедленно отлавливать исключения, возникающие из-за отрывочных параметров, если вы знаете, что то, что вы передаете, является нулевым в некотором ожидаемом случае, и вы активно хотите, чтобы вызываемая вами функция проверяла его для вас. Как говорит Килиан в ответе, ваша общая политика может заключаться в том, чтобы запретить ноль везде, где это явно не разрешено. Тогда вы не поймаете NullReferenceExceptionили AhaINoticedYouPassingInNullExceptionгде-нибудь еще, кроме высокоуровневого кода аварийного восстановления. И этот обработчик исключений, вероятно, должен создать отчет об ошибке :-)
Стив Джессоп
2
@FCin: ваш дизайн интерфейса должен соответствовать потребностям пользователей в сервисе. Таким образом, если каждый вызывающий собирается использовать try / catch для этого, то либо этот метод, либо какой-нибудь удобный метод должен проверять и игнорировать null, потому что это то, что ваши публичные требования. Однако, как говорит Килиан, обычно лучше обрабатывать растущие нули как ошибку программирования, а не пытаться продолжать на каждом сайте вызова. В этом отношении, что если служба, для которой вызывается этот метод, называется null- контроллеры содержат шаблон для перехвата и продолжения в этом случае?
Стив Джессоп
2
... если так, то кажется, что кодовая база последовательно рассматривает отсутствующие компоненты как ожидаемое условие, которое вы должны обработать на низком уровне и продолжить. Вы можете разумно спросить: «Кто несет ответственность за предоставление услуги и перечисление, чтобы эта операция могла продолжаться?». Если ответ «кто-то», то ваша функция проверяет предварительные условия , и результатом невыполненного предварительного условия обычно будет исключение, полученное на высоком уровне, а не при каждом вызове. Если требуемый материал для операции действительно необязателен, то ваша функция обрабатывает ожидаемый вариант использования .
Стив Джессоп
2
Выражение «бросить» в явном виде ArgumentNullExceptionлучше, чем разрешить внутреннему коду генерировать NullReferenceException- просто глядя на сообщение об исключении, очевидно, что это ошибка ввода, а не логическая ошибка в теле метода на сайте выброса, а обеспечение раннего выхода означает, что вы, скорее всего, оставили другие данные в допустимом состоянии, а не частично мутировали из неожиданного нулевого значения позже.
Мирал
9

Нулевое значение

Как уже сказал @KilianFoth, придерживайтесь своей общей политики. Если это должно рассматриваться nullкак «сокращение» для пустого списка, сделайте это так.

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

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

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

Пустой список

Для DeleteItems()метода передача пустого списка фактически означает ничего не делать. Я бы позволил это в качестве аргумента, не выбрасывая исключение, а просто быстро возвращаясь.

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

Ральф Клеберхофф
источник
1

Брось исключение и обработай нули в вызывающем коде.

В качестве правила проектирования старайтесь избегать нулевых значений параметров. Это уменьшит исключения NullPointerException в целом, поскольку значения null действительно будут исключением.

Кроме того, посмотрите на остальную часть вашего кода. Если это обычный шаблон в вашем проекте, оставайтесь последовательными.

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

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

Вы можете применить этот мыслительный процесс к этой ситуации. Учитывая, что это открытый класс с открытым методом, у вас есть открытый API, и в теории нет контроля над тем, что ему передается.

Ваш код не имеет контекста о том, что его вызывает (как он должен делать), и вы ничего не можете разумно сделать с нулевым значением. Это, безусловно, будет кандидатом на ArgumentNullException.

richzilla
источник
«если в коде нет разумных действий в текущем контексте». Но я думаю, что у него разумный курс действий, который должен вернуть пустоту. Он делает то же самое, что и при передаче пустой коллекции, поэтому, если я разрешу пустую коллекцию, то почему бы мне не разрешить null.
FCin
1
@FCin Из-за пустой коллекции вы знаете, что она пуста. С nullтобой нет коллекции. Если предполагается, что вызывающий код должен предоставить коллекцию методу, значит, что-то не так, поэтому вы должны выбросить. Единственная причина рассматривать нулевое значение так же, как пустую коллекцию, состоит в том, если вы можете разумно ожидать, что вызывающий код создаст нулевые коллекции. Как отмечалось в других ответах, в большинстве случаев существо nullявляется аномалией. Если вы относитесь к ним так же, как к пустым коллекциям, вы потенциально игнорируете проблему в другом месте кода.
JAD
@FCin: также, если принять nullзначение пустым, это зависит от контекста этого метода. Где-то еще в той же кодовой базе у вас может быть параметр IEnumerableили, IContainerкоторый выполняет какую-то фильтрацию, nullможет означать «нет фильтра» (разрешить все), тогда как пустой фильтр означает, что ничего не разрешено. А в другом месте nullможет явно означать «я не знаю». Итак, учитывая, что nullэто не всегда означает, что везде везде одно и то же, будет хорошей идеей вообще не придумывать для него никакого значения, если только это значение не является необходимым или, по крайней мере, полезным для кого-то.
Стив Джессоп
0

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

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

Желаете ли вы это разрешить nullили нет, можно обсуждать, как показывают многие и многие хиты Google об этом. Это означает, что вы не получите четкого ответа, и это в некоторой степени зависит от мнения и традиций, в которых вы работаете.

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

Таким образом, вы должны решить: действительно ли работа библиотечного метода состоит в том, чтобы применять довольно большую политику, такую ​​как nullобработка? Особенно, если вашей библиотекой пользуются и другие люди (у которых может быть другая политика).

Я бы, вероятно null, допустил ошибку, допустив значения, определив и задокументировав их семантику (т. Е. В null = empty arrayданном случае), и не выбросил бы исключение, если 99% вашего другого аналогичного кода (кода стиля «библиотека») не делают это иначе. 'круглый.

Anoe
источник
1
«Действительно ли работа библиотечного метода состоит в том, чтобы применять довольно большую политику, такую ​​как обработка нуля?» - на этом пути мышления лежат утверждения и режимы отладки, которые позволяют вам помочь с применением политики, а не с ее применением, если вы этого хотите. Так что если вы собираетесь глотать nullна принципе быть либералом в том, что вы принимаете, а затем войти или даже бросали бы полезен для людей , чьи намерения должны быть строгими в том, что они проходят в, но которые испортили как их код и их модульные тесты до такой степени, что они nullвсе равно проходят .
Стив Джессоп
@ SteveJessop, я действительно не знаю, если вы спорите против духа моего ответа, или вы продолжаете в том же духе. Тем не менее, я не предлагаю просто проглотить null, но открыто заявить об этом в контракте метода, который является приемлемым (или нет, в зависимости от того, какое решение придет OP), а затем вопрос, выдавать ли исключение (или нет) решает сам.
AnoE