Принцип DRY (Do not Repeat Yourself) давно всем вполне очевиден и любим многими программистами. И многие согласны, что Copy/Paste это совсем не круто. В этой статье я хочу привести пример того, в каких случаях в промышленном программировании использование Copy/Paste более уместно и помогает красиво реализовать Open-Closed принцип из SOLID.

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

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

Имеем класс Parcel, который описывает работу обычной посылки:

public interface IParcel {
      string Barcode {get; set;}
}
public class Parcel: IParcel  { 
      public string Barcode {get; set;}
}

Очень соблазнительно просто добавить поле и в старый класс Parcel, и в интерфейс IParcel:

public interface IParcel {
      string Barcode  {get; set;}
      bool   IsUrgent {get; set;}
}
public class Parcel: IParcel  { 
      public string Barcode  {get; set;}
      public bool   IsUrgent {get; set;}
}

Однако, такой код не должен проходить CodeReview! Строгий и опытный «проверяльщик» кода должен вернуть его с замечанием: «такая реализация нарушает Open-closed принцип.»

Гораздо лучше создать новый класс UrgentParcel, и не нужно будет менять ни интерфейс, ни класс Parcel. Файлы класса и интерфейса останутся нетронутыми:

public class UrgentParcel: IParcel { 
      public string Barcode {get; set;}
}

Это будет соблюдением Open-closed принципа, и такой код не получит замечания при CodeReview.

Теперь давайте вернемся к DRY и к тому, каким образом он мешает реализовать Open-closed принцип.

Представим, что в классе Parcel у нас есть поле «статус посылки» и некая логика изменения этого статуса:

public class Parcel: IParcel  { 
      public string Barcode {get; set;}
      // Статус посылки (в пути, или уже доставлена и т.п.)
      public ParcelStatuses Status {get; }
      // метод, который меняет статус посылки на "доставлена"
      public void ArrivedToRecipient(){
            this.Status = ParcelStatuses.Arrived;
      }
}

Нужно ли эту логику копировать в класс UrgentParcel? Принцип DRY говорит, что ни в коем случае. Гораздо лучше, чтобы класс UrgentParcel просто наследовался от класса Parcel, что решит задачу и не придется Copy/Paste'ть тело метода ArrivedToRecipient в класс UrgentParcel.

Однако, если не копировать код, а наследовать его, то изменения метода ArrivedToRecipient в классе Parcel сразу же приведут к изменению поведения класса UrgentParcel, что и будет являться нарушением Open-closed принципа. Это действительно нарушение, потому что задача со срочными посылками (реализация класса UrgentParcel) уже была сдана, протестирована и, как следствие, работает «в бою». Значит, эта логика, реализованная в методе UrgentParcel.ArrivedToRecipient и примененная к срочным посылкам — всех устраивает и она НЕ должна меняться при изменении работы других видов посылок. Так вот Open-closed принцип как раз и предназначен защищать систему от подобных действий неопытных junior-программистов, которые, решая задачу, как обычно «в лоб», не осознают еще пока всех зависимостей, и их изменения в одном месте затрагивают многие другие функциональные области.

Обычно, одним из главных аргументов в пользу DRY является тот факт, что, если найдена ошибка в методе ArrivedToRecipient, то ее надо исправлять везде, куда ее скопировали. Так вот этого, как раз, делать не нужно. Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.

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

Вот пример такого комментария:

public class Parcel: IParcel{ 
    ... 
    /// <summary>
    /// Проставляет посылке статус "доставлена" 
    /// </summary>    
    /// <remarks>NOTE: код метода копировался в классы: <see cref="UrgentParcel"/></remarks>
    public void ArrivedToRecipient(){ ... }
}

public class UrgentParcel: IParcel{ 
    ... 
    /// <summary>
    /// Проставляет посылке статус "доставлена"
    /// </summary>    
    /// <remarks>NOTE: код метода был скопирован из класса: <see cref="Parcel"/></remarks>
    public void ArrivedToRecipient(){ ... }
}

Очень интересно мнение сообщества по этому вопросу. Заранее благодарен за ваши комментарии.

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


  1. AEP
    17.09.2018 11:33

    Вы не первый, кто поднимает такую тему. Примерно те же мысли, но для конфигов Nginx, есть тут: habr.com/company/oleg-bunin/blog/313666


    1. Basim108 Автор
      17.09.2018 13:08

      Спасибо! Почитал, полезная и очень интересная ссылка :)


  1. SUA
    17.09.2018 11:43
    +1

    А на работу срочных посылок никто не жаловался

    Пример плохой — 95% пожалуются

    Или как не раз видел — при таком подходе копирования «не жалуются потому что работают с еще одной копией такого же класса SpeedParcel где все исправлено, и про эту реализацию UrgentParcel никто даже не слышал»


  1. DrPass
    17.09.2018 12:00
    +1

    Очень интересно мнение сообщества по этому вопросу. Заранее благодарен за ваши комментарии.

    Моё мнение, например, SOLID — это не самоцель, а инструмент. Инструмент, который должен упрощать сопровождение кода. Если речь идет о добавлении метода в интерфейс, который затем требует перепроектирования вызовов в массе мест программы, то есть смысл добавить новый интерфейс. Наш инструмент работает, мы ему следуем.
    Если такая проблема не стоит, интерфейс ещё не задействован во множестве мест, требующих перепроектирования, то следуя нашему инструменту, мы вместо одной простой компоненты порождаем две, каждая из которых нуждается в сопровождении. Это наоборот, усложняет код. Значит, тут SOLID не справляется со своей задачей. Значит, мы не должны ему следовать.
    Опять же-таки, нет единого шаблона относительно
    . Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.

    Каждый такой случай нуждается в индивидуальной оценке.


  1. Deosis
    17.09.2018 12:02
    +1

    Кране против такого предложения.
    Если один код скопировали в 10 различных мест, а при изменении требований изменили только в одном, то стоит ожидать в трекере 9 новых багов.


    1. michael_vostrikov
      18.09.2018 11:34

      «Вы же сказали, что исправили, почему опять не работает?»


    1. MaxVetrov
      18.09.2018 12:08

      Ага, их еще нужно найти и скопипастить исправление в каждое из них, и еще не факт что ваше исправление будет работать во всех этих местах .)


  1. APXEOLOG
    17.09.2018 12:12
    +1

    В описываемом вами случае можно ввести некий GenericParcel: IParcel, куда сложить всю логику, которая не зависит от типа посылок и затем уже наследовать Parcel и UrgentParcel от него


    Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.

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


    1. Basim108 Автор
      17.09.2018 13:06
      -1

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

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

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

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


      1. APXEOLOG
        17.09.2018 13:19
        +2

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

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


      1. VolCh
        17.09.2018 21:29

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


      1. areht
        17.09.2018 22:10

        > Да и вообще подобные комментарий откуда/куда копировалось, помогают держать копи-паст под контролем.

        Нет. Вообще нет. Что толку от ваших комментариев после того, как «исправлять надо именно работу обычных посылок»? А что толку до этого момента?

        > Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.

        Вот ведь как интересно получается:
        — Если ВЫ откопипастили, то починяться только обычные посылки
        — Если ВЫ не откопипастили, то починяться все посылки

        То есть вы в момент копипасты предсказали скоп ошибки!

        Ну и да, понимание OCP у вас… спорное.
        А из примера не понятно чем именно UrgentParcel отличается, что бы заслужить отдельного класса.


  1. mayorovp
    17.09.2018 12:27

    Мне одному кажется, что public string Barcode {get; set;} — это верный признак DTO, в котором никакой логики не должно быть в принципе?


  1. Arlekcangp
    17.09.2018 12:27
    +1

    Почему только «O»? Если этот принцип возвести в абсолют, как и любой другой, так он все что угодно будет нарушать. И еще по моему как то упрощенно понят open/closed. Он запрещает изменять поведение в наследниках класса/интерфейса таким вот несовместимым образом. Т е формальные аргументы и возвращаемое значение не меняются, но при этом меняется семантика, так что клиенты IParcel более не могут работать с этим новым классом. Как пример он меняет внутренние состояние объекта так, как это не предполагается в других реализациях или добавляет стэйт-машину как в статье, которой изначально в интерфейсе не предполагалось. В таких случаях надо принять либо факт ошибки проектирования, либо выход функциональных требований за изначально заложенные рамки и рефакторить (или делать новый интерфейс со всеми вытекающими) Dry или не Dry — тут не поможет, т к существующие клиенты IParcel ничего не знают о том, что эта посылка может быть в нескольких состояниях.


    1. Basim108 Автор
      17.09.2018 12:57

      Спасибо Вам за комментарий,

      Т е формальные аргументы и возвращаемое значение не меняются, но при этом меняется семантика, так что клиенты IParcel более не могут работать с этим новым классом.
      Тут Вы скорее описываете нарушение принципа замещения Лисков.


      P.S. Я не хотел описывать сразу весь интерфейс IParcel со всеми его свойствами и методами, дабы читателю было проще читать. Метод ArrivedToRecipient был добавлен не только в класс, он, как я предполагал, но не описал в статье, был изначально у IParcel и его реализация есть в классе Parcel. Так вот вопрос обсуждался стоит ли копи/пастить тело метода в класс UrgentParcel.


  1. Steed
    17.09.2018 14:25

    Однако, если не копировать код, а наследовать его, то изменения метода ArrivedToRecipient в классе Parcel сразу же приведут к изменению поведения класса UrgentParcel, что и будет являться нарушением Open-closed принципа.
    Откуда у вас изменение метода ArrivedToRecipient? Если исправлена ошибка, то это не нарушение O по вашему же собственному определению. Если он изменился при добавлении какой-то другой функциональности (DangerousParcel), то нарушение О именно в DangerousParcel, а не тут. Если третья причина, то назовите ее;)

    Целью принципа О как раз и является то, чтобы в базовом классе происходили только ценные изменения, которые вам по умолчанию нужны. Если "работает-не трогай" для вашей системы является более приоритетным, чем стоимость поддержки и модификации кода, то выпускайте базовый класс как отдельную версионируемую библиотеку и пусть UrgentParcel вечно живет с зависимостью от Parcel v.1.0 (это близко к смыслу, вложенному в open-closed principle автором термина Bertrand Meyer).


    Кстати, с подходом расширения функциональности через наследование у вас скоро случится комбинаторный взрыв — UrgentParcel, DangerousParcel, UrgentDangerousParcel, InternationalUrgentParcel. В таких случаях лучше подходит компонентная модель (https://en.wikipedia.org/wiki/Component-based_software_engineering). Там и версионирование можно устроить, и комбинаторного взрыва избежать, да и качество абстракций получается более высоким.


  1. strayker1206
    17.09.2018 15:11

    А как насчёт того, чтобы вынести метод ArrivedToRecipient в базовый абстрактный класс GenericParcel и сделать его виртуальным? Кто захочет — переиспользует логику (как в случае с UrgentParcel), кому нужно — переопределит (Parcel). Тогда и копипасты не будет, и open-closed principle не будет нарушен. То есть, если у наследников изначально есть общая логика, то очевидно, что она должна лежать в базовом абстрактном классе, а не быть скопированной повсюду.
    Сорри за повтор, товарищ APXEOLOG уже выше написал об этом


  1. fukkit
    17.09.2018 16:35
    +1

    Гораздо лучше создать новый класс UrgentParcel, и не нужно будет менять ни интерфейс, ни класс Parcel.

    Это серьезно? Повбывав бы…


  1. Javaphite
    17.09.2018 17:41

    Всё же, копирование кода — крайне ужасная практика. Пару-тройку копий вы ещё сможете отслеживать, но их количество будет расти вслед за новыми требованиями и в разных частях проекта. Рано или поздно предел контроля будет превышен и начнётся опасный хаос.

    Мне кажется, если DRY не стыкуется с SOLID, то стоит посоветоваться с GoF. Данный пример очень напоминает ситуацию в красках описанную в «Head First: Design Patterns» в главе о Strategy.


    1. Basim108 Автор
      17.09.2018 17:45
      -1

      Согласен! И т.к. нарастание копи-пастов не происходит организованно, комментари о том откуда/куда скопированно будет уместен, чтобы понимать сколько раз копировали. И если возникает надобность третий раз коппировать, то нужно помечать код опасным и кондидатом на рефакторинг.

      Мне кажется, если DRY не стыкуется с SOLID, то стоит посоветоваться с GoF.
      Точно!!! :)


  1. Golovanoff
    17.09.2018 17:54
    -1

    На мой взгляд, «комментарий в коде» и перфекционисты — несовместимые вещи в данном случае.
    Разве не каноничнее в таком случае сделать класс-родитель metaParcel, в нем реализовать общее для обоих видов посылок, от него унаследовать Parcel и urgentParcel и т.д.? И не надо будет комментировать, где у вас там что надо менять, если «вдруг чо».


  1. Antervis
    17.09.2018 19:56
    +1

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


  1. debesha
    17.09.2018 22:39
    +2

    Не понимаю, почему тут нужно жертвовать чем-то.
    В рамках описанной эволюции всё делается и SOLID и DRY

    Я переведу это в PHP, но суть не меняется.
    Сначала была просто посылка.

    interface ParselInterface {
    
      public function getBarcode() : string;
      public function setBarcode(string $barcode);
    }
    
    class Parsel implements ParselInterface {
    
      public function getBarcode() : string {...};
      public function setBarcode(string $barcode) {...};
    }
    


    Потом нас просят добавить urgent фичу, мы расширяем (но не изменяем и не дублицируем) наш код.

    interface ParselInterface {
    
      public function getBarcode() : string;
      public function setBarcode(string $barcode);
    }
    
    class Parsel implements ParselInterface {
    
      public function getBarcode() : string {...};
      public function setBarcode(string $barcode) {...};
    }
    
    interface UrgentInterface {
      public function isUrgent() : bool;
    }
    
    class UrgentParsel extends Parsel implements UrgentInterface {
      public function isUrgent() : bool {return $this->isUrgent;}
    }
    


    Что там дальше идёт? Доставка реципиенту? Тут нужно уточнить поведение, и либо мы расширяем базовый интерфейс ParselInterface (если фича общая для всех посылок), либо добавляем новый. Соответсвенно, имплементация будет или на уровне родительского Parsel, либо нет. Допустим, первое.

    Наш код будет уже
    interface ParselInterface {
    
      public function getBarcode() : string;
      public function setBarcode(string $barcode);
      public function arrivedToRecipient();
    }
    
    class Parsel implements ParselInterface {
    
      public function getBarcode() : string {...};
      public function setBarcode(string $barcode) {...};
      public function arrivedToRecipient() {...};
    }
    
    interface UrgentInterface {
      public function isUrgent() : bool;
    }
    
    class UrgentParsel extends Parsel implements UrgentInterface {
      public function isUrgent() : bool {return $this->isUrgent;}
    }
    


    И никакого нарушения тут нет. arrivedToRecipient добавился к UrgentParsel, и изменения его текущего поведения не произошло. Extended, but not modified.

    А уже после этого — нельзя просто так менять arrivedToRecipient(). Как раз таки потому что O.

    Где тут что нарушается?


    1. MaxVetrov
      18.09.2018 03:46

      Как-то не очень получается — интерфейс UrgentInterface содержит функцию isUrgent. Она всегда «true» выдает?


      1. debesha
        18.09.2018 09:33

        Да, действительно. Но не суть. Может быть и пустым интерфейс, а может содержать специфические для urgent функции


    1. areht
      18.09.2018 13:13

      Скажите, вот есть код, он принимает массив ParselInterface. После ваших действий он Extended, but not modified?

      Или OCP ограничивается классом Parsel?


      1. debesha
        18.09.2018 13:41

        Ну конечно not modified. Если был класс, который работал с массивом ParselInterface, он будет всё так же работать с массивом ParselInterface и поддерживать все существующие к этому моменту фичи.

        Появление нового интерфейса (и нового кода, знающего, как работать с ним) не требует ни одного изменения в существующем коде и существующих features.

        В чем violation?


        1. areht
          18.09.2018 14:36

          Подозреваю, что именно существующий код то и придется модифицировать для поддержки UrgentInterface, иначе срочные посылки будут обрабатываться как обычные. Так что там нет «O», поэтому не будет и «C».

          Да и вообще знание, что реализации ParselInterface могут(?) реализовывать UrgentInterface несколько нарушает инкапсуляцию. В публичном контракте это тайное знание не описано


          1. debesha
            18.09.2018 15:32

            > ParselInterface могут(?) реализовывать UrgentInterface
            Вообще-то interface segregation principle как раз о том, что могут. Парсел это парсел, урджент это урджент. Друг другу они мешать не должны.

            > Подозреваю, что именно существующий код то и придется модифицировать для поддержки UrgentInterface, иначе срочные посылки будут обрабатываться как обычные.

            Существующий код тоже должен расширяться. Поведение существующих классов меняться не должно, но должны добавляться новые классы, которые будут реализовывать новые или существующие интерфейсы.

            Т.е. говоря кодом, одновременно с ParselProcessor, реализующим ParselProcessorInterface должен появиться UrgentParselAwareProcessor, реализующий тот же интерфейс и он инджектится вместо существующего ParselProcessor


            1. areht
              18.09.2018 16:22

              > Вообще-то interface segregation principle как раз о том, что могут. Парсел это парсел, урджент это урджент. Друг другу они мешать не должны.

              Ну как-бы да, но нет. Если у вас на вход UrgentParselAwareProcessor приходит Object, а вы его кастите к случайным интерфейсам — это уже не ISP. А вот если у вас на вход приходит UrgentInterface, который наследует(!) ParselInterface — тогда вы имеете полное право откастить к предку и передать в ParselProcessor.

              А без наследования — где у вас прописано что к чему кастить? Завязка на детали реализации.

              ISP — это когда UrgentParselAwareProcessor использует UrgentInterface, но не ParselInterface, а ParselInterface использует ParselProcessor. А новый отдельный интерфейс под каждую новую проперть делать — это что-то другое.

              Т.е. тогда

              interface UrgentInterface {
                public function isUrgent() : bool;
                public function getBarcode() : string;
                public function setBarcode(string $barcode);
                public function arrivedToRecipient();
              }


              1. debesha
                18.09.2018 16:35

                Немного переходит в религиозный спор, не хочу настаивать.


                1. areht
                  18.09.2018 22:41

                  Если SOLID каждый понимает по-своему, то он смысла не имеет.


  1. areht
    17.09.2018 23:33

    > Гораздо лучше создать новый класс UrgentParcel, и не нужно будет менять ни интерфейс, ни класс Parcel.

    Вы, кстати, соберите полностью получившиеся исходники со всей копипастой и сравните варианты (без оглядки на DRY и SOLID). Вы уверены, что получилось лучше?


  1. bores
    18.09.2018 07:28

    На мой взгляд это типичная ошибка, уходящая корнями на студенческую скамью с примерами по ООП типа иерархия фигур или животных. SOLID — это архитектурный принцип и больше относится к интерфейсной части приложения, нежели к данным.
    А у вас в примере как раз структура данных. Так можно и до проблемы ромба в наследовании дойти :)


    1. Basim108 Автор
      18.09.2018 07:29

      Спасибо за комментарий, но позволю себе не согласиться с Вами относительно

      и больше относится к интерфейсной части приложения, нежели к данным.


    1. MaxVetrov
      18.09.2018 10:32

      Вы что-то путаете. Где здесь про UI? https://ru.wikipedia.org/wiki/SOLID


      1. bores
        18.09.2018 11:26

        Прошу простить строго бекэндщика, для меня интерфейс то, что светит наружу (интерфейс класса, API компонета и т.п.).


        1. MaxVetrov
          18.09.2018 11:41

          Да, у слова «интерфейс» много обличий =)


      1. MaxVetrov
        18.09.2018 16:15

        Минусанули-то, почему?


  1. adev_one
    18.09.2018 09:40

    На мой взгляд, больше похоже на пример применения SRP, если Parcel и UrgentParcel — это разные, с точки зрения бизнеса, сущности и будут меняться по разным причинам.


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


    Если же это UrgentParcel — просто особый вид Pracel, то почему бы не унаследовать его интерфейс от IParcel, применив в реализации делегирование интерфейса, например? Если переиспользование кода через наследование не устраивает по каким-то причинам


    1. VolCh
      18.09.2018 09:48

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


  1. MaxVetrov
    18.09.2018 11:25

    Однако, если не копировать код, а наследовать его, то изменения метода ArrivedToRecipient в классе Parcel сразу же приведут к изменению поведения класса UrgentParcel, что и будет являться нарушением Open-closed принципа.
    А не будет ли нарушением Open-closed принципа изменение в классе Parcel? Если исправляешь ошибку в Parcel, то она автоматически исправится и в UrgentParcel.


  1. forewar
    18.09.2018 15:42

    Поддержу предыдущие комментарии о том, что принципы программирования — это инструменты, а не самоцель, а также что в классе посылки не должно быть логики.
    Зачем посылке вообще атрибут isUrgent? Это верный путь к конструкциям типа
    if(parcel.isUrgent()) {
    // fast delivery
    } else {
    // usual delivery
    }

    Посылка не должна знать, как надо ее доставлять. Эту задачу на себя берёт «доставщик». Обычная посылка и срочная посылка — это одна и та же посылка, которую доставляют разные доставщики. Паттерн стратегия.


  1. isden
    18.09.2018 18:27

    А по-моему, это можно рассматривать как багу проектирования — базовый класс должен содержать все базовые свойства посылки, в том числе и type (в котором у нас может быть urgent|regular|etc).


    1. VolCh
      19.09.2018 13:18

      Не было type на момент проектирования. Когда он появился появилась и дилемма: или просто добавить свойство в существующий класс, модифицировав его, то ли как-то отнаследовавшись.


      1. isden
        19.09.2018 15:01

        Может быть плохо написанное ТЗ? У посылки (физической) есть куча разных свойств, кроме баркода, и довольно странно не описать тип в ТЗ.
        Или мы сейчас абстрактно? Если так, то базовый класс Parcel не трогаем (в нем, по сути, оставляем геттеры/сеттеры и прочие базовые вещи), а от него наследуем обычные и срочные посылки, где уже и играем с логикой.


        1. VolCh
          19.09.2018 15:47

          Ну когда запускали бизнес, то никто не думал о срочных или ещё каких посылках, так что в ТЗ попасть не могло. Ну или заикнулись по типу «пригодится», но увидели смету на разработку и передумали. А потом оказалось, что очень всотребованно, клиенты всегда спрашивают, готовы перплачивать в разы.


          1. isden
            19.09.2018 15:51

            Ну т.е. таки бага в проектировании (== составлении ТЗ) и выходит.
            Если так (т.е. по сути багофикс) — то идем в рефакторинг или начинаем прикручивать костылики. А еще есть интересный паттерн «декоратор».


            1. DrPass
              19.09.2018 21:40

              Ну т.е. таки бага в проектировании (== составлении ТЗ) и выходит.

              Я не знаю, правильно ли говорить о баге в ТЗ в таких случаях. Бизнес — штука живая и постоянно изменяющаяся, и ТЗ на разработку чего-либо, автоматизирующего бизнес, редко когда бывает «железобетонным». Часто это как раз сознательно заложенный процесс, «сейчас делаем так, а потом будем корректировать по мере необходимости».


            1. VolCh
              20.09.2018 12:58

              Тут бага если и есть, то на уровень или на два выше ТЗ. ТЗ формируются на основе постановки бизнес-задач и бизнес-процессоы, а они — на основе анализа бизнес-целей. Если ни на одном из этих уровней при старте проекта в итоговых артефактах не появились понятия «тип посылки», «срочность посылки», то или баги на этих уровнях, или багов вообще нет, а просто изменились требования к процессам и автоматизирующих их продукта(ов) в результате появления новых бизнес-целей или повторного анализа существующих.