Привет, Хабр! Есть несколько способов проверять аргументы на правильность. Например, для проверки на null можно использовать:


  1. if (!ReferenceEquals(arg, null)) throw…
  2. Code Contracts: Contract.Requires(!ReferenceEquals(arg, null))
  3. Guard.IsNotNull(arg, nameof(arg))

В статье я рассмотрю только третий вариант (все примеры кода — для C#, однако некоторые из них будут полезны и в Java).


Ошибка №1: не подготовились к проверкам аргументов и в теле метода


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


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


Итак, для начала лучше всего заранее договориться об именованиях обоих случаев. Например, Guard.IsNotNull для тела метода и Guard.ArgumentIsNotNull для аргументов


Ошибка №2: вызов string.format при каждой проверке


Сразу примеры ошибочного кода:


Guard.IsNotNull(connection, $"Unable to setup connection with host {host} and port {port}")
Guard.IsNotNull(connection, string.Format("Unable to setup connection with host {0} and port {1}", host, port)) // это просто развернутая строчка выше

Оба примера выше генерят строку на каждую проверку (в теории, все Guard не бросают исключений на боевом сервере, то есть мы генерим немало строк просто для того, чтобы их потом съел сборщик мусора).


Исправленный вариант:


public static class Guard
{
    public static void IsNotNull(object value, string format, params object[] formattingArgs) // тут мы сконструируем строку в самый последний момент, когда выделение небольшого куска памяти уже не будет ударять по производительности
}

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


Самое обидное, что программа будет тормозить, однако простой профайлинг не подстветит проблему. У вас просто программа будет чаще останавливаться для очистки памяти (я исправил такую ошибку в одной из программ, сборка мусора стала занимать вместо прежних 15% всего лишь 5%).


Итак, поступаем так же, как сделано в string.Format: нагенерим побольше методов для разного числа аргументов.


public static class Guard
{
    public static void IsNotNull(object value, string errorMessage)

    public static void IsNotNull(object value, string errorMessageFormat, object arg1)

    public static void IsNotNull(object value, string errorMessageFormat, object arg1, object arg2)

    public static void IsNotNull(object value, string errorMessageFormat, object arg1, object arg2, object arg3)
}

Итак, теперь массив выделяться не будет. Однако, мы автоматом получили новую проблему — Boxing.
Рассмотрим вызов метода: Guard.IsNotNull(connection, "Unable to setup connection with host {0} and port {1}", host, port). Переменная port имеет тип int (чаще всего по крайней мере).


Получается, что для того, чтобы передать переменную по значению, .Net каждый раз будет создавать int в куче, чтобы передать его как object. Эта ситуация будет встречаться намного реже, но всё же будет.


И другая проблема — если изначальный проверяемый объект — это value type (например, мы проверяем на null в generic методе, который не имеет ограничений на тип).
Исправить это можно созданием Generic методов для проверок:


public static class Guard
{
    public static void IsNotNull<TObject>(TObject value, string errorMessage)

    public static void IsNotNull<TObject, TArg1>(TObject value, string errorMessageFormat, TArg1 arg1)

    public static void IsNotNull<TObject, TArg1, TArg2>(TObject value, string errorMessageFormat, TArg1 arg1, TArg2 arg2)

    public static void IsNotNull<TObject, TArg1, TArg2, TArg3>(TObject value, string errorMessageFormat, TArg1 arg1, TArg2 arg2, TArg3 arg3)
}

Ошибка №3: отсутствие кодогенерации


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


  1. Создать два набора функций — для проверки аргументов и для проверок в теле метода
  2. В каждом наборе — создать кучу дубликатов, который будут содержать один и тот же код

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


Еще улучшения


Пункты ниже не ускорят вашу программу, а просто незначительно улучшат читаемость


ReSharper Annotations


Часто ReSharper ругается, что значение может быть null, хотя его вроде бы проверили с помощью Guard'а. В этом случае можно либо начать постепенно забивать на предупреждения в коде (что может быть чревато), либо объяснить проверяющим, что всё нормально. Полный список аттрибутов можно просмотреть здесь, однако вот полезные для нас:


  • AssertionMethodAttribute и AssertionConditionAttribute — они вдвоем объяснят системе, что метод только проверяет аргумент, а заодно и распишут, что именно проверяют
  • NoEnumerationAttribute — покажет, что если передать на вход IEnumerable, то по нему никто не будет итерироваться
  • CollectionAccessAttribute — если вы вдруг решили проверить все аргументы коллекции (например, что их не больше пяти, и что они не null), то с помощью этого аттрибута можно сказать, что именно происходит с коллекцией (чтение, запись и т.д.)
  • StringFormatMethodAttribute — объясняет, что один из параметров является строкой, которую потом будут использовать как формат. В этом случае её дополнительно проверят, что она может быть разобрана string.Format, что число аргументов соответствует ожиданиям и т.д. Я, кстати, еще не видел проекта, в котором была бы отключена эта проверка, и в котором после её включения не было бы ошибок, что string.Format просто не может выполниться

Записывать больше информации


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


  1. Тип (а лучше — вызов ToString у него), который оказался некорректным.
  2. Если есть еще аргументы для сравнения — то информацию о них тоже
  3. Полный stacktrace (т.к. иначе он обрезается до места, где исключение было поймано)

Заключение


Надеюсь, эта статья поможет сравнительно просто улучшить проверки в вашем коде.
Немного ранее я реализовал все наработки в библиотеке (исходный код под MIT лицензией) — так что можете просто использовать её (или скопировать код к себе и т.д.)

Поделиться с друзьями
-->

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


  1. dimaaan
    13.06.2017 13:37
    +2

    В C# 7 так же можно:


    arg = arg ?? throw new ArgumentNullException(nameof(arg));


    1. mayorovp
      13.06.2017 13:40
      +1

      Ничем не лучше обычного if (arg == null) throw new ArgumentNullException(nameof(arg));


      1. dimaaan
        13.06.2017 13:43
        +1

        Я и не говорил, что это лучше. Немного короче. На вкус и цвет...


        1. playermet
          13.06.2017 20:49
          +2

          Короче, только если длина имени переменной arg меньше шести символов.


  1. dimaaan
    13.06.2017 13:49

    Интересно было бы послушать об опыте применения AOP техник (а-ля Fody + атрибуты) для "вшивания" проверок в код.


    1. imanushin
      13.06.2017 14:01

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


      Не всегда отладчик нормально реагирует на подобный код
      Исключения часто бросаются из неожиданных мест

      В итоге от подобного подхода пришлось постепенно отказываться.


  1. shai_hulud
    13.06.2017 22:47
    +1

    Наплодить ненужных сущностей, а потом решать проблемы перфоманса, которые они породили. Великолепно. Чем «if..then throw» хуже отдельного класса? В чем преимущество?

    Все проверки на null, <0 итд отлично генерируются решарпером или свежей студией с расширениями.


    1. imanushin
      14.06.2017 11:31

      shai_hulud, статья не о споре между .Net и Java разными способами проверок. А для тех, кто уже почему-то выбрал способ использования подобного рода Guard, Preconditions и хочет их использовать удобнее.


      Я спорить об этом не буду, однако, если ты прав, то ты сможешь убедить владельцев репозиториев удалить следующие два класса:


      • Preconditions из Guava
      • Validation от одного из популярных Open Source разработчиков


      1. shai_hulud
        14.06.2017 11:37

        Я уже успел поспорить, услышать другие мнения и вообще обсудить Guard классы в этой статье.

        Open Source не святой грааль, туда попадают решения с запашком. Мое личное ИМХО что большая часть Open Source это решения с запашком, и только малая часть может быть примером другим разработчикам.

        Validation — 39 звезд. meh


        1. imanushin
          14.06.2017 13:25

          Еще раз: есть разные мнения, разные проекты и предпочтения.


          Я не спорю, что ты придерживаешься политики отсутствия Guard классов. Я — нет.


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


          Open Source не святой грааль, туда попадают решения с запашком. Мое личное ИМХО что большая часть Open Source это решения с запашком, и только малая часть может быть примером другим разработчикам.

          Я не берусь это оценивать. Раз люди используют решение, значит оно им подходит (или лучше, в сравнении с остальными).
          И Google Guava имеет 17к звезд. Если бы действительно Predonditions были бы абсолютно плохими, то их бы удалили. Верно?


          1. shai_hulud
            14.06.2017 13:55

            >И Google Guava имеет 17к звезд. Если бы действительно Predonditions были бы абсолютно плохими, то их бы удалили. Верно?
            Нет не верно. Вот, к примеру, неработающий проект вебсокетов, не поддерживает большие сообщения, кривой логгинг, жуткий мемори трафик и не работает под моно и не поддерживается — 600 звезд которые заработал в первые месяцы. А вот аналог полный фич и работающий — 200 звезд за все время существования. О чем это говорит? Чем известнее разработчик, тем больше звезд. Качество проекта становится вторичным.

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


            1. Serg046
              14.06.2017 15:02

              По классам с проверками нет единого мнения даже в .NET фреймворке, часть библиотек написано с ними, часть без них.

              К счастью, это может поменяться — Champion "Method Contracts"


    1. igormich88
      14.06.2017 13:46
      +1

      Или в случае с Java @NonNull из Lombok может быть удобней чем Guard.