Комментарии 53
Вы не первый, кто поднимает такую тему. Примерно те же мысли, но для конфигов Nginx, есть тут: habr.com/company/oleg-bunin/blog/313666
А на работу срочных посылок никто не жаловался
Пример плохой — 95% пожалуются
Или как не раз видел — при таком подходе копирования «не жалуются потому что работают с еще одной копией такого же класса SpeedParcel где все исправлено, и про эту реализацию UrgentParcel никто даже не слышал»
Очень интересно мнение сообщества по этому вопросу. Заранее благодарен за ваши комментарии.
Моё мнение, например, SOLID — это не самоцель, а инструмент. Инструмент, который должен упрощать сопровождение кода. Если речь идет о добавлении метода в интерфейс, который затем требует перепроектирования вызовов в массе мест программы, то есть смысл добавить новый интерфейс. Наш инструмент работает, мы ему следуем.
Если такая проблема не стоит, интерфейс ещё не задействован во множестве мест, требующих перепроектирования, то следуя нашему инструменту, мы вместо одной простой компоненты порождаем две, каждая из которых нуждается в сопровождении. Это наоборот, усложняет код. Значит, тут SOLID не справляется со своей задачей. Значит, мы не должны ему следовать.
Опять же-таки, нет единого шаблона относительно
. Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.
Каждый такой случай нуждается в индивидуальной оценке.
Кране против такого предложения.
Если один код скопировали в 10 различных мест, а при изменении требований изменили только в одном, то стоит ожидать в трекере 9 новых багов.
В описываемом вами случае можно ввести некий GenericParcel: IParcel, куда сложить всю логику, которая не зависит от типа посылок и затем уже наследовать Parcel и UrgentParcel от него
Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.
Не согласен. Это повод провести дополнительное тестирование и четко определить как именно должна работать данная логика для срочных посылок
Это повод провести дополнительное тестирование и четко определить как именно должна работать данная логика для срочных посылок
Это правда, так нужно делать обязательно. Однако набор тестов (тест-кейсов) будет зависить от того кода который тестируем, и я думаю комментарии c пометкой какие классы вовлечены в копи-пасту помогли бы как-раз определить этот набор тестов. Да и вообще подобные комментарий откуда/куда копировалось, помогают держать копи-паст под контролем.
В описываемом вами случае можно ввести некий GenericParcel: IParcel, куда сложить всю логику, которая не зависит от типа посылок и затем уже наследовать Parcel и UrgentParcel от негоВ случаи, когда мы знаем что надо проектировать одновремено обычные посылки и срочные, то согласен. Но в случаи когда срочные посылки появились позже, рефакторить и как либо менять класс Parcel было бы не разумным риском обрушить модули работы обычных посылок.
P.S. Представьте микросервисную архитектуру, в случаи когда обычные посылки работают в своем микросервисе, мы реализовываем работу срочных посылок в отдельном своем микросервисе и что делать с кодом ела копировать? я думаю это было бы разумнее, чем менять микросервис обычных посылок.
Но в случаи когда срочные посылки появились позже, рефакторить и как либо менять класс Parcel было бы не разумным риском обрушить модули работы обычных посылок.
Ну тут два варианта — рефакторить или пилить костыли. Если мы боимся трогать существующий код, то речь о соблюдении архитектурных принципов я бы вообще не ставил
Ну вообще и тут можно применить если не классическое наследование, то композицию: третий общий сервис к которому первые два просто проксируют часть вызовов или декорируют их. Это, конечно, увеличит сложность системы в целом, поэтому в каждом конкретном случае надо оценивать. плюсы и минусы. Навскидку, в примере с посылками очень хорошо бы минимизировать копи-пасту, поскольку предметная область довольно зарегулирована и вероятно регуляция будет усиливаться, причём без особого разделения на срочное и нет.
> Да и вообще подобные комментарий откуда/куда копировалось, помогают держать копи-паст под контролем.
Нет. Вообще нет. Что толку от ваших комментариев после того, как «исправлять надо именно работу обычных посылок»? А что толку до этого момента?
> Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.
Вот ведь как интересно получается:
— Если ВЫ откопипастили, то починяться только обычные посылки
— Если ВЫ не откопипастили, то починяться все посылки
То есть вы в момент копипасты предсказали скоп ошибки!
Ну и да, понимание OCP у вас… спорное.
А из примера не понятно чем именно UrgentParcel отличается, что бы заслужить отдельного класса.
Нет. Вообще нет. Что толку от ваших комментариев после того, как «исправлять надо именно работу обычных посылок»? А что толку до этого момента?
> Если найдена ошибка в методе ArrivedToRecipient при работе с обычными посылками, то и исправлять надо именно работу обычных посылок. А на работу срочных посылок никто не жаловался и, вероятно, всех устраивает, как срочные посылки работают.
Вот ведь как интересно получается:
— Если ВЫ откопипастили, то починяться только обычные посылки
— Если ВЫ не откопипастили, то починяться все посылки
То есть вы в момент копипасты предсказали скоп ошибки!
Ну и да, понимание OCP у вас… спорное.
А из примера не понятно чем именно UrgentParcel отличается, что бы заслужить отдельного класса.
Мне одному кажется, что public string Barcode {get; set;} — это верный признак DTO, в котором никакой логики не должно быть в принципе?
Почему только «O»? Если этот принцип возвести в абсолют, как и любой другой, так он все что угодно будет нарушать. И еще по моему как то упрощенно понят open/closed. Он запрещает изменять поведение в наследниках класса/интерфейса таким вот несовместимым образом. Т е формальные аргументы и возвращаемое значение не меняются, но при этом меняется семантика, так что клиенты IParcel более не могут работать с этим новым классом. Как пример он меняет внутренние состояние объекта так, как это не предполагается в других реализациях или добавляет стэйт-машину как в статье, которой изначально в интерфейсе не предполагалось. В таких случаях надо принять либо факт ошибки проектирования, либо выход функциональных требований за изначально заложенные рамки и рефакторить (или делать новый интерфейс со всеми вытекающими) Dry или не Dry — тут не поможет, т к существующие клиенты IParcel ничего не знают о том, что эта посылка может быть в нескольких состояниях.
Спасибо Вам за комментарий,
Т е формальные аргументы и возвращаемое значение не меняются, но при этом меняется семантика, так что клиенты IParcel более не могут работать с этим новым классом.Тут Вы скорее описываете нарушение принципа замещения Лисков.
P.S. Я не хотел описывать сразу весь интерфейс IParcel со всеми его свойствами и методами, дабы читателю было проще читать. Метод ArrivedToRecipient был добавлен не только в класс, он, как я предполагал, но не описал в статье, был изначально у IParcel и его реализация есть в классе Parcel. Так вот вопрос обсуждался стоит ли копи/пастить тело метода в класс UrgentParcel.
Понял. Наверное зря опустили другие методы интерфейса. Стало менее понятно.
По вопросу о DRY мое решение такое: если метод в две — три строки и не содержит сильно сложных или уникальных алгоритмов, то буду настаивать на копировании. Если же там что то объемное, сложная логика буду пробовать предлагать вариант воспользоваться одним из паттернов инкапсуляции поведения — стратегия, интерпретатор и тд. Если и это не выходит — что ж надо подумать как можно было бы изменить интерфейс. Если изменения не приемлемы с точки зрения затрат или нет тех.возможности — ну тогда миримся с этим и нарушаем dry.
По поводу исправления ошибок — я бы предложил команде тестирования создать доп. тест- кейсы и если при этом они не найдут ошибок — явно указать на возможность таковых и проконсультироваться с заказчиком на эту тему — если и тогда ошибок не будет — что ж мы что то упустили в проектировании — посылки ведут себя сильно различно и это различие обнаружилось неявно ( посредством бага) — крайне не удобная ситуация, способная кипятить мозги на протяжении долгого времени (если конечно код там боевой и постоянно меняется из-за развития). Если таки баг подтвердят — исправлять оба алгоритма и возможно все же вынести в стратегию (раз уж там что то, где можно баг посеять). Не всегда у меня получается действовать по описанному алгоритму, но ни разу не было что бы хоть одно действие в нем было сделано зря (тесты лишними не бывают, консультация с заказчиком — тоже — обычно что то да вылезает в результате =)
По вопросу о DRY мое решение такое: если метод в две — три строки и не содержит сильно сложных или уникальных алгоритмов, то буду настаивать на копировании. Если же там что то объемное, сложная логика буду пробовать предлагать вариант воспользоваться одним из паттернов инкапсуляции поведения — стратегия, интерпретатор и тд. Если и это не выходит — что ж надо подумать как можно было бы изменить интерфейс. Если изменения не приемлемы с точки зрения затрат или нет тех.возможности — ну тогда миримся с этим и нарушаем dry.
По поводу исправления ошибок — я бы предложил команде тестирования создать доп. тест- кейсы и если при этом они не найдут ошибок — явно указать на возможность таковых и проконсультироваться с заказчиком на эту тему — если и тогда ошибок не будет — что ж мы что то упустили в проектировании — посылки ведут себя сильно различно и это различие обнаружилось неявно ( посредством бага) — крайне не удобная ситуация, способная кипятить мозги на протяжении долгого времени (если конечно код там боевой и постоянно меняется из-за развития). Если таки баг подтвердят — исправлять оба алгоритма и возможно все же вынести в стратегию (раз уж там что то, где можно баг посеять). Не всегда у меня получается действовать по описанному алгоритму, но ни разу не было что бы хоть одно действие в нем было сделано зря (тесты лишними не бывают, консультация с заказчиком — тоже — обычно что то да вылезает в результате =)
Однако, если не копировать код, а наследовать его, то изменения метода 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). Там и версионирование можно устроить, и комбинаторного взрыва избежать, да и качество абстракций получается более высоким.
А как насчёт того, чтобы вынести метод ArrivedToRecipient в базовый абстрактный класс GenericParcel и сделать его виртуальным? Кто захочет — переиспользует логику (как в случае с UrgentParcel), кому нужно — переопределит (Parcel). Тогда и копипасты не будет, и open-closed principle не будет нарушен. То есть, если у наследников изначально есть общая логика, то очевидно, что она должна лежать в базовом абстрактном классе, а не быть скопированной повсюду.
Сорри за повтор, товарищ APXEOLOG уже выше написал об этом
Сорри за повтор, товарищ APXEOLOG уже выше написал об этом
Гораздо лучше создать новый класс UrgentParcel, и не нужно будет менять ни интерфейс, ни класс Parcel.
Это серьезно? Повбывав бы…
Всё же, копирование кода — крайне ужасная практика. Пару-тройку копий вы ещё сможете отслеживать, но их количество будет расти вслед за новыми требованиями и в разных частях проекта. Рано или поздно предел контроля будет превышен и начнётся опасный хаос.
Мне кажется, если DRY не стыкуется с SOLID, то стоит посоветоваться с GoF. Данный пример очень напоминает ситуацию в красках описанную в «Head First: Design Patterns» в главе о Strategy.
Мне кажется, если DRY не стыкуется с SOLID, то стоит посоветоваться с GoF. Данный пример очень напоминает ситуацию в красках описанную в «Head First: Design Patterns» в главе о Strategy.
Согласен! И т.к. нарастание копи-пастов не происходит организованно, комментари о том откуда/куда скопированно будет уместен, чтобы понимать сколько раз копировали. И если возникает надобность третий раз коппировать, то нужно помечать код опасным и кондидатом на рефакторинг.
Мне кажется, если DRY не стыкуется с SOLID, то стоит посоветоваться с GoF.Точно!!! :)
На мой взгляд, «комментарий в коде» и перфекционисты — несовместимые вещи в данном случае.
Разве не каноничнее в таком случае сделать класс-родитель metaParcel, в нем реализовать общее для обоих видов посылок, от него унаследовать Parcel и urgentParcel и т.д.? И не надо будет комментировать, где у вас там что надо менять, если «вдруг чо».
Разве не каноничнее в таком случае сделать класс-родитель metaParcel, в нем реализовать общее для обоих видов посылок, от него унаследовать Parcel и urgentParcel и т.д.? И не надо будет комментировать, где у вас там что надо менять, если «вдруг чо».
для формального соблюдения solid можно, конечно, ввести новый интерфейс, наследующий старый, сделать новый класс-реализацию, который является композитом экземпляров старой реализации и класса с реализацией доп. функциональности. Наиболее простым код от этого, скорее всего, не станет. Как правило, практичнее версионировать интерфейсы
Не понимаю, почему тут нужно жертвовать чем-то.
В рамках описанной эволюции всё делается и SOLID и DRY
Я переведу это в PHP, но суть не меняется.
Сначала была просто посылка.
Потом нас просят добавить urgent фичу, мы расширяем (но не изменяем и не дублицируем) наш код.
Что там дальше идёт? Доставка реципиенту? Тут нужно уточнить поведение, и либо мы расширяем базовый интерфейс ParselInterface (если фича общая для всех посылок), либо добавляем новый. Соответсвенно, имплементация будет или на уровне родительского Parsel, либо нет. Допустим, первое.
Наш код будет уже
И никакого нарушения тут нет. arrivedToRecipient добавился к UrgentParsel, и изменения его текущего поведения не произошло. Extended, but not modified.
А уже после этого — нельзя просто так менять arrivedToRecipient(). Как раз таки потому что O.
Где тут что нарушается?
В рамках описанной эволюции всё делается и 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.
Где тут что нарушается?
НЛО прилетело и опубликовало эту надпись здесь
Скажите, вот есть код, он принимает массив ParselInterface. После ваших действий он Extended, but not modified?
Или OCP ограничивается классом Parsel?
Или OCP ограничивается классом Parsel?
Ну конечно not modified. Если был класс, который работал с массивом ParselInterface, он будет всё так же работать с массивом ParselInterface и поддерживать все существующие к этому моменту фичи.
Появление нового интерфейса (и нового кода, знающего, как работать с ним) не требует ни одного изменения в существующем коде и существующих features.
В чем violation?
Появление нового интерфейса (и нового кода, знающего, как работать с ним) не требует ни одного изменения в существующем коде и существующих features.
В чем violation?
Подозреваю, что именно существующий код то и придется модифицировать для поддержки UrgentInterface, иначе срочные посылки будут обрабатываться как обычные. Так что там нет «O», поэтому не будет и «C».
Да и вообще знание, что реализации ParselInterface могут(?) реализовывать UrgentInterface несколько нарушает инкапсуляцию. В публичном контракте это тайное знание не описано
Да и вообще знание, что реализации ParselInterface могут(?) реализовывать UrgentInterface несколько нарушает инкапсуляцию. В публичном контракте это тайное знание не описано
> ParselInterface могут(?) реализовывать UrgentInterface
Вообще-то interface segregation principle как раз о том, что могут. Парсел это парсел, урджент это урджент. Друг другу они мешать не должны.
> Подозреваю, что именно существующий код то и придется модифицировать для поддержки UrgentInterface, иначе срочные посылки будут обрабатываться как обычные.
Существующий код тоже должен расширяться. Поведение существующих классов меняться не должно, но должны добавляться новые классы, которые будут реализовывать новые или существующие интерфейсы.
Т.е. говоря кодом, одновременно с ParselProcessor, реализующим ParselProcessorInterface должен появиться UrgentParselAwareProcessor, реализующий тот же интерфейс и он инджектится вместо существующего ParselProcessor
Вообще-то interface segregation principle как раз о том, что могут. Парсел это парсел, урджент это урджент. Друг другу они мешать не должны.
> Подозреваю, что именно существующий код то и придется модифицировать для поддержки UrgentInterface, иначе срочные посылки будут обрабатываться как обычные.
Существующий код тоже должен расширяться. Поведение существующих классов меняться не должно, но должны добавляться новые классы, которые будут реализовывать новые или существующие интерфейсы.
Т.е. говоря кодом, одновременно с ParselProcessor, реализующим ParselProcessorInterface должен появиться UrgentParselAwareProcessor, реализующий тот же интерфейс и он инджектится вместо существующего ParselProcessor
> Вообще-то interface segregation principle как раз о том, что могут. Парсел это парсел, урджент это урджент. Друг другу они мешать не должны.
Ну как-бы да, но нет. Если у вас на вход UrgentParselAwareProcessor приходит Object, а вы его кастите к случайным интерфейсам — это уже не ISP. А вот если у вас на вход приходит UrgentInterface, который наследует(!) ParselInterface — тогда вы имеете полное право откастить к предку и передать в ParselProcessor.
А без наследования — где у вас прописано что к чему кастить? Завязка на детали реализации.
ISP — это когда UrgentParselAwareProcessor использует UrgentInterface, но не ParselInterface, а ParselInterface использует ParselProcessor. А новый отдельный интерфейс под каждую новую проперть делать — это что-то другое.
Т.е. тогда
Ну как-бы да, но нет. Если у вас на вход 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();
}> Гораздо лучше создать новый класс UrgentParcel, и не нужно будет менять ни интерфейс, ни класс Parcel.
Вы, кстати, соберите полностью получившиеся исходники со всей копипастой и сравните варианты (без оглядки на DRY и SOLID). Вы уверены, что получилось лучше?
Вы, кстати, соберите полностью получившиеся исходники со всей копипастой и сравните варианты (без оглядки на DRY и SOLID). Вы уверены, что получилось лучше?
На мой взгляд это типичная ошибка, уходящая корнями на студенческую скамью с примерами по ООП типа иерархия фигур или животных. SOLID — это архитектурный принцип и больше относится к интерфейсной части приложения, нежели к данным.
А у вас в примере как раз структура данных. Так можно и до проблемы ромба в наследовании дойти :)
На мой взгляд, больше похоже на пример применения SRP, если Parcel и UrgentParcel — это разные, с точки зрения бизнеса, сущности и будут меняться по разным причинам.
Тогда тот факт, что в них один и тот же код — случайность и скоро их код может стать совершенно разным.
Если же это UrgentParcel — просто особый вид Pracel, то почему бы не унаследовать его интерфейс от IParcel, применив в реализации делегирование интерфейса, например? Если переиспользование кода через наследование не устраивает по каким-то причинам
НЛО прилетело и опубликовало эту надпись здесь
Поддержу предыдущие комментарии о том, что принципы программирования — это инструменты, а не самоцель, а также что в классе посылки не должно быть логики.
Зачем посылке вообще атрибут isUrgent? Это верный путь к конструкциям типа
Посылка не должна знать, как надо ее доставлять. Эту задачу на себя берёт «доставщик». Обычная посылка и срочная посылка — это одна и та же посылка, которую доставляют разные доставщики. Паттерн стратегия.
Зачем посылке вообще атрибут isUrgent? Это верный путь к конструкциям типа
if(parcel.isUrgent()) {
// fast delivery
} else {
// usual delivery
}
Посылка не должна знать, как надо ее доставлять. Эту задачу на себя берёт «доставщик». Обычная посылка и срочная посылка — это одна и та же посылка, которую доставляют разные доставщики. Паттерн стратегия.
А по-моему, это можно рассматривать как багу проектирования — базовый класс должен содержать все базовые свойства посылки, в том числе и type (в котором у нас может быть urgent|regular|etc).
Не было type на момент проектирования. Когда он появился появилась и дилемма: или просто добавить свойство в существующий класс, модифицировав его, то ли как-то отнаследовавшись.
Может быть плохо написанное ТЗ? У посылки (физической) есть куча разных свойств, кроме баркода, и довольно странно не описать тип в ТЗ.
Или мы сейчас абстрактно? Если так, то базовый класс Parcel не трогаем (в нем, по сути, оставляем геттеры/сеттеры и прочие базовые вещи), а от него наследуем обычные и срочные посылки, где уже и играем с логикой.
Или мы сейчас абстрактно? Если так, то базовый класс Parcel не трогаем (в нем, по сути, оставляем геттеры/сеттеры и прочие базовые вещи), а от него наследуем обычные и срочные посылки, где уже и играем с логикой.
Ну когда запускали бизнес, то никто не думал о срочных или ещё каких посылках, так что в ТЗ попасть не могло. Ну или заикнулись по типу «пригодится», но увидели смету на разработку и передумали. А потом оказалось, что очень всотребованно, клиенты всегда спрашивают, готовы перплачивать в разы.
Ну т.е. таки бага в проектировании (== составлении ТЗ) и выходит.
Если так (т.е. по сути багофикс) — то идем в рефакторинг или начинаем прикручивать костылики. А еще есть интересный паттерн «декоратор».
Если так (т.е. по сути багофикс) — то идем в рефакторинг или начинаем прикручивать костылики. А еще есть интересный паттерн «декоратор».
Ну т.е. таки бага в проектировании (== составлении ТЗ) и выходит.
Я не знаю, правильно ли говорить о баге в ТЗ в таких случаях. Бизнес — штука живая и постоянно изменяющаяся, и ТЗ на разработку чего-либо, автоматизирующего бизнес, редко когда бывает «железобетонным». Часто это как раз сознательно заложенный процесс, «сейчас делаем так, а потом будем корректировать по мере необходимости».
Тут бага если и есть, то на уровень или на два выше ТЗ. ТЗ формируются на основе постановки бизнес-задач и бизнес-процессоы, а они — на основе анализа бизнес-целей. Если ни на одном из этих уровней при старте проекта в итоговых артефактах не появились понятия «тип посылки», «срочность посылки», то или баги на этих уровнях, или багов вообще нет, а просто изменились требования к процессам и автоматизирующих их продукта(ов) в результате появления новых бизнес-целей или повторного анализа существующих.
Очень интересно мнение сообщества по этому вопросу. Заранее благодарен за ваши комментарии.
Ок:) Представим, что помимо срочных писем бывают еще предоплаченные, заказные и с уведомлением. А теперь представим что письмо может быть и срочным и заказным одновременно. В этом случае при использовании наследования возникает классическая ловушка, когда необходимый функционал размещен в другом независимом классе.
В рассматриваемой задаче видится более естественным использовать аттрибуты — некие дополнительные маркеры, содержащиеся в письме, описывающие его доп. свойста.
В наивной реализации можно было бы сделать так:
[Flags]
public enum PostAttrFlags
{
Urgent,
Prepaid,
//...
}
public interface IParcelSimple
{
string Barcode { get; set; }
PostAttrFlags Attributes { get; }
}
public class ParcelSimple : IParcelSimple
{
public string Barcode { get; set; }
public PostAttrFlags Attributes { get; }
}В более углубленной, выделим аттрибут в отдельный интерфейс, где письмо содержит множество (HashSet) таких элементов:
public interface IParcelAttribute
{
}
public struct ParcelAttrPrepaid : IParcelAttribute
{
}
public struct ParcelAttrUrgent : IParcelAttribute
{
}
public interface IParcel
{
string Barcode { get; set; }
HashSet<IParcelAttribute> Attributes { get; }
}
public class Parcel : IParcel
{
public string Barcode { get; set; } = "";
public HashSet<IParcelAttribute> Attributes { get; set; } = new HashSet<IParcelAttribute>();
}Вторая реализация дает интересные следствия:
- Аттрибут может быть обычной пометкой. Для этого достаточно обозваться IParcelAttribute
- Часть бизнес логики реализуется с помощью возможностей строгой типизации и фундаментальных алгоритмов (HashSet гарантирует уникальность аттрибутов и быстрый доступ к конкретному аттрибуту письма, контроль типов гарантирует получение полных сведений о письме на всем протяжении отправки) и т.д.
- Выделение аттрибутов как отдельной сущности, позволяет проделывать с письмами интересные штуки. Например, «Сортировочный центр» может поставить отметку на письмо что оно прибыло на сортировку тогда-то и тогда-то, покинуло сортировку тогда-то и тогда-то и т.д.
- Теситровать такой код легче
Кстати, потестим наш код, что бы убедится в сказанном:
namespace UnitPost
{
[TestClass]
public class UnitTest1
{
[TestMethod]
public void TestIsParcel()
{
//-- Создадим срочную посылку
Parcel p = new Parcel();
p.Attributes.Add(new ParcelAttrUrgent());
//-- Убедимся что это именно срочная посылка
Assert.AreEqual(p.Attributes.Where(t => t.GetType() == typeof(ParcelAttrUrgent)).Any(), true);
}
[TestMethod]
public void TestDblUrgentExcept()
{
//-- Создадим срочную посылку
Parcel p = new Parcel();
p.Attributes.Add(new ParcelAttrUrgent());
//-- Дважды срочную посылку создать не получится
Assert.AreEqual(p.Attributes.Add(new ParcelAttrUrgent()), false);
}
}
}Да, действительно работает.
Я считаю, что главный метапринцип, который должен лежать в проектировании любой архитектуры — это управляемость данной архитектуры. Этому принципу уже подчинены все остальные методологии, типа KISS, DRY, SOLID; шаблоны проектирования и т.д.
Если приложение небольшое и за использованием копипасты можно следить (управлять, контролировать) — копипаста это отличное решение. Это экономит время и средства. Но это не ваш случай, похоже, хотя именно его и реализовали)
Основная проблема примера в том, что все то, что должно находиться в одном месте у вас разносится по разным классам. В данном случае — это метод изменения статуса посылки. Эта проблема не разрешима в рамках 2х представленных классов. А значит она должна быть разрешена ВНЕ классов Parcel и UrgentParcel. Достаточно создать отдельный (статический или обычный, двойная диспетчеризация как у визитора или просто вызов нужного метода — это дело вашего вкуса) класс, который будет отвечать за реализацию всех методов изменения статусов у всех типов отправлений. В итоге получаем, что весь код для изменения статусов находится в одном месте:
У любой программной системы есть внешние ограничения, которые являются первичными по отношению к ней. Например, это требования бизнес-процессов, которые накладываются на неё. Также, у любой архитектуры есть внутренние рамки, в которых она может меняться — именно для этих рамок действуют все методологии и в частности принципы SOLID. Если в систему приходят внешние требования, которые не укладываются в возможности текущей архитектуры, то у нас два выхода — либо костыли (наше все), либо частичное перепроектирование архитектуры (недостижимая мечта). Когда мы выбираем второе, то на момент проектирования некоторые принципы SOLID из старой системы не действуют в новой — нам приходится менять базовые классы, их поведение и т.д. Чаще всего нарушается принцип Open-Closed.
В частности, студенческое решение вида — «а добавим мы к базовому классу поле bool IsUrgent {get; set;}» является частичным перепроектированием архитектуры, на который принцип Open-Closed не распространяется. Нужно ли оно или нет, правильное — или нет — это все зависит от того, как, где и зачем эксплуатируется система, кто её поддерживает и т.д.
Если приложение небольшое и за использованием копипасты можно следить (управлять, контролировать) — копипаста это отличное решение. Это экономит время и средства. Но это не ваш случай, похоже, хотя именно его и реализовали)
Основная проблема примера в том, что все то, что должно находиться в одном месте у вас разносится по разным классам. В данном случае — это метод изменения статуса посылки. Эта проблема не разрешима в рамках 2х представленных классов. А значит она должна быть разрешена ВНЕ классов Parcel и UrgentParcel. Достаточно создать отдельный (статический или обычный, двойная диспетчеризация как у визитора или просто вызов нужного метода — это дело вашего вкуса) класс, который будет отвечать за реализацию всех методов изменения статусов у всех типов отправлений. В итоге получаем, что весь код для изменения статусов находится в одном месте:
- Алгоритмы работы методов для класса легко «просматриваемые»
- Копипаст убирается отдельными реализациями различных закрытых методов внутри класса-стратегии
- Принцип SPR прекрасно реализован)
У любой программной системы есть внешние ограничения, которые являются первичными по отношению к ней. Например, это требования бизнес-процессов, которые накладываются на неё. Также, у любой архитектуры есть внутренние рамки, в которых она может меняться — именно для этих рамок действуют все методологии и в частности принципы SOLID. Если в систему приходят внешние требования, которые не укладываются в возможности текущей архитектуры, то у нас два выхода — либо костыли (наше все), либо частичное перепроектирование архитектуры (недостижимая мечта). Когда мы выбираем второе, то на момент проектирования некоторые принципы SOLID из старой системы не действуют в новой — нам приходится менять базовые классы, их поведение и т.д. Чаще всего нарушается принцип Open-Closed.
В частности, студенческое решение вида — «а добавим мы к базовому классу поле bool IsUrgent {get; set;}» является частичным перепроектированием архитектуры, на который принцип Open-Closed не распространяется. Нужно ли оно или нет, правильное — или нет — это все зависит от того, как, где и зачем эксплуатируется система, кто её поддерживает и т.д.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Так ли хорош DRY или все же он может нарушать O из SOLID