Ошибочное понимание принципа DRY

Original author: Matthieu Cneude
  • Translation


Я знаю, о чём вы подумали: «Ещё одна скучная статья про DRY? Нам их мало, что ли?».


Возможно, вы правы. Но я встречаю слишком много разработчиков (junior и senior), применяющих DRY так, словно они охотятся на ведьм. Либо совершенно непредсказуемо, либо везде, где можно. Так что, судя по всему, в интернете никогда не будет достаточно статей о принципе DRY.


Если кто не знает: принцип DRY — это "Don't Repeat Yourself" (не повторяйся), и впервые о нём упомянуто в книге The Pragmatic Programmer. Хотя сам по себе этот принцип был известен и применялся задолго до появления книги, однако в ней ему было дано название и точное определение.


Итак, без лишних рассуждений отправимся в путешествие по чудесной стране DRY!


Не повторяйте свои знания


Хотя фраза «не повторяйся» звучит очень просто, в то же время она слишком общая.


В книге The Pragmatic Programmer принцип DRY определяется как «Каждая часть знания должна иметь единственное, непротиворечивое и авторитетное представление в рамках системы».


Всё замечательно, но… что такое «часть знания»?


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


Если взять популярные аналогии из электронной коммерции, то класс shipment и его поведение будут частью предметной области бизнеса вашего приложения. Отгрузка — это реальный процесс, используемый вашей компанией для отправки товара своим клиентам. Это часть бизнес-модели компании.


Следовательно, логика работы класса shipment должна появляться в приложении однократно.


Причина очевидна: допустим, вам нужно отправить груз на склад. Для этого придётся активировать логику отгрузки в 76 разных местах приложения. Без проблем: вставляем логику 76 раз. Через некоторое время приходит начальник и просит кое-что изменить в логике: вместо одного склада компания теперь распределяет товары по трём складам. В результате вы потратите кучу времени на изменение логики, поскольку вам придётся сделать это в 76 местах! Совершенно впустую потраченное время, хороший источник багов и прекрасный способ выбесить начальство.


Решение? Создайте единственное представление вашего знания. Положите куда-нибудь логику отправки товара на склад, а затем используйте представление этого знания везде, где нужно. В ООП отправка груза может быть методом класса shipment, который вы можете использовать по своему усмотрению.


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


DRY и дублирование кода


Так значит DRY относится к knowledge? К бизнес-логике?


Начнём с очевидного:


<?php

interface Product
{
    public function displayPrice();
}

class PlasticDuck implements Product
{
    /** @var int */
    private $price;

    public function __construct(int $price)
    {
        $this->price = $price;
    }

    public function displayPrice()
    {
        echo sprintf("The price of this plastic duck is %d euros!", $this->price);
    }
}

$plasticDuck = new PlasticDuck(2);
$plasticDuck->displayPrice();

Этот код выглядит недурно, правда?


Однако Лёха, твой коллега-разработчик, с вами не согласится. Когда Лёха увидит этот код, подойдёт к вашему столу и возмутится:


  1. Слово price повторяется 6 раз.
  2. Метод displayPrice() повторяется в интерфейсе, в реализации, и вызывается во время runtime.

Лёха — начинающий эксперт в вашей компании, и не имеет понятия об ООП.


Я прямо вижу, как вы, замечательный разработчик, смотрите на Лёху, словно опытный садовник на личинку, и отвечаете:


  1. Это переменная (и свойство), и вам нужно повторять её в коде.
  2. Однако логика (отображения цены) представлена однократно, в самом методе. Здесь не повторяется ни знание, ни алгоритм.

Здесь никак не нарушается принцип DRY. Лёха затыкается, подавленный вашей аурой величия, заполняющей комнату. Но вы поставили под сомнение его знания, и он злится. Лёха гуглит приницп DRY, находит другой пример вашего кода, возвращается и тычет вам в лицо:


<?php

class CsvValidation
{
    public function validateProduct(array $product)
    {
        if (!isset($product['color'])) {
            throw new \Exception('Import fail: the product attribute color is missing');
        }

        if (!isset($product['size'])) {
            throw new \Exception('Import fail: the product attribute size is missing');
        }

        if (!isset($product['type'])) {
            throw new \Exception('Import fail: the product attribute type is missing');
        }
    }
}

Лёха торжествующе восклицает: «Ты олень! Этот код не соответствует DRY!». А вы, прочитав эту статью, отвечаете: «Но бизнес-логика и знание всё ещё не повторяются!»


Вы снова правы. Метод проверяет результат парсинга CSV только в одном месте (validateProduct()). Это знание, и оно не повторяется.


Но Лёха не готов смириться с поражением. «А что насчёт всех этих if? Разве это не очевидное нарушение DRY?». Вы контролируете свой голос, тщательно выговариваете каждое слово, и ваша мудрость отражается от стен, создавая эхо осознания: «Ну… нет. Это не нарушение. Я бы назвал это ненужным дублированием кода, а не нарушением принципа DRY».


Вы молниеносно набираете код:


<?php

class CsvValidation
{
    private $productAttributes = [
        'color',
        'size',
        'type',
    ];

    public function validateProduct(array $product)
    {
        foreach ($this->productAttributes as $attribute) {
            if (!isset($product[$attribute])) {
                throw new \Exception(sprintf('Import fail: the product attribute %s is missing', $attribute));
            }
        }
    }
}

Выглядит лучше, верно?


Подведём итог:


  1. Дублирование знания всегда является нарушением принципа DRY. Хотя можно представить ситуацию, когда это не так (напишите в комментах, если у вас тоже есть такие примеры).
  2. Дублирование кода не обязательно является нарушением принципа DRY.

Но Лёха ещё не убеждён. Со спокойствием высшего духовного наставника, вы ставите точку в споре: «Многие думают, что DRY запрещает дублировать код. Но это не так. Суть DRY гораздо глубже».


Кто это сказал? Дэйв Томас, один из авторов The Pragmatic Programmer, книги, в которой дано определение самого принципа DRY!


Применяем DRY везде: рецепт катастрофы


Бесполезные абстракции


Возьмём более жизненный, интересный пример. Сейчас я работаю над приложением для компании, занимающейся производством фильмов. В него можно легко загружать сами фильмы и метаданные (заголовки, описания, титры…). Всё эта информация затем отображается в VoD-платформе. Приложение построено на основе паттерна MVC, и выглядит оно так:



Но создавать с помощью этого приложения контент могут не только съёмочные группы. К примеру, им может воспользоваться и команда по управлению контентом в моей компании. Почему? Некоторые съёмочные группы не хотят этим заниматься. У съёмочной команды и команда по управлению контентом совершенно разные потребности. Вторые работают с CMS, а первые — нет.


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



Выглядит как очевидное и уродливенькое нарушение принципа DRY: повторяются представления (view) и контроллер.


Другие решения? Можно было бы сгруппировать повторяющуюся логику с помощью своеобразной абстракции. Но тогда пришлось бы объединить контроллеры. Если изменится абстракция, то каждый контроллер должен будет поддерживать это изменение. Мы знали, что в большинстве случаев, в зависимости от использования, эти представления будут отображаться по-разному. Пришлось бы в действиях контроллеров создавать кучу if, а мы этого не хотели. Код стал бы сложнее.


Более того, контроллеры не должны содержать в себе бизнес-логику. Если вспомните определение DRY, то это как раз знание, которое не стоит дублировать.


Иными словами, если пытаться везде применять DRY, то можно прийти к одному из двух:


  1. Ненужному объединению.
  2. Ненужной сложности.

Очевидно, что ни то, ни другое вам не улыбается.


Преждевременная оптимизация


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


Если вы начинаете что-то абстрагировать, потому что «позже это может понадобиться», то не делайте этого. Почему?


  1. Вы потратите время на абстрагирование того, что потом может быть никогда не востребовано. Бизнес-потребности могут изменять очень быстро и очень сильно.
  2. Повторюсь, вы можете привнести в код сложность и объединение ради… пшика.

Многократное использование кода и дублирование кода — разные вещи. DRY гласит, что не нужно дублировать знания, а не что нужно всегда делать код многократно используемым.


Сначала напишите код, отладьте его, а затем уже держите в уме все эти принципы (DRY, SOLID и прочие), чтобы эффективно рефакторить код. Работать с нарушением принципа DRY нужно тогда, когда знание уже дублировано.


Дублирование знания?


Помните, выше я говорил, что повторение бизнес-логики всегда является нарушением DRY? Очевидно, что это справедливо для ситуаций, когда повторяется одна и та же бизнес-логика.


Пример:


<?php

/** Shipment from the warehouse to the customer */
class Shipment
{
     public $deliveryTime = 4; //in days

     public function calculateDeliveryDay(): DateTime
     {
         return new \DateTime("now +{$this->deliveryTime} day");
     }
}

/** Order return of a customer */
class OrderReturn
{
    public $returnLimit = 4; //in days

    public function calculateLastReturnDay(): DateTime
    {
         return new \DateTime("now +{$this->returnLimit} day");
    }
}

Вы уже слышите, как ваш коллега Лёха нежно вопит вам в ухо: «Это очевидное нарушение всего, во что я верю! А как же принцип DRY? У меня сердце сжимается!».


Но Лёха снова ошибается. С точки зрения электронной коммерции, время доставки товара покупателю (Shipment::calculateDeliveryDay()) не имеет отношения к сроку возврата товара покупателем (Return::calculateLastReturnDay).


Это две разные функциональности. То, что выглядит дублированием кода, на самом деле чистое совпадение. Что произойдёт, если вы объедините два метода в один? Если ваша компания решит, что покупатель теперь может вернуть товар в течение месяца, то вам снова придётся разделить метод. Ведь если этого не сделать, то срок доставки товара тоже будет составлять один месяц!


А это не лучший способ завоевать лояльность клиентов.


DRY — не просто принцип для педантов



Сегодня даже джин может быть DRY!


DRY — это не то, что вам стоит уважать только в своём коде. Не нужно повторять знания в каждом аспекте вашего проекта. Снова процитирую Дэйва Томаса: «Понятие «знания» в системе охватывает далеко не только код. Оно относится и к схемам баз данных, планам тестирования, системе сборки и даже документации».


Суть DRY проста: не нужно обновлять параллельно несколько вещей, если вносится какое-то изменение.


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


DRY — это принцип


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


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


Однако принципы — не правила. Они лишь инструменты, помогающие идти в правильном направлении.


В разработке у всего есть цена. И DRY не исключение. Очевидно, что не стоит везде повторять бизнес-логику, но также не нужно и тесно объединять всё подряд, экономя на абстракциях. Необходимо искать баланс, который зависит от текущей ситуации.

Mail.ru Group
Building the Internet

Comments 16

    +10
    Мне где-то попадалась мысль, что когда пишешь какую-то функциональность первый раз — делай частное решение, когда сталкиваешься с этой же проблемой второй раз — скопирую код, модифицируй чтобы работал для этого случая и пометь на будущее и вот когда ты сталкиваешься с проблемой в третий раз — обобщай.

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

    Подтверждение этой мысли я встречался и в своем коде и в коде коллег. Чаще было, что код излишне обобщен.
      +3

      https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)


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

        +1
        Спасибо за ссылку. Заодно вспомнил где я на него наткнулся — в книге Фаулера.
      +1

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

        +1
        возвращается и тычет вам в лицо

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

        Как это происходит:
        1)Тимлид/ТехДир/Кто-то еще (Пусть будет Мистер М) делает ультимативным тоном спорное утверждение обильно посыпая его высокомерными ужимочками.
        Обычно суть утверждения в следующем «Я Очень умный, надо делать так-то, а вы все Дебилы если со мной не согласны».
        2) «Вы все» — слушают, но с такой оценкой себя они, конечно, не согласны — и начинается спор о том, чем «чьё кунг-фуDRY круче».

        Последствия:
        1)Когда оказывается, что М ошибся (так тоже бывает не редко) — признать это и принять решение которое было бы лучше для проекта ему мешает то заявление, в котором он назвал всех «идиотами»

        2)«Вы все» — будут и дальше часть времени тратить не на то чтобы «дотащить проект», а на то чтобы найти где еще мистер М налажал

        3)Эго МистераМ оплачено человеко-часами из бюджета проекта.

        В чём мораль?
        1)Не будьте «М». Вы не «самый умный в этой комнате».
        2)Если вы нанимаете к себе в команду человека — постарайтесь убедиться что он не М.
          0
          Выглядит лучше, верно?


          Нет. Не выглядит. Прозрачность потеряна. Введены лишние сущности. Хотя код и «модно» и «по-ООПовски», но он работает медленнее и стал заметно сложнее.

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

          [
          'price' => 'message about invalid price'
          ]

          Но… Зачем?
            0
            Прозрачность потеряна.

            ну вообще-то нет. Можно переименовать филд в requredAttributes что бы было "прозрачнее", но это в целом вопрос семантики кода.


            Введены лишние сущности.

            которые закрыты интерфейсом — не страшно.


            Хотя код и «модно» и «по-ООПовски»

            в каком месте оно "модно" и "по-ООПовски"? Это старые добрые процедуры. Процедура потому что функция не чистая, да и валидатор не должен кидать исключения. Для валидатора невалидные данные — это обычное дело.


            но он работает медленнее и стал заметно сложнее.

            на счет сложнее — да, но если у нас есть N валидаторов, саму логику мы можем инкапсулировать в какой-то более общий валидатор, например RequredAttributesValidator и дальше делать все за счет композиции. Тогда все красиво и "оопэшно".


            Что до вопросов производительности — это спички. Не страшно. Для меня важнее что валидатор почему-то кидается исключениями.

              0
              В кавычках же («модно» и «по ООП-овски»).

              Для меня важнее что валидатор почему-то кидается исключениями.

              Кстати, а как надо делать? Собирать все проблемы в кучу и возвращать структуру
              {
              is_validated: false,
              invalid_entries: []
              }

              ?
              P.s. с именами полей согласен, семантика не на высоте ;)
                0
                имхо, у валидатора должен быть метод
                public function isValid($object): bool;

                А уже клиентский код решает что делать с невалидным объектом
                  0

                  То есть клиентский код должен знать почему именно объект невалиден? Зачем тогда нужен валидатор?

                    0
                    При таком подходе эксепшены оправданы ;-)

                    Вот только мнится мне, что подход такой верен лишь в некоторых случаях (не люблю ультимативности в формулировках, а искать/выдумывать примеры не хочу).
                      0
                      Нет, почему не валиден можно запросить методом getViolations. Клиентский код должен решать что делать. если объект не валиден. Бросить ексепшен, использовать другую стратегию или просто ничего не сделать \ записать в лог
                        0

                        то есть валидатор должен иметь стэйт? зачем?

                      +1

                      validate($object): ErrorList тогда уже. isValid можно добавить как сокращение к проверке длины списка ошибок.

                0

                Многие пытаюсь усилить DRY через флаги в методах функций, что является неправильным


                Например: контроллер getItems у которого есть флаг count_items, т.е. возвращает количество элементов.
                Понять тут программиста можно — ведь получить товары и получить их количество очень похожие методы, но он тогда нарушает принцип Single Responsibility, ведь у нашего метода уже 2 ответственности.
                Поэтому в этом случае в угоду DRY надо сделать методы: getItems, getCountItems.

                  +1
                  пытаюсь усилить DRY через флаги в методах функций

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


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

                Only users with full accounts can post comments. Log in, please.