Guard классы — использовать или нет?11.04.2017 11:34

На днях мне довелось делать довольно крупные изменения в одном C# проекте — удаление сторонней сборки. Самое «замечательное», что львиную долю времени я потратил на изменение мест, где использовались helper’ы из этой сборки (так сказать бонус к основной функциональности).


Helper’ы такого вида: 


Guard.ArgumentNotNull(myobject, "myobject");

Такие классы встречаются очень часто, они могут оформляться в common libraries и кочевать из проекта в проект. Но нужны ли они? Распространенное мнение, что подобные guard’ы уменьшают количество строк кода и увеличивают ясность, но, как по мне, это очень субъективное мнение. Вот пример: 


class Manager(IEventProvider eventProvider, IDataSource dataSource)
{
    Guard.ArgumentNotNull(eventProvider, "eventProvider");
    Guard.ArgumentNotNull(dataSource, "dataSource");

    this.eventProvider = eventProvider;    
    this.dataSource = dataSource;    
}

class Manager(IEventProvider eventProvider, IDataSource dataSource)
{
    if (eventProvider == null)
        throw new ArgumentNullException("eventProvider");

    if (dataSource == null)
        throw new ArgumentNullException("dataSource");

    this.eventProvider = eventProvider;    
    this.dataSource = dataSource;    
}

Да количество строк кода больше, но пострадала ли ясность? Нет.


Еще одно мнение, что при необходимости выполнить проверки для большого количества зависимостей, конструкция if (…) throw превращает код в кашу. Согласен, но это симптомы проблем в дизайне, а не конструкции if (…) throw.


Интересное рассуждение на эту тему есть в блоге Марка Симана.


А что вы думаете?


P.S. Есть еще code contracts, которые в самом простом варианте могут использоваться для подобного рода проверок.

Комментарии (6)

  • 11 апреля 2017 в 10:17

    0

    ↑

    ↓

    Лично я думаю, что писать, как у вас во втором примере, не следует. Или заключайте тело if в операторные скобки, или пишите всё на одной строке — это защищает от типовой ошибки, когда к телу if что-то добавили, но в скобки не взяли.
    Ну и нетрудно заметить, что со скобками получается 3–4 строки на проверку — довод в пользу хелперов. Хотя агитировать за них и не буду, думаю, тут нет серебряной пули.


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

  • 11 апреля 2017 в 10:17

    0

    ↑

    ↓

    крупные изменения в одном C# проекте — удаление сторонней сборки.

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

  • 11 апреля 2017 в 10:18

    0

    ↑

    ↓

    Гвард позволяет реагировать на некорректные параметры единообразно, и это важно, на мой взгляд.

    Если в вашем стайлгайде прописана стандартная реакция на null аргументы, на ourofrange, на все частые кейсы — то этого вполне достаточно.

    Ну и, как обычно, любая стандартизация (в случае в гвардом) позволяет легко добавять туда логгирование, игнорирование (ну, а вдруг?), какую-нибудь хитрую обработку и т.д. входных параметров.

  • 11 апреля 2017 в 10:30

    +2

    ↑

    ↓

    C# 7 позволяет это делать в одну строчку, btw.
    this.eventProvider = eventProvider ?? throw new ArgumentNullException(nameof(eventProvider));

  • 11 апреля 2017 в 10:32

    +1

    ↑

    ↓

    Использую методы расширения
            public static void CheckArgumentNotNull(this T arg, string paramName)
            where T : class
        {
            if (arg == null)
            {
                throw new ArgumentNullException(paramName: paramName);
            }
        }

        public static void CheckArgumentNotNull(this T arg, string paramName, string message)
            where T : class
        {
            if (arg == null)
            {
                throw new ArgumentNullException(paramName: paramName, message: message);
            }
        }

        public static void CheckArgumentNotNull(this T arg, string paramName, Func message)
            where T : class
        {
            if (arg == null)
            {
                throw new ArgumentNullException(paramName: paramName, message: message());
            }
        }

        public static async Task CheckArgumentNotNull(this T arg, string paramName, Func> message)
            where T : class
        {
            if (arg == null)
            {
                throw new ArgumentNullException(paramName: paramName, message: await message());
            }
        }

        public static void CheckNotNull(this T arg, string message)
            where T : class
        {
            if (arg == null)
            {
                throw new NullReferenceException(message: message);
            }
        }

        public static void CheckNotNull(this T arg, Func message)
            where T : class
        {
            if (arg == null)
            {
                throw new NullReferenceException(message: message());
            }
        }

        public static async Task CheckNotNull(this T arg, Func> message)
            where T : class
        {
            if (arg == null)
            {
                throw new NullReferenceException(message: await message());
            }
        }

  • 11 апреля 2017 в 11:25 (комментарий был изменён)

    0

    ↑

    ↓

    Не очень знаком с C#, может быть, там это невозможно, но на мой взгляд, если функция требует, чтобы аргументы у нее были не null, нужно, в идеале, явно запретить туда передавать null на этапе компиляции. Скажем, в c++ это было бы
    Manager(IEventProvider& eventProvider, IDataSource& dataSource)
{
    this->eventProvider = eventProvider;    
    this->dataSource = dataSource;    
}
    вместо
    Manager(IEventProvider* eventProvider, IDataSource* dataSource)
{
    Guard.ArgumentNotNull(eventProvider, "eventProvider");
    Guard.ArgumentNotNull(dataSource, "dataSource");
    this->eventProvider = eventProvider;    
    this->dataSource = dataSource;    
}

    Такой подход повышает readability, переносит проверку ошибок из run-time в compile-time и заставляет программиста, который хочет вызвать этот метод, но имеет на руках лишь указатели, задуматься, может ли там быть nullptr.

© Habrahabr.ru