Дублирование проверки на null

#c_sharp #исключения #инспекция_кода #валидация

Один из методов бизнес-логики начинается с кода валидации:

public void Update(ProductDto dto)
{
    if (dto == null)
    {
        throw new ArgumentNullException(nameof(dto));
    }

    var validContext = new ValidationContext(dto);
    Validator.ValidateObject(dto, validContext);     

    //...
}


Если в метод передается null, то я выбрасываю стандартное исключение ArgumentNullException
и никакой дополнительной информации как видите не отсылаю.

Мне кажется, что блок проверки на null излишен, потому что в случае dto = null следующее
за ним выражение  var validContext = new ValidationContext(dto) так и так выбросит
то же самое исключение:





Будет ли ругаться "практика правильного дизайна", если я сокращу метод так?

public void Update(ProductDto dto)
{
    var validContext = new ValidationContext(dto);
    Validator.ValidateObject(dto, validContext);  

    //...
}

    

Ответы

Ответ 1

Да, такое изменение я бы не назвал правильным.

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


конструктор ValidationContext проверяет входящий объект на null, и бросает исключение, и
у вас до вызова этого конструктора нету другого кода, который использует dto или
делает какую-то другую полезную работу.


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

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

Чем меньше нужно держать в голове, тем лучше, и тем меньше вероятность ошибки.



Ещё одно мелкое соображение в пользу ранней проверки: если вы видите stack trace
упавшего на ArgumentNullException приложения, то ошибка скорее всего находится в методе
на один фрейм выше метода, бросившего исключение. В случае «пропуска» плохого аргумента
в глубину это соображение не работает.


Ответ 2

Внесу краткое дополнение к ответу. 

Не сложность заметить, что конструкции вида:

public void Update(ProductDto dto)
{
    if (dto == null)
    {
        throw new ArgumentNullException(nameof(dto));
    }

    //...
}


выглядят довольно громоздко и не эстетично, так и возникает желание избавиться от
них. А потому одним из способов добиться этого, может стать использование контрактов
(Code Contracts). Например, в случае их использования код выглядел бы так:

public void Update(ProductDto dto)
{
     // Определили Предусловие. Нарушение предусловия говорит о том, 
     // что клиент не прав. 
     Contract.Requires(dto != null);

    //...
}


Это не только сократит его и сделает чище, но а так же более явно выразит ваши требования.