Cухой антипаттерн

в 11:08, , рубрики: codesmell, Алгоритмы, Программирование, метки: ,

Долгое время я задумывался, что же не в порядке с некоторыми частями кода. Раз за разом, в каждом из проектов находится некий «особо уязвимый» компонент, который все время «валится». Заказчик имеет свойство периодически менять требования, и каноны agile завещают нам все хотелки воплощать, запуская change request-ы в наш scrum-механизм. И как только изменения касаются оного компонента, через пару дней QA находят в нём несколько новых дефектов, переоткрывают старые, а то и сообщают о полной его неработоспособности в одной из точек применения. Так почему же один из компонентов все время на устах, почему так часто произносится фраза а-ля «опять #компонент# сломался»? Почему этот компонент приводится как антипример, в контексте «лишь бы не получился ещё один такой же»? Из-за чего этот компонент так неустойчив к изменениям?

Когда находят причину, приведшую к, или способствовавшую развитию такого порока в приложении, эту причину обозначают, как антипаттерн.
В этот раз камнем преткновения стал паттерн Strategy. Злоупотребление этим паттерном привело к созданию самых хрупких частей наших проектов. Паттерн сам по себе имеет вполне «мирное» применение, проблема скорее в том, что его суют туда где он подходит, а не туда, где он нужен. «Если вы понимаете о чем я» (с).

Классификация

Паттерн существует в нескольких «обличьях». Суть его от этого меняется не сильно, опасность его применения существует в любом из них.
Первый, классический вид — это некий долгоживущий объект, который принимающий другой объект по интерфейсу, собственно стратегию, через сеттер, при некотором изменении состояния.
Второй вид, вырожденный вариант первого — стратегия принимается один раз, на все время жизни объекта. Т.е. для одного сценария используется одна стратегия, для другого другая.
Третий вид, это исполняемый метод, либо статический, либо в короткоживущем объекте, принимающий в качестве входного параметра стратегию по интерфейсу. У «банды четырёх» этот вид назван как «Template method».
Четвертый вид — интерфейсный, ака UI-ный. Иногда называется как паттерн «шаблон», иногда как «контейнер». На примере веб-разработки — представляет из себя некий, кусок разметки, содержащий в себе плейсхолдер (а то и не один), куда во время исполнения отрендерится изменяемая, имеющая несколько разных реализаций, часть разметки. Параллельно разметке, в JavaScript коде также живут параллельные вью-модели или контроллеры, в зависимости от архитектуры, принятой в приложении, организованные по второму виду.

Общие черты этих всех случаев:
1)некоторая неизменяемая часть ака имеющая одну реализацию, далее, я буду называть ее контейнером
2)изменяемая часть, она же стратегия
3)место использования, создающее/вызывающее контейнер, определяющее какую стратегию контейнер должен использовать, далее буду называть это сценарием.

Развитие болезни

Поначалу, когда компонент, с использованием этого паттерна только реализовывался в проекте, он не казался таким уж плохим. Применен он был, когда было нужно создать две одинаковые страницы(опять же, на примере веб-разработки), которые лишь немного отличаются контентом в середине. Даже наоборот, разработчик порадовался, насколько красиво и изящно получилось реализовать принцип DRY, т.е. полностью избежать дублирования кода. Именно такие эпитеты я слышал о компоненте, когда он был только-только создан. Тот самый, который стал попаболью всего проекта несколькими месяцами позже.
И раз уж я начал теоретизировать, зайду чуть дальше — именно попытки реализовать принцип DRY, через паттерн strategy, собственно как и через наследование, приводят к тьме. Когда в угоду DRY, сам того не замечая, разработчик жертвует принципом SRP, первым и главным постулатом из SOLID. К слову, DRY не является частью SOLID, и при конфликте, надо жертвовать именно им, ибо он не благоприятствует созданию устойчивого кода в отличие от, собственно, SOLID. Как оказалось — скорее даже наоборот. Переиспользование кода должно быть приятным бонусом тех или иных дизайн-решений, а не целью принятия оных.
А соблазн переиспользования возникает, когда заказчик приходит с новой историей на создание третьей страницы. Ведь она так похожа на первые две. Еще этому немало способствует желание заказчика реализовать все «подешевле», Ведь переиспользовать ранее созданный контейнер быстрее, чем реализовать страницу полностью. История попала к другому разработчику, который быстро выяснил, что функционала контейнера недостаточно, а полноценный рефакторинг не вписывается в оценки. Ещё одна из ошибок тут в том, что разработчик продолжает следовать плану, поставленным оценкам, и это происходит «в тишине», ведь ответственности как бы нет, она лежит на всей команде, принявшей такое решение и такую оценку.
И вот в контейнер добавляется новый функционал, в интерфейс стратегии добавляются новые методы и поля. В контейнере появляются if-ы, а в старых реализациях стратегии появляются «заглушки», чтоб не сломать уже имеющиeся страницы. На момент сдачи второй истории, компонент уже был обречен, и чем дальше, тем хуже. Все сложнее разработчикам понимать как он устроен, в том числе тем, которые «совсем недавно в нём что то делали». Все сложнее вносить изменения. Все чаще приходится консультироваться с «предыдущими потрогавшими», чтобы спросить как это работает, почему были внесены некоторые изменения. Все больше вероятность того, что даже малейшее изменение внесёт новый дефект. Собственно уже речь начинает идти о том, что все больше вероятность внесения двух и более дефектов, ибо один дефект появляется уже с околоединичной вероятностью. И вот настает момент, когда новое требования заказчика реализовать невозможно. Выхода два: либо полностью переписать, либо сделать явный хак. А в ангуляре как раз есть подходящий хак-инструмент — можно сделать emit события снизу вверх, затем broadcast сверху вниз, когда закончил грязные делишки наверху. Технический долг при этом уже не увеличивается, он и так уже давно равен стоимости реализации этого компонента с нуля.

Сухая альтернатива

Наследование нередко порицается, а корпорация добра в своем языке Go вообще решила обойтись без него, и как мне кажется, негатив к наследованию частично исходит из опыта реализации принципа DRY через него. «Стратежный» DRY так же приводит к печальным результатам. Остается прямая агрегация. Для иллюстрации я возьму простой пример и покажу как он может быть представлен в виде стратегии, то есть шаблонного метода и без него.
Допустим у нас есть два сильно похожих сценария, представленных следующим псевдокодом:
В них повторяются 10 строчек X в начале и 15 строчек Y в конце. В середине же один сценарий имеет строчки А, другой — строчки B

СценарийА{
строчка X1
...строчка X10
Строчка А1
...Строчка А5
строчка Y1
...Строчка Y15
}

СценарийВ{
строчка X1
...строчка X10
Строчка B1
...Строчка B3
строчка Y1
...Строчка Y15
}

Вариант избавления от дублирования через стратегию

Контейнер{
строчка X1
...строчка X10
ВызовСтратегии()
строчка Y1
...Строчка Y15
}

СтратегияА{
Строчка А1
...Строчка А5
}

СтратегияВ{
Строчка B1
...Строчка B3
}

СценарийА{
Контейнер(new СтратегияА)
}

СценарийВ{
Контейнер(new СтратегияB)
}

Вариант через прямую агрегацию

МетодХ{
строчка X1
...строчка X10
}

МетодY{
строчка Y1
...Строчка Y15
}

МетодА{
Строчка А1
...Строчка А5
}

МетодВ{
Строчка B1
...Строчка B3
}

СценарийА{
МетодХ()
МетодA()
МетодY()
}

СценарийВ{
МетодХ()
МетодB()
МетодY()
}

Здесь предполагается, что все методы в разных классах.
Как я уже говорил, на момент реализации, первый вариант смотрится не так уж и плохо. Недостаток его не в том, что он изначально плох, а в том, что он неустойчив к изменениям. И, все же, хуже читается, хотя на простом примере это может быть и не очевидно. Когда нужно реализовать третий сценарий, который похож на первые два, но не на 100%, возникает желание переиспользовать код, содержащийся в контейнере. Но его не получится переиспользовать частично, можно лишь взять его целиком, поэтому приходится вносить изменения в контейнер, что сразу несет риск сломать другие сценарии. Тоже самое происходит, когда новое требование предполагает изменения в сценарии А, но это не должно затрагивать сценарий B. В случае же с агрегацией, метод X можно с легкостью заменить на метод X' в одном сценарии, совершенно не затрагивая другие. При этом нетрудно предположить, что методы X и X' могут также почти полностью совпадать, и их также можно подразбить. При «стратежном» подходе, если каскадировать таким же «стратежным» образом, то зло, помещаемое в проект возводится во вторую степень.

Когда можно

Многие примеры использования паттерна strategy лежат на виду, и часто используются. Их все объединяет одно простое правило — в контейнере нет бизнес логики. Совсем. Там может быть алгоритмическое наполнение, например хэш-таблица, поиск или сортировка. Стратегия же содержит бизнес-логику. Правило, по которому один элемент равен другому или больше-меньше есть бизнес-логика. Все операторы linq также являются воплощением паттерна, например оператор .Where() также является шаблонным методом, и принимаемая им лямбда есть стратегия.
Кроме алгоритмического наполнения, это может быть наполнение связанное с внешним миром, асинхронные запросы например, или, в примере от «банды четверых» — подписка на событие нажатия мыши. То что называют коллбэками по сути есть та же стратегия, надеюсь мне простят все мои гипер-обобщения. Также, если речь идет о UI, то это могут быть табы, или всплывающее окно.
Словом, это может быть что угодно, полностью абстрагированное от бизнес-логики.
Если же вы в разработке используете паттерн strategy, и в контейнер бизнес-логика попала — знайте, линию вы уже перешагнули, и стоите по щиколотку в… ммм, болоте.

Запахи

Иногда непросто понять, где черта между бизнес-логикой и общими задачами программирования. И поначалу, когда компонент только создан, определить, что он в будущем принесет геморроя, непросто. Да и если бизнес-требования никогда не будут меняться, то этот компонент может никогда и не всплыть. Но если изменения будут, то неизбежно будут появляться следующий code-smells:
1. Количество передаваемых методов. Параметр обсуждаемый, сам по себе не вредный, но таки может намекнуть. Два-три еще нормально, но если в стратегии содержится с десяток методов, то наверное что-то уже не так.
2. Флаги. Если в стратегии кроме методов есть поля/свойства, стоит обратить на то как они называются. Такие поля как Name, Header, ContentText — допустимы. Но если видите такие поля как SkipSomeCheck, IsSomethingAllowed, это означает, что стратегия уже попахивает.
3. Условные вызовы. Cвязано с флагами. Если в контейнере есть похожий код, значит вы уже ушли в болото по пояс

if(!strategy.SkipSomeCheck)
{
  strategy.CheckSomething().
}

4. Неадекватный код. На примере из JavaScript —

if(strategy.doSomething)

Из названия видно, что doSomething это метод, а проверяется как флаг. То есть разработчики поленились создать флаг, обозначающий тип, а использовали в качестве флага наличие/отсутствие метода, причем внутри блока if даже не он вызывался. Если вы встречаете такое, знайте — компонент уже по горло в техническом долге.

Заключение

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

Автор: VladVR

Источник

Поделиться новостью

* - обязательные к заполнению поля