company_banner

«Нулевой» ад и как из него выбраться

Автор оригинала: Anna Filina
  • Перевод
Значения null, при бездумном их использовании, могут сделать вашу жизнь невыносимой и вы, возможно, даже не понимаете, что именно в них причиняет такую боль. Позвольте мне объяснить.


Значения по умолчанию


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

public function insertDiscount(
  string $name,
  int $amountInCents,
  bool $isActive = true,
  string $description = '',
  int $productIdConstraint = null,
  DateTimeImmutable $startDateConstraint = null,
  DateTimeImmutable $endDateConstraint = null,
  int $paymentMethodConstraint = null
): int

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

Если вы хотите создать скидку для определённого способа оплаты, метод надо будет вызвать следующим образом:

insertDiscount('Discount name', 100, true, '', null, null, null, 5);

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

Давайте разберём этот пример аргумент за аргументом.

Что такое валидная скидка?


Мы уже выяснили, что скидка без ограничений применяется везде. Таким образом, валидная скидка содержит всё, кроме ограничений, которые мы можем добавить позже. Аргумент isActive имеет значение по умолчанию true. Следовательно, метод может быть вызван следующим образом:

insertDiscount('Discount name', 100);

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

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

То же самое касается и аргумента, отвечающего за описание. По умолчанию он является пустой строкой — это может вызвать множество проблем в месте, где вы ожидаете увидеть описание. Например, оно может быть напечатано в чеке, но поскольку оно пустое, пользователь просто увидит пустую строку рядом со строкой с суммой. Система не должна допускать, чтобы такое происходило.

Я бы переписала этот метод так:

public function insertDiscount(
  string $name,
  string $description,
  int $amountInCents,
  bool $isActive
): int

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

insertDiscount(
  'Discount name',
  'Discount description',
  100,
  Discount::STATUS_ACTIVE
);

Я также использовала константы для статуса активности скидки. Теперь не нужно смотреть на сигнатуру метода, чтобы узнать, что означает true для этого аргумента: становится очевидно, что мы создаём именно активную скидку. В дальнейшем мы сможем ещё улучшить этот метод (спойлер: с помощью объектов-значений).

Добавление ограничений


Теперь можно добавлять различные ограничения. Чтобы избежать нулевого, нулевого, нулевого ада, мы создадим отдельные методы.

public function addProductConstraint(
  Discount $discount,
  int $productId
): Discount;

public function addDateConstraint(
  Discount $discount,
  DateTimeImmutable $startDate,
  DateTimeImmutable $endDate
): Discount;

public function addPaymentMethodConstraint(
  Discount $discount,
  int $paymentMethod
): Discount;

Таким образом, если мы хотим создать новую скидку с определёнными ограничениями, мы сделаем это так:

$discountId = insertDiscount(
  'Discount name',
  'Discount description',
  100,
  Discount::STATUS_ACTIVE
);

addPaymentMethodConstraint(
  $discountId,
  PaymentMethod::CREDIT_CARD
);

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

Null в свойствах объектов


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

$currencyCode = strtolower(
  $record->currencyCode
);

Бууум! «Не могу передать значение null в strtolower». Это произошло потому, что разработчик забыл, что currencyCode может быть null. Поскольку многие разработчики до сих пор не используют IDE или подавляют предупреждения в них, это может оставаться незамеченным в течение многих лет. Ошибка продолжит валяться в каком-то непрочитанном логе, а клиенты будут сообщать о периодически возникающих проблемах, которые, по-видимому, не связаны с этим, поэтому никто не потрудится взглянуть на эту строку кода.

Мы можем, конечно, добавить проверки на null везде, где получаем доступ к currencyCode. Но тогда мы окажемся в аду другого рода:

if ($record->currencyCode === null) {
  throw new \RuntimeException('Currency code cannot be null');
}

if ($record->amount === null) {
  throw new \RuntimeException('Amount cannot be null');
}

if ($record->amount > 0) {
  throw new \RuntimeException('Amount must be a positive value');
}

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

Объекты-значения


Объекты-значения (value objects) — это мощные, но простые вещи. Проблема, которую мы пытались решить, заключалась в том, что необходимо постоянно валидировать все наши свойства. А делаем мы это потому, что не знаем, можно ли доверять свойствам объекта, валидны ли они. А что если бы мы могли?

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

final class Amount
{
  private $amountInCents;
  private $currencyCode;

  public function __construct(int $amountInCents, string $currencyCode): self
  {
    Assert::that($amountInCents)->greaterThan(0);

    $this->amountInCents = $amountInCents;
    $this->currencyCode = $currencyCode;
  }

  public function getAmountInCents(): int
  {
    return $this->amountInCents;
  }

  public function getCurrencyCode(): string
  {
    return $this->currencyCode;
  }
}

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

Поскольку мы используем объявления типов, то гарантируем, что тип также является правильным. Таким образом, мы не можем передать int в strtolower. Если вы используете более старую версию PHP, которая не поддерживает объявления типов, то можете использовать этот пакет для проверки типов с помощью ->integer() и ->string().

После создания объекта значения нельзя изменить, потому что у нас есть только геттеры, но нет сеттеров. Это называется иммутабельность. Добавление final не позволяет расширять этот класс для добавления сеттеров или магических методов. Если вы видите Amount $amount в параметрах метода, то можете быть на 100% уверены, что все его свойства были валидированы и объект безопасен для использования. Если бы значения были не валидными, мы бы не смогли создать объект.

Теперь с помощью объектов-значений мы можем ещё улучшить наш пример:

$discount = new Discount(
  'Discount name',
  'Discount description',
  new Amount(100, 'CAD'),
  Discount::STATUS_ACTIVE
)

insertDiscount($discount);

Обратите внимание, что мы сначала создаём Discount, а внутри используем Amount в качестве аргумента. Это гарантирует, что метод insertDiscount получает валидный объект скидки в дополнение к тому, что весь этот блок кода теперь намного проще понять.

«Нулевая» история ужасов


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

$collection = $this->findBy(['key' => 'value']);
$result = $this->filter($collection, $someFilterMethod);

if ($result === null) {
   $result = $collection;
}

Если результат null, то использовать изначальную коллекцию в качестве результата? Это проблематично, так как метод фильтрации возвращает null, если не нашёл подходящие значения. Таким образом, если всё отфильтровано, мы проигнорируем фильтр и вернём все значения. Это полностью ломает логику.

Почему изначальная коллекция используется в качестве результата? Мы никогда этого не узнаем. Я подозреваю, что у разработчика было определённое предположение о том, что означает null в этом контексте, но оно оказалось неверным.

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

try {
  $result = $this->filter($collection, $someFilterMethod);
} catch (CollectionCannotBeEmpty $e) {
  // ...
}

Этот код однозначен. Разработчик вряд ли сможет неправильно истолковать его.

Стоит ли это таких усилий?


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

Вот и наступил конец моей null-тирады. Я надеюсь, это поможет вам писать более понятный и поддерживаемый код.
FunCorp
141,98
Разработка развлекательных сервисов
Поделиться публикацией

Комментарии 36

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

    А Олег то не так прост, как кажется (:
      +9
      Статья – перевод, а автор оригинала Anna Filina.
      0
      Теперь сравните это с первоначальным вызовом. Вы увидите, насколько удобнее стало читать.

      Читать-то стало удобнее, но вместо одного (возможно, атомарного) вызова функции теперь нужно делать два. И при всем при этом из сигнатур функций по-прежнему не очевидно, в каких случаях надо отдельно задавать констрейнты, в каких — достаточно обойтись тем, что задается по умолчанию. Плюс если кто-то где-то забудет задать констрейнт после задания скидки, то найти багу будет непросто.
        0

        Скорее это проблема конкретного языка (версии), ведь много где можно передавать параметры с указанием их имён...

        +1
        Анна изобрела рефакторинг?
          0
          Не пугайте так! А то ещё чуть-чуть и они ALGOL68 изобретут, наконец.
          +9
          /**
           * Это была функция из самодокументированного кода переданного нам очень давно.
           * PS. HACK: Не трогать!!!!!!!!!!! Кривой код.
           * @param {any} a - Тип скидки
           * @param {any} b - Тип клиента, IClient.CORP | IClient.INDIVIDUAL
           * @param {any} c - Фаза луны
           * @param {any} d - Непонятно что, но НУЖНО ставить null 
           * @param {any} e - тип валюты, блин этот чувак точно был больной
           */
          function SelfDocumentedFunction(a,b,c,d,e) {
          ....
          }
          
            +1

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

              +1

              Полностью согласен. Хочу добавить, что нужно приложить немало усилий, чтобы настроить себя "это проблема, и я не пойду дальше, пока не решу её". И малейшее давление ("ну когда будет готово?") может этот настрой прибить.

                +7

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

            –3
            так много «Я»
              +1
              Хорошо у нас в свифте. Есть опшнлы, и сам язык заставляет больше думать об обработке всех значений, особенно null/не null (nil только у нас).

              Помню, спорил много лет назад, «опытные» (TM) программисты говорили, что язык должен позволять все, а я профессионал такого высокого класса, а С++ (например) — язык для профессионалов, что мне можно доверять. Ошибки делают все, и каждый день, если конечно что-то делают, и лучше какие-то наши ошибки пофиксит компилятор, а язык будет от них максимально оберегать, чем ситуации, описанные в статье
                0
                Если выбирать между инструментом, которым можно сделать абсолютно всё, но он опасен для детей и им можно прострелить ногу и инструментом, который позволяет сделать лишь часть действий я выберу первое (естественно, если действие будет удобнее/быстрее делать первым).
                Тут уже дело некоторого «риска» в угоду максимальной мощности и управляемости.
                –2
                В Ruby удобно — у каждого типа свой нулевой объект ('', [], {}, 0 и т.д.)
                  +1

                  Пустой объект (строка длиной 0, массив без элементов) != null

                    –1
                    Я к тому, что в ruby не нужен null
                      0
                      Отсутствие массива и пустой массив — это, мягко говоря, совершенно разные вещи семантически.

                      Все, что вы описали (пустые массивы, 0 и т.д.), есть везде и эти значения можно использовать вместо null. По сути дела это null-объекты, у которых ограниченная сфера применения. Только иногда нужно сообщить, что массив неинициализирован совсем, а не то, что он пустой.
                        0
                        В каких реальных ситуациях нужно сообщить, что массив не инициализирован совсем? Мне действительно интересно, так как не сталкивался.
                          +1
                          Ну например при кэшировании или ленивой загрузке: пустой массив — это корректное значение, полученное откуда-то (из базы данных), а null — это указатель на то, что значение еще не запрашивали вовсе.
                            0
                            При этом в базе данных тоже может быть NULL, так что может потребоваться ещё и «настоящий null» и «NULL из базы данных» иногда…
                              0
                              Чистая правда
                                +1
                                в этом плане, как бы смешно это не звучало, мне нравится javascript. там есть `null` и `undefined`. очень удобноо как раз использовать в семантике «получено пустое значение» и «значение еще не было проинициализировано», особенно при работе api
                                  0
                                  Ну дык Эйх (который Брендан) изначально вообще в браузер Scheme хотел затащить. Откуда он и спёр идейку (изуродовав, как обычно, «чтобы людей не пугать»). И Null/Undefined в Java. И None/False в Python. И всё прочее в этом духе обобщается в Lisp-подобных языках до понятия символа — выделенного значение. По умолчанию обычно бывают доступны #nil и #true/#false, но можно завести и какой-нибудь #database-connection-lost, если нужно…

                                  JavaScript (и Python) «подобрали» эту идею с несколькими встроенными символами, но до того, чтобы дать возможность разработчику их создавать… «недоросли»…
                        0

                        null это просто абстракция. Invalid pointer. Даже на С можно писать без использования null. Просто его придумали во времена, когда одного invalid pointer a хватало всем ;)

                    +1

                    Проблема не в null, а в том, что мы не видим имен аргументов. В последнем варианте можно так же спросить про строки, не видно же что каждая значит. Нагляднее (и более гибким) был бы такой вариант:


                    insertDiscount(new Discount()
                        .setName('Discount name')
                        .setDescription('Discount description')
                        .setAmount(100, 'CAD')
                        .setActive()
                    )
                      +1
                      В этом коде все аргументы не обязательны. В исходном коде все наоборот.
                        0
                        всегда есть Builder для этого
                          0
                          С билдерами не работают синтаксические анализаторы
                        +1
                        То же самое хотел написать — в «лучших домах» советуют не оставлять объекты недоинициализированными, и при необходимости юзать Builder, а тут все по новой — сначала инициализируем обязательные, а потом возможно остальные. Следующая статья видимо будет про криворуких разработчиков, которые исправили(скопировали, отрефакторили) только конструктор и забыли про дополнительные сеттеры.
                          0
                          Наглядный вариант, как сделать возможность создать невалидное состояние обьекта и нарушить инапсуляцию
                          m.habr.com/ru/post/469323
                          0
                          В соседней ветке куча людей обсуждает функциональное программирование, может тут есть кто из них. Как все это будет выглядеть в ФП?
                            –1
                            Так эта, товарисч dead_man, как раз стиле ФП и написал
                              0
                              Там описана просто цепочка сеттеров (fluent interface). Это паттерн из мира ООП, ФП вообще не при чем. Не вводите людей в заблуждение.
                            +2

                            psalm, phpstan, phan… inspect в шторме наконец.
                            Но нет, люди продолжают придумывать себе проблемы) на что только не пойдёшь лишь бы не работать

                              0
                              Assert::that($amountInCents)->greaterThan(0);

                              Странно, что это называется assert. Это же precondition. Либо библиотека писалась для ассертов, но используется для прекондишнов. Технически разница может невелика, но семантика у ассерта и прекондишна разная.

                                0
                                Ассерт никак не связан с тем используют его в пред- или постусловии. Это просто проверка некоторого утверждения. В пыхе в принципе даже родной ассерт есть
                                www.php.net/manual/ru/function.assert.php

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

                              Самое читаемое