Есть ли случай, когда проверка параметров может считаться избыточной? - PullRequest
4 голосов
/ 07 февраля 2009

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

private List<Item> m_items = ...;

public Item GetItemByIdx( int idx )
{
    if( (idx < 0) || (idx >= m_items.Count) )
    {
        throw new ArgumentOutOfRangeException( "idx", "Invalid index" );
    }

    return m_items[ idx ];
}

В этом случае параметр index напрямую связан с индексами в списке, и я точно знаю (например, документация), что сам список будет делать то же самое и будет выдавать то же исключение. Должен ли я удалить это подтверждение или лучше оставить его в покое?

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

Заранее спасибо.

Ответы [ 3 ]

6 голосов
/ 07 февраля 2009

Это не просто вопрос вкуса, рассмотрим

if (!File.Exists(fileName)) throw new ArgumentException("...");            
var s = File.OpenText(fileName);

Это похоже на ваш пример, но есть несколько причин (параллелизм, права доступа), почему метод OpenText() может все еще не работать, даже с ошибкой FileNotFound. Таким образом, проверка Exists просто дает ложное чувство безопасности и контроля.

Это умонастроение, когда вы пишете метод GetItemByIdx, это, вероятно, выглядит довольно разумно. Но если вы просматриваете случайный фрагмент кода, обычно есть много предположений, которые вы можете проверить, прежде чем продолжить. Просто не практично проверять их все снова и снова. Мы должны быть избирательными.

Так что в простом методе передачи, таком как GetItemByIdx, я бы поспорил с избыточными проверками. Но как только функция добавляет больше функциональности или если есть очень явная спецификация, которая говорит что-то о idx, этот аргумент оборачивается.

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

3 голосов
/ 07 февраля 2009

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

1 голос
/ 07 февраля 2009

Это правда, что, возможно, вы дублировали работу, которая уже была выполнена в API, но сейчас она есть. Если ваша структура обработки ошибок работает и надежна и не вызывает проблем с производительностью ( профилирование IYF ), тогда я считаю, что оставлю ее и постепенно откажусь от нее, если у вас будет время. Это не похоже на главный приоритет!

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