Оптимизация кода и развитие микросервисной архитектуры занимает значительную часть жизни команды разработчиков МВидео-Эльдорадо. Тем любопытней изучить опыт коллег за рубежом. Предлагаем вашему вниманию очередной пост на тему: «А как там у них».

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

Все проблемы в computer science можно решить ещё одним уровнем абстракции… Кроме проблемы слишком большого количества уровней абстракций. — Батлер Лэмпсон

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

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

Описанные в посте идеи основываются на принципах Keep it Simple, Stupid (KISS) и You Aren’t Gonna Need It (YAGNI), означающих, что мы стремимся минимизировать абстракции и добавлять сложность только тогда, когда она обеспечивает существенную и реальную выгоду. Эти идеи применимы к большинству типов разработки ПО.


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

Распространённые преждевременные абстракции


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

  1. Слишком мелкая детализация ответственностей
  2. Применение шаблонов проектирования без реального выигрыша
  3. Преждевременная оптимизация производительности
  4. Повсеместное внедрение слабого связывания

Давайте внимательнее взглянем на каждую по отдельности.

1. Слишком мелкая детализация ответственностей


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

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

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

Так когда же нужно разделять ответственности? Распространённый и крайне подходящий случай: когда логику нужно использовать в нескольких местах. Если один и тот же HTTP-вызов или запрос к базе данных необходим в нескольких местах кода, то дублирование логики часто снижает удобство поддержки. В таком случае перенос кода в общий многократно используемый компонент, скорее всего, будет хорошей идеей. Главное не делать этого прежде, чем станет необходимо. Ещё один подходящий случай: когда логика очень сложна и отрицательно влияет на читаемость окружающего кода. Если элемент логики занимает триста строк кода, это приемлемо, но в случае всего нескольких строк это только ухудшит читаемость и усложнит ориентирование в коде. Помните, что разделение ответственностей всегда добавляет в код больше структурной сложности.

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


Слева мы поместили логику класса Service непосредственно в Command Handler, которому она необходима. Справа мы перенесли запрос к базе данных в Repository непосредственно в Event Handler, которому он нужен.

2. Применение шаблонов проектирования без реального выигрыша


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

Хорошим примером этого является шаблон «Декоратор». Этот шаблон часто используется для добавления дополнительной функциональности поверх имеющегося компонента. Это может быть компонент, выполняющий HTTP-запрос, к которому нам нужно добавить механизм повторного запроса. Без изменения исходного компонента мы можем добавить новый компонент, оборачивающий исходный в добавленную поверх него логику повторного запроса. Реализовав тот же интерфейс, его можно заменить для исходного компонента напрямую или через внедрение зависимости.

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

Ещё один часто применяемый шаблон — это «Команда» и «Издатель-подписчик». В нём класс вместо непосредственной обработки запроса абстрагирует его в команду, которая должна обрабатываться в другом месте. Примером этого может быть API-контроллер, отображающий HTTP-запросы в команды и публикующий их, чтобы они обрабатывались соответствующим обработчиком, подписанным на эту конкретную команду. Это обеспечивает слабое связывание и чёткое разделение между частью кода, получающей и интерпретирующей запросы, и частью, знающей, как обрабатывать запросы. Разумеется, существуют подходящие случаи применения такого шаблона, но правильно будет задаться вопросом, не является ли он на практике просто бесполезным слоем отображения. Слоем отображения, ещё сильнее запутывающим отслеживание пути исполнения программы, так как издатель, по определению шаблона, не знает, где выполняется обработка команды.

И это только пара примеров шаблонов проектирования, которые часто используют преждевременно. То же самое можно сказать почти о любом из шаблонов. У всех них есть недостатки, поэтому применяйте шаблон только тогда, когда его преимущества необходимы и перевешивают минусы. Ниже показано, как повлияло на нашу исходную архитектуру устранение необязательных шаблонов проектирования. Слева мы удалили шаблон «Декоратор», а справа весь поток выполнения команд, в том числе и механизм издателя/подписчика.


Удаление преждевременно добавленных шаблонов проектирования. Слева удалён шаблон «Декоратор». Справа удалён шаблон «Команда» и «Издатель-подписчик».

3. Преждевременная оптимизация производительности


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

Преждевременная оптимизация — корень всех зол — Дональд Кнут

Оптимизации могут вноситься и на архитектурном уровне. Одним из примеров этого является шаблон Command Query Responsibility Segregation (CQRS). По сути, CQRS означает, что у нас имеются две отдельные модели данных, одна из которых используется для обновления данных, а другая для чтения данных, что разделяет приложение на части чтения и записи. Это позволяет оптимизировать одну часть для эффективного чтения, а другую — для эффективной записи, а также масштабировать одну из частей в случае, если приложение особенно активно использует чтение или запись.

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

Ниже показана иллюстрация удаления части чтения из нашего примера выполнения программы. Повторные запросы выполняют считывание не из специальной таблицы Read Model, а непосредственно из Event Source, который в нашем примере является тем, куда данные записываются изначально.


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

4. Повсеместное внедрение слабого связывания


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

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

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

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

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

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


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

Дополнительный совет для любящих рисковать


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

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

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


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

Выполняйте рефакторинг, когда возникнет потребность


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

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

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

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


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

Заключение


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

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

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

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


  1. onets
    05.01.2022 16:14
    +7

    Service interface в большинстве случаев тоже не нужен.


  1. Racheengel
    05.01.2022 16:26
    +42

    Ну в принципе всё правильно. Абстракции ради абстракций и паттерны ради паттернов - это те вещи, которые сразу выдают неопытных разработчиков.

    Но обычно с годами это проходит. После Х лет опыта люди снова начинают писать простой код. И, как правило, он работает намного быстрее и надёжнее нагромождений абстракных фабрик декораторов шаблонных обсерверов :)


    1. pin2t
      05.01.2022 20:19
      +10

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


      1. lea
        05.01.2022 22:39
        +5

        Иногда храбрость есть, а вот начальство говорит "мы не будем тратить на это время".


        1. vitecd
          06.01.2022 06:35
          +3

          тут следует найти соратника ))

          бывшая жена пришла в компанию с 3мя имеющимися разрабами, потом еще 2их приняли, "специалисты", выходцы из сопоставимой с Китаем, по популяции, страны, постоянно конфликтовали с бывшей на уровне фундаметальных знаний... что поддерживалось начальником отдела, киви. Но за год проект очень слабо сдвинулся, наняли еще двух контрактников, киви... Пара совещаний, с участием контрактников, неожиданно, "по семейным обстоятельствам", уволились две специалистки, через месяц еще двое. Сейчас в отделе осталась моя бывшая и один контрактник, и результативность отдела выросла в сотни раз! Ну короче стали появляться результаты. А... к чему это я? Весь код, который наплодили "специалисты", удалили на... и за месяц написали новый. А еще, в течении года, система несколько раз ложилась, причем боевая, из-за того, что "специалисты" инжектировали свой "гениальный код" не проверив, даже не тестовой системе, т.е. оно написало пару процедур, проверило - как бы работает, переосмыслило, переписало, дописало нового еще десяток, ну и... т.к. две недели назад проверка была, кидает в рабочую систему, и .... ах?! да... не работает даже та часть, которая "проверялась". И да, сейчас у них обсуждается таки вопрос удаления и старого, 5-10и летней давности кода, т.к. он не оптимален, но что будет, вопрос... т.к. у бывшей предложение от головной компании, т.е. ей предлагают работу сильно выше, чем её начальница, которая поддерживала все иннициативы "специалистов", и таки да... сейчас она с пеной у рта продвигает любые иннициативы бывшей, чуть ли не кофе ей носит.


        1. MIke_Nomatter
          06.01.2022 07:45
          +1

          иногда приходится сделать небольшой саботаж с целью переубедить начальство)


          1. syrslava
            07.01.2022 14:53

            небольшой саботаж с целью переубедить начальство

            Только чтоб большим не стал


            Короткометражное кино в тему

            английский оригинал — https://www.youtube.com/watch?v=MEOZkf4imaM
            русский дубляж — https://www.youtube.com/watch?v=By4NE1LgigQ


    1. cheevauva
      05.01.2022 21:24
      +4

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

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


      1. vitecd
        06.01.2022 06:37
        +2

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


      1. Racheengel
        06.01.2022 17:27

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

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

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

        ЗЫ. Хотел про интерфейсы ещё отдельно написать, но кто-то карму зело слил, так что наверно только завтра смогу...


        1. BugM
          07.01.2022 05:57

          А при чем тут менеджер? Менеджеры не должны лазить в код никогда. И они не должны что-то пускать или не пускать в прод. Их дело пользовательские фичи, а что там в коде их не касается.

          Есть времена когда скорость важнее. Так бывает. Фигачим в прод, на изоленте держится и ок. Надо не забывать писать беклог и каждые две недели напоминать всем до кого можно дотянуться о необходимости рефакторинга. Потому и потому. С доводами в общем. Бизнес обычно не собирается умирать через год и адекватные рефакторинги одобряет через какое-то время.

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

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


    1. Revertis
      05.01.2022 22:37
      +2

      Хуже только то, что иногда в проект приходят новые молодые разработчики, решают выбросить старые абстракции и запилить новые в том же количестве, только заменяя Java на Kotlin, например. А старых разработчиков с опытом, которые могут сказать "вот эту пачку интерфейсов мы ни разу не использовали" просто увольняют.


      1. vitecd
        06.01.2022 06:40
        +4

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


    1. tropico
      06.01.2022 02:01
      +4

      Ага, у нас так пишут - "простые функции" вместо классов/методов/декомпозиции, которые разростаются до 200 строк с 50-70 комментариями, потому что ноль абстракций, а рефакторить никто уже не будет. Потом две недели на обычную багу, где два дня только собираются духом в этот код лезть. :)

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


  1. SadOcean
    05.01.2022 16:47
    +4

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


  1. aaabramenko
    05.01.2022 16:51
    +28

    Щас насоветуете тут избавляться от абстракций - как же тогда собесы проходить?


    1. LynXzp
      06.01.2022 01:04
      +21

      Одно дело спортивное программирование собеседование, другое — реальный код

      спойлер
      На собеседовании:
      — Как поменять местами значения переменных не используя третью?
      — A=A^B; B=A^B; A=A^B;
      На работе:
      — Что это ты за код написал?
      — Меняю значения двух переменных не используя третью.
      — Ты уволен.


      1. Viceroyalty
        06.01.2022 02:53
        -2

        Кстати задание на собесе само по себе странное: присвоение в большинстве случаев - это создание указателей, а не копирующий конструктор.


  1. mOlind
    05.01.2022 17:50
    +8

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

    Истина где-то посередине. С одной стороны 5 слоев абстракций конечно во вред. И лишние интерфейсы не нужны. С другой стороны они позволяют избежать спагетти кода, и внятно и коротко описать и протестировать логику работы, прочитать и быстро понять код.

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


    1. kenoma
      05.01.2022 21:21
      +21

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


      1. mOlind
        06.01.2022 15:06
        +1

        Можно и на микросервисах кашу сделать. Не панацея. Это с опытом приходит как рефакторить и где бритвой Оккама проводить. Тупое следование правилам, конечно не поможет. "Всюду микросервисы!" - "Всюду фабрики" - "Склеить все воедино!". Крайности не помогут.


    1. tropico
      06.01.2022 02:10
      +2

      Потому сначала пишется решение которое работает, а потом причесывается и разбивается на блоки

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

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


      1. Viceroyalty
        06.01.2022 02:56

        А может это не так уж плохо для бизнеса, если бизнес не слишком зависит от эффективности кода? (Что не всегда так)


  1. taluyev
    05.01.2022 18:00
    +9

    1) Улучшенный дизайн выглядит подозрительно с точки зрения создания юнит-тестов. У вас есть юнит-тесты? Не интеграционные, а именно юнит-тесты. Написание тестируемого кода накладывает "ограничения" на дизайн. Отсутствие репозитория сразу бросается в глаза, как это тестировать.

    2) Декоратор, который приведен в статье мог использоваться для "быстрого фикса", с последующим рефакторингом, то есть возвращению обратно к простому дизайну.

    3) Стиль дизайна определяется плотностью запросов реализации новых фич (требоаний, реквариментов), выше плотность - больше паттернов/компонентов. Чем размельченнее код, тем время внедрения новых фич в код более линейно, или даже еще лучше. Характер заказчика играет в этом роль.

    4) Граница между разными сервисами проходит там, где можно выделить независимую и заменяемую часть логики.

    5) "потому что ты не знаешь непосредственно, какой конкретно код будет исполняться. Тебе сначала нужно проверить, какие реализации существуют в интерфейсе, а затем разобраться, какая конкретно применяется во время исполнения." - интерфейс вводится для того, чтобы "не знать", какая будет реализация - важен контракт использования.


    1. awfun
      05.01.2022 18:18
      +6

      1 - в статье не описано, какие задачи должна решать написанная система, но возможно что юнит-тесты им и не нужны. Для тестирования юзер флоу зачастую достаточно api тестов, а юнит-тесты используются для покрытия редких участков сложного кода, содержащего алгоритмические вычисления, а также в качестве документации для валидации входных данных.

      Удивительно, сразу после написания комментария пришло письмо от М.Видео - Выписка по бонусному счету.


      1. 0xd34df00d
        06.01.2022 06:50
        +4

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


        Писал недавно один маленький кусочек компилятора (парсер и тайпчекер, если конкретнее) — никаких юнит-тестов, и некуда их там приткнуть, только интеграционные.
        Писал чуть более давно чуть более полный компилятор, от парсера до оптимизатора и статического анализатора — аналогично, никаких юнит-тестов, только интеграционные.
        Писал ещё более давно систему для кластеризации новостей — ну, вы поняли, аналогично.
        Пишу сейчас ещё один тайпчекер — вообще никаких тестов, только доказательства.


        Короче, какие юниты вы там алгоритмически тестируете?


    1. dopusteam
      05.01.2022 18:31
      +6

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

      Судя по тому, что там далее, тут просто интерфейсы используются неправильно, просто чтоб было. Причём сигнатура конкретной реализации влияет на контракт (!). Цитата, собственно

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

      Налицо нарушение dependency inversion


      1. SadOcean
        06.01.2022 13:51
        +1

        Изменение фичи может потянуть за собой изменение и реализации и контракта, все верно.


        1. dopusteam
          06.01.2022 15:14

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

          Обычно изменения контракта приводят к изменению реализации, а не наоборот


          1. SadOcean
            07.01.2022 15:08

            Ну это терминологический вопрос.
            У пользователя был телефон, а теперь добавился емейл.
            Нужен он для одной фичи с отправкой обратной связи, но повлечет за собой и изменение контракта создания пользователя и возможно изменение контракта отправки обратной связи.
            Можно считать это изменением контракта (нужны новые данные), можно не считать (та же задача отправки обратной связи по запросу), но интерфейс изменится.


    1. sashagil
      06.01.2022 01:11

      Переводчик вам вряд ли ответит на вопрос к автору - они работают в разных фирмах и вряд ли общаются напрямую. В заметке есть ссылка на следующий пост автора, посвящённый именно тестированию:
      https://betterprogramming.pub/quit-unit-testing-classes-and-use-a-behavior-oriented-approach-306a667f9a31
      Кратко - на юнит тестирование забили большой болт, но, вроде как, выстроили супер-дупер систему интеграционного тестирования. По мне - есть риск, что когда аторы супер-дупер системы интеграционного тестирования переместятся на более зелёные пастбища, она заржевеет, и следующий набор сотрудников будет или её переписывать, или таки-восстанавливать возможность юнит-тестирования. Моё персональное мнение, ни в коей мере не претендующее на близость к реальности, так красиво (с картинками!) изображённой автором.


  1. ciuafm
    05.01.2022 18:21
    +2

    Очень хорошая статья. А есть какая-нибудь книга про шаблоны, паттерны и про то, когда их применять не надо?


    1. pfffffffffffff
      05.01.2022 22:35
      +4

      https://refactoring.guru/ru хороший ресурс


  1. wolfy_str
    05.01.2022 18:23
    -6

    Кто-то: Как мы избавились от 80% своего кода, повысив скорость разработки и уменьшив количество ошибок

    Я: Пытаюсь написать хотя бы пару строчек когда.


    1. tropico
      06.01.2022 02:12
      +1

      Вот когда, тогда.


  1. kai3341
    05.01.2022 18:34
    +1

    Как мы избавились от 80% своего кода, повысив скорость разработки и уменьшив количество ошибок

    Поздравляю, отличные результаты! Тоже радуюсь сокращению своей кодовой базы

    Недавно мы фронт-энд (react) рефакторили -- буду статью писать о наработках. Кратко: выкинули более 1000 строк кода, сократили пространство для появления ошибок


  1. pavelsc
    05.01.2022 19:20
    +1

    Ну слабое связывание просто надо уметь применять. Зачем кидать интерфейс в конструктор не абстрактного класса? Яркий пример php doctrine. В конструктор EntityRepository передается EntityManagerInterface, хотя соответственное приватное свойство класса и его геттер помечены только как EntityManager. Что они там собрались связывать в таком виде? Причем IDE нотации с типами EntityManager не видит и когда ты хочешь ткнуть в EntityManager::transactional из своего кода, то попадаешь просто в интерфейс вместо реализации.

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


    1. anydasa
      05.01.2022 23:02

      Т.е. контракт может использовать только абстрактный класс?

      Может EntityManagerInterface создан потому что EntityManager можно задекорировать?


  1. GaDzik
    05.01.2022 21:25
    +1

    80 это много. По картинке так вообще 90% сократили.


  1. Sipaha
    05.01.2022 21:25
    +2

    1. Слишком мелкая детализация ответственностей

    Ерунду написали об SRP. Принцип не о том, что нужно дробить весь код на атомы, а о том, что у одного модуля (функции, класса или файла с исходным кодом) не было разных интересантов для внесения изменений.


    1. Alew
      06.01.2022 14:29
      +1

      так это возможно только если код раздроблен до состояния атомов


      1. Sipaha
        07.01.2022 08:24

        Не совсем так. Функция из одной строчки может нарушать SRP, а класс из тысячи строк может не нарушать. Дробление на атомы это другой принцип и Мартин об этом говорит в книге "Чистая архитектура":

        Примером нарушения принципа может быть утилитная функция

        String toPrettyString(Person person)

        Которую используют для преобразования Person в строку (имя + фамилия например). Функцию используют при выводе в лог и при построении отчетов. В один прекрасный момент в логах захотели вывести еще и UUID пользователя и поменяли функцию, а следом поехали все отчеты, которые её использовали.


        1. Alew
          07.01.2022 16:54
          +1

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


    1. jvmdude
      06.01.2022 18:11
      +5

      >Принцип не о том, что нужно дробить весь код на атомы

      Есть такая странная книга "Мартин Р. - Чистый код. Cоздание, анализ и рефакторинг". Там английским по белому настоятельно рекомендуется: делать методы по 2-3 строчки(!), классы по полэкранчика, switch-и упаковывать абстрактные фабрики, всё максимально выносить в переменные классов, чтобы у методов стало по 0-1 аргументу (2++ нини) и тому подобное... В итоге "чистый код" в три раза длинее "нечистого" )

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

      Мне очень интересно, где-то есть конторы, где пишут код по Мартину... чтоб резюме туда случайно не отправить )


      1. tropico
        07.01.2022 13:09
        -1

        С таким кодом очень легко работать. Все абстракции и зависимости четко определены и ясны. Любые изменения легко даются или по принципу open/closed из SOLID или легко рефакторятся, чтобы не впиливать какие-то костыли.


        1. SadOcean
          07.01.2022 15:11
          +1

          Работать может и легко, а вот читать сложно.

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


  1. anydasa
    05.01.2022 22:39
    +3

    Это все верно только для микросервисов.

    Для средних сервисов это - копипаст и лапшекод. Да, плохая абстракция хуже копипаста, но если она грамотная это ок, даже если можно и без нее.

    Много классов, мало классов, это такое себе мерило. Я видел обратную сторону, написали в 10 строчек сервис, и потом заюзали его в 100 местах. А потом поняли, что получился бетон.

    Осознанный нейминг + четкие границы контекстов + нормальная модульность


  1. lea
    05.01.2022 22:42
    +3

    "Совершенство достигается не тогда, когда уже нечего прибавить, но когда уже ничего нельзя отнять." (Антуан де Сент-Экзюпери)


    1. KislyFan
      06.01.2022 18:12
      +1

      Удав сьел слишком много зверюшек. Обезьяна:
      - Давайте удаву отрубим голову ?
      - Да нет, это очень жестоко !
      - Давайте отрежем удаву хвост ?
      - Хорошо..
      - .. но по самые уши!

      Мне кажется, что в конторе персонажа Jonas Tulstrup удаву таки отрезали хвост по самые уши


  1. rtemchenko
    06.01.2022 00:23
    +1

    Выглядит неплохо, но чутка переборщили. Репозиторий лучше выделять в отдельный клас. Сегодня у вас одна БД, завтра другая. Очень часто встречается такая ситуация.


    1. SiSteamNiK
      06.01.2022 07:46
      +5

      можете пример привести? по-моему БД меняется очень редко


      1. KGeist
        06.01.2022 09:32
        +4

        В unit-тестах меняется на in memory представление :)


      1. rtemchenko
        06.01.2022 10:31
        +3

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

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

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


        1. KGeist
          06.01.2022 17:48
          +4

          Есть конкретный пример из недавнего - встроенный entity manager по скорости не вывозил - переписали на чистый SQL. При правильном разделении в таком случае затрагивается только инфраструктурный слой, слой приложения не меняется, т.к. интерфейсы остались те же. А, казалось бы, БД та же.


  1. distroid
    06.01.2022 01:53
    +7

    Сложно воспринимать описанное как сферического коня в вакууме (без контекста реальной системы и того, что она решает), но многое выглядит из серии плохих советов, особенно, что нарисованы абстрактные картинки, где нет понимания ни логики ни качества сделанных интерфейсов\абстракций.

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

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

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


    1. KGeist
      06.01.2022 09:36
      +2

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

      Такая практика (1 класс = 1 файл) была популяризована Джавой (где это ограничение рантайма, т.к. исходный файл компилировался в файл типа .class 1-к-1), и было скопировано в PHP, C# и т.п., но по сути ничего плохого в этом нет, если у классов сильная связность и они всё равно меняются вместе. Подход "несколько структур в одном файле" поощряется в Go.


      1. distroid
        06.01.2022 14:02

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

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


        1. KGeist
          06.01.2022 15:10

          >Когда смотришь код по файлам пооекта ожидаешь где и что лежит, а тут опана, в контроллере еще какие-то классы лежат.

          Есть и обратная ситуация - алгоритм оперирует вложенной иерархией объектов, где часть не имеет смысла без остального (какой-то агрегат, например, или алгоритмическая структура данных), а в итоге всё это размазано по 10 файлам, по которым приходится прыгать туда-сюда, чтобы понять, как это работает. Нужно смотреть по ситуации. Да, в контроллере сразу не придумать, зачем там могут быть лишние классы. Даже в той же Джаве были придуманы "вложенные классы", чтобы можно было несколько классов вложить в один файл, т.к. потребность есть. В Go я часто использую такой паттерн: например, есть сервис, он занимается чем-то своим, но где-то в одной из функций ему нужно делегировать работу в деталь реализации, которую я скрываю за интерфейсом, и если у этого интерфейса потребитель только один (этот единственный сервис, т.к. довольно специфичная одноразовая штука), то я кладу описание интерфейса в тот же файл, что и сервис. Не вижу, в чём тут дичь - сразу в одном месте видно что это такое, я не захламляю проект файлами для мелких интерфейсов, получается такая локализация в одном месте.


          1. distroid
            06.01.2022 15:54

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

            Вероятно там что-то вроде PHP\C#\Ruby и для этих языков закидывать классы в один файл, плохой подход. Иначе имеем индусский код, когда в одном файле у нас смешана логика из разных уровней приложения


            1. KGeist
              06.01.2022 17:11

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


      1. Findli
        06.01.2022 18:11

        вот мы и дошли до сути - эта статья просто и ясно говорит java это оверинжиниринг как правило. а вот go это nodejs express framework написанный для сишников и это типа тру.


        1. souls_arch
          07.01.2022 01:07

          Зато масштабируемый в процессе от нагрузок


    1. pokercase651
      06.01.2022 16:07

      Согласен. Много файлов ≠ сложный код. Разве будет хуже поместить файлы Request и Response в папку посвященную их фиче (вместе с файлом контроллера)?

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

      Неужели лучше скролить файл с несколькими классами?


      1. KGeist
        06.01.2022 17:29
        +1

        >Неужели лучше скролить файл с несколькими классами?

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


    1. bbc_69
      06.01.2022 16:46

      легкорасширяемую систему

      А оно надо? Для микросервисов-то? Я понимаю, для чего это делать в монолите, без этого ад и израль. Но микросервис. Написал и выкинул. Иначе получится не микросервис.

      юнит тесты, вызывало аналогичные эмоции

      Как говорится, юниттесты показывают как хорошо работают ваши моки. Интеграционные чаще полезнее в конечном итоге.

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

      Ну вот в Питоне так делают, в Го тоже. Как писали выше, особого смысле нет файлы плодить. Историческое наследие джавы.

      Поинт автора в том, что инструмены надо применять тогда, когда они нужны, а не заранее.

      Мой поинт в том, что концепции меняются, а методы работы - нет. И никто не задумывается, почему методы были придуманы такими.


      1. distroid
        06.01.2022 17:18

        А оно надо? Для микросервисов-то? Я понимаю, для чего это делать в
        монолите, без этого ад и израль. Но микросервис. Написал и выкинул.
        Иначе получится не микросервис.

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

        Как говорится, юниттесты показывают как хорошо работают ваши моки. Интеграционные чаще полезнее в конечном итоге.

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

        Ну вот в Питоне так делают, в Го тоже. Как писали выше, особого смысле нет файлы плодить. Историческое наследие джавы.

        Делать то это можно много где, не скажу на счет джавы, не работал с ней, но в PHP \ Ruby тоже встречаются, когда такое пытаются делать, но это из разряда плохих подходов. Не вижу ничего плохого в кол-ве файлов, если их слишком много - вероятно, что скорее с кодом, что-то не так. На мой взгляд в разы удобнее, когда каждый класс выделен в отдельный файл. Как и с тестами для них же - 1 файл на класс, 1 тест на этот класс.

        Мой поинт в том, что концепции меняются, а методы работы - нет. И никто не задумывается, почему методы были придуманы такими.

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


        1. noize
          07.01.2022 13:04

          Делать то это можно много где, не скажу на счет джавы, не работал с ней, но в PHP \ Ruby тоже встречаются, когда такое пытаются делать, но это из разряда плохих подходов. Не вижу ничего плохого в кол-ве файлов, если их слишком много - вероятно, что скорее с кодом, что-то не так. На мой взгляд в разы удобнее, когда каждый класс выделен в отдельный файл. Как и с тестами для них же - 1 файл на класс, 1 тест на этот класс.

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


      1. alemiks
        07.01.2022 15:03

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


    1. goldrobot
      07.01.2022 11:46
      +1

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

      Лучше прочитать один файлик на 300 строк, чем 10 файликов на 50 строк каждый.


      1. tropico
        07.01.2022 13:16

        В большинстве случаев сразу читать все не нужно, а только конкретную ветвь реализации. Вот тогда эти 300 строк начинают очень сильно отвлекать.


        1. goldrobot
          07.01.2022 13:35

          Вероятно, когда вы наизусь знаете все эти "ветви" (что за ветви?).

          А когда вы только ознакамливаетесь с кодом, или когда лезете что-то поменять, то это очень сильно мешает. Во всем должен быть баланс.


  1. AlexSpaizNet
    06.01.2022 02:51
    +6

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

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

    Выберите слоеную/хексагональную архитектуру, запилите генератор бойлерплейта, и будет вам щастье.

    Сразу в любом проекте будет понятно из чего он состоит и куда смотреть в случае чего.

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

    Хотя сложно сказать что за абстракции у них там были что убрав их они убрали 80% кода. Сколько там строчек кода занимает интерфейс репозитори/хэндлера/сервиса?


  1. alexg-nn
    06.01.2022 09:32
    +3

    Судя по картинкам, ребята отказались от грамотного DDD и CQRS. Выглядело громоздко, если только не требовалось по началу. Ради CRUD такого делать точно не стоило, но сложную бизнес логику в новой модели писать будет явно сложнее.


  1. MherArsh
    06.01.2022 13:57
    +2

    Переделали (микро)сервисную архитектуру в монолитную. На самом деле, на ограниченном домене, очень интересное решение, просто учитывая нынешние тренды на распределенность, морально сложно к этому придти)) но молодцы, круто)


  1. KislyFan
    06.01.2022 18:04
    +1

    мы перенесли запрос к базе данных в Repository непосредственно в Event Handler, которому он нужен

    Честно говоря стремное решение. Чему меня научили несколько лет разработки всякой шняги, так это то что абстракции нужны и придуманы не просто так. *пусть не в этом случае а в другом сферическом* вдруг у вас меняется база, меняется ее структура.. и тут вам надо вмешиваться в логику толстого event handler. Вот тут какбы разделению ответственности самое место, но его оттуда почему-то убрали. На мой взгляд отрефакторили до состояния на зло маме отморожу уши.


  1. souls_arch
    07.01.2022 01:05

    Первое, что приходит на ум из эффективнейших решений - перестали кодить, и вот, - результат! Шутка черный йумор, но жизненно.


  1. Battary
    07.01.2022 06:41

    И снова убеждаюсь в принципе: "Истина где-то посеридине".


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

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

    А вот тут меня аж покоробило...
    Вообще вся глава про интерфейсы очень странная, а большинство "проблем" решаются использованием средств IDE.


  1. panzerfaust
    07.01.2022 10:42
    +1

    Мне почему-то кажется, что вы уже говорите "гоп", но еще не перепрыгнули.

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

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

    И тогда начался второй этап рефакторинга. Когда на уже наработанной кодовой базе с хорошей структурой кода и сквозным неймнгом стало видно, где реально просятся обобщения и паттерны. В основном провели рефакторинг по SOLID, где-то внедрили CQRS и event-driven architecture. И вот тогда я поставил свой личный рекорд, когда написал новый "плагин" за 2 рабочих дня.

    Думаю, ваша история еще не окончена.


  1. Simonis
    07.01.2022 16:46
    +4

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

    Какое то очень своеобразное понимание и реализация принципа solid. Если вы столкнулись с подобной ситуацией, то вы явно что то сделали не так.

    Solid существует для того чтобы придерживаться реализации при которой код внутри класса имеет high cohesion и low coupling с внешним миром.

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

    Отсутствие причин для изменений - преимущество само по себе. Когда на ревью вы смотрите изменения в большом файле на несколько строк тут и там это становится головной болью и причиной по которой ревью проходит на авось.

    Больший контекст при реальной работе не добавляет абсолютно ни каких преимуществ. Откройте любой код на 4т строк и я уверен вы поймёте о чем я говорю. Человеческий мозг очень таки ограничен в своих возможностях.

    Понимание Изменения влияния на окружение будет происходить только при учёте анализа точек вызова и ни как иначе. Вне зависимости от размера файла.

    Тем не менее, почти все шаблоны имеют недостаток: они повышают структурную сложность и снижают согласованность кода.

    Программное обеспечение всегда сложно. Со временем его сложность только растет. Сложность нельзя просто взять и убрать. Ей можно только пытаться манипулировать.

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

    Множество сервисов реализованных в рамках одной архитектуры УВЕЛИЧИВАЮТ согласованность и консистентность кодовой базы. Упрощают понимание кода соседнего сервиса и таким образом значительно облегчают работу по их поддержке и модификации. Облегчают найм людей способных понимать стандартные решения. Я надеюсь что в индустрии таких людей большинство.

    Сюрприз но то что вы сделали со своей кодовой базой тоже будет шаблоном пусть и комплексным. Любой новый разработчик будет понимать что происходит на проекте изучив один ендпоинт при условии что все остальные выполнены в рамках той же архитектуры.

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

    Полностью поддерживаю.

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

    Казалось бы верное утверждение, но на самом деле фичи бывают разных 'объемов'. Можно легко написать фичу на ещё 4т строк. Слабое связывание помогает при тестировании фич в отсутствии изначальной системы. Позволяет разобрать фичу на части и тестировать их отдельно. Позволяет разрабатывать ее разными командами людей. Достаточно согласовать интерфейсы. Если вы будете часть фич делать 'побыстрому', а часть как полагается исходя из вашей стандартной архитектуры, то очень быстро придёте к ситуации полного бардака. Потому что, сюрприз, программисты решили что можно и так. А если спросят то всегда сослаться на сраки и дедлайны. И вообще они не виноваты. У вас везде так фичи написаны. Это ваша новая архитектура.

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

    Мне кажется что я не так понимаю вашу 'согласованность кода'.

    Если у интерфейса есть множество реализаций то как минимум нужно понимать для каких случаев они нужны. Опять же мы понятия не имеем что за интерфейсом скрывается. Да и вся суть собственно в том чтобы и не задумываться об этом. Чтобы инженер фокусировался на важной для него задаче, а не копался в потрохах какой то конкретной реализации какого то конкретного интерфейса. Зачем? Альтернатива этому (шаблон стратегия) будет полная реализация всех альтернатив на месте ( внутри текущего файла) и свитч для выбора нужной на месте вызова. По вашему это действительно упростит чтение и понимание?

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

    Мне нужно подготовить по заглушке на каждый вызов метода имеющего внутренний вызов библиотеки? А если я хочу проверить метод с разными данными, значительно влияющими на поток выполнения?

    Method(input)

    a = lib.method_1(input);

    if a

    then b = lib.method_2(input);

    else b = lib.method_3(input);

    Сколько заглушке для библиотеки мне нужно подготовить чтобы протестировать все пути исполнения? Это действительно экономия? Тащить в проект фейковые либы в проект. Между прочим их тоже кому то придется поддерживать. И как раз в этом случае сделать это будет сильно сложнее. Потому что их будет много и для каждой конкретной нужно чётко понимать где и как она используется. И это без учёта возможных кодов ошибок или же исключительных ситуаций которые тоже нужно тестировать.

    Вы действительно думаете что не стоит закрывать это интерфейсами?

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

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

    Ранее вы описывали особенность solid - закрытость к изменениям и открытость к расширению. Если бы вы действительно этого придерживались то подобные проблемы с тестами происходили бы у вас крайне редко.


    1. michael_v89
      07.01.2022 19:09

      — серьёзно рассмотрите возможность перехода на библиотеку-заглушку, позволяющую имитировать конкретные классы
      — Мне нужно подготовить по заглушке на каждый вызов метода имеющего внутренний вызов библиотеки? Тащить в проект фейковые либы в проект.

      Подразумевается не конкретная библиотека-заглушка, а библиотека, позволяющая делать заглушки. В оригинале: "seriously consider switching to a mocking library that allows mocking concrete classes".


      Сколько заглушек для библиотеки мне нужно подготовить чтобы протестировать все пути исполнения?

      Поэтому одну, так же как и с интерфейсом.


      1. Simonis
        07.01.2022 20:04

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

        По всей видимости вы правы.

        Только вот совет все равно выглядит вредным. Стабать конкретный клас осмысленно только в случае когда вы не имеете доступа к самому классу (не являетесь владельцем кода) и он действительно не имеет интерфейса. Кроме того этот способ имеет серьезные ограничения.

        Warning: Substituting for classes can have some nasty side-effects!

        For starters, NSubstitute can only work with virtual members of the class that are overridable in the test assembly, so any non-virtual code in the class will actually execute! If you try to substitute for a class that formats your hard drive in the constructor or in a non-virtual property setter then you’re asking for trouble. (By overridable we mean public virtualprotected virtualprotected internal virtual, or internal virtual with InternalsVisibleTo attribute applied. See How NSubstitute works for more information.)

        Зачем снимать сиденье с велосипеда мне не понятно. Вы на нем может быть и не сидите, но люди привыкли к другу.


        1. michael_v89
          07.01.2022 20:39

          В C# может и так, а например в PHP можно замокать класс целиком. Делать так имеет смысл всегда, если интерфейс в остальном коде нигде не будет использоваться.


          1. Simonis
            07.01.2022 20:57

            Делать так имеет смысл всегда, если интерфейс в остальном коде нигде не будет использоваться.

            То есть все таки далеко не всегда. И как я показал своим примером далеко не везде. Это и есть моя основная идея.

            Даже если разброс 50/50 использование небезопасного подхода приведет к кому что 100% нужно задумываться лишний раз о том а как лучше реализовать в этом конкретном месте вместо того чтобы придерживаться безопасной реализации всегда.

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


            1. michael_v89
              07.01.2022 23:10
              +1

              Даже если разброс 50/50 использование небезопасного подхода

              Каким образом моки классов более небезопасны, чем интерфейсы?


              Потратить 5 минут на создание интерфейса в процессе работы не вносит значительного импакта на время разработки.

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


              Убережёт вас от ненужной контекстной информации.

              Я вот вообще не вижу никакой разницы в контекстной информации между вызовами someVar.method() и someVar.method(). То, что там где-то выше по коду в одном случае тип someVar объявлен без префикса I, не играет никакой роли при анализе кода.


              1. alexg-nn
                08.01.2022 10:45

                Я вот вообще не вижу никакой разницы в контекстной информации между вызовами someVar.method() и someVar.method()

                Наверное имелся в виду code completion. У класса вывалится чёрт знает сколько методов и полей, а у интерфейса - обычно гораздо меньше.


              1. Simonis
                08.01.2022 11:53

                Каким образом моки классов более небезопасны, чем интерфейсы?

                Кажется вы плохо читаете мои комментарии.

                Дублирую

                Warning: Substituting for classes can have some nasty side-effects!

                For starters, NSubstitute can only work with virtual members of the class that are overridable in the test assembly, so any non-virtual code in the class will actually execute! If you try to substitute for a class that formats your hard drive in the constructor or in a non-virtual property setter then you’re asking for trouble. (By overridable we mean public virtual, protected virtual, protected internal virtual, or internal virtual with InternalsVisibleTo attribute applied. See How NSubstitute works for more information.)

                Понадобилось добавить method_4 в реализацию, которая единственная, и надо лезть еще и в интерфейс.

                Для начала стоит хорошенечко подумать: а с какой такой стати вы меняете интерфейс взаимодействия с подсистемой (интерфейс может скрывать не просто класс из вашего проекта но и вызов к внешней системе)? Вероятно вы выбрали неверную абстракцию изначально. Во вторых это вновь нарушение принципа закрытости и открытости. Если этот метод плотно связан с предыдущей функциональностью то его вызов должен быть скрыт за простым, публичным интерфейсом. Если это независимый от предыдущейфункциональности метод то он должен быть выделен в отдельный интерфейс (interface segregation principle).

                Теперь задайте себе вопрос : как часто вы будете добавлять по новому методу который нужно будет обязательно дополнительно вызывать в части или во всех точках вызова? Мое личное имхо на основе моего личного опыта - умозрительно редко. Чаще будет происходить сокрытое добавление вызова внутри публичных мелодов (валидации, блокировки).

                Понадобилось поменять тип аргумента в method_3, и надо снова лезть в интерфейс.

                За вас это прекрасно сделает ide. Только не говорите что вы пишете код в блокноте.

                Я вот вообще не вижу никакой разницы в контекстной информации между вызовами

                Ниже вам уже ответили о том что внутри класса может быть сколь угодно много других методов и полей не относящихся к текущему контексту. Более того интерфейсом вы подчеркивает какая именно функциональность востребована конкретно тут в отрыве от реализации (которая может быть любой). Это даёт нам право игнорировать эту самую реализацию и фокусировать внимание на текущем уровне абстракции вместо того чтобы исследовать конкретную реализацию. Тк если она фиксирована то она имеет значение, может повлиять на поток выполнения или каким то образом явно мутировать данные (этим в целях экономии памяти могут заниматься приватные методы класса). Конечно, в общем случае, достигнуть подобного возможно и при работе с интерфейсом, но чуть большими усилиями в сравнении с тем чтобы создать конкретную реализацию класса на месте и передать в него по ссылке данные используемые в последствии. Интерфейс не гарантирует безопасности вызова сам по себе, но подталкивает по другому относиться к проектированию компонент. Ещё раз invention of control.


                1. michael_v89
                  08.01.2022 13:09

                  Дублирую

                  Так вы там говорите про конкретный язык в конкретной версии, а не про "подход". Если моки классов небезопасны в вашем языке, это не означает что небезопасен сам подход в целом. В PHP с этим проблем нет.


                  Для начала стоит хорошенечко подумать: а с какой такой стати вы меняете интерфейс взаимодействия с подсистемой (интерфейс может скрывать не просто класс из вашего проекта но и вызов к внешней системе)? Вероятно вы выбрали неверную абстракцию изначально.

                  Нет. Потому что требования поменялись. Потому что в новой версии lib (или тем более внешней системы) появились новые возможности, или поменялись существующие (например исправили баг, что повлекло добавление нового параметра в метод). Потому что нельзя на старте идеально продумать протокол взаимодействия компонентов и никогда за 10 лет развития приложения его не менять.


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

                  Мы говорим про добавление нового метода в этот простой публичный интерфейс. Например, раньше подключение к какому-то ресурсу закрывалось автоматически, а потом решили, что надо добавить метод Close().


                  Если это независимый от предыдущей функциональности метод то он должен быть выделен в отдельный интерфейс

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


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

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


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

                  Лично я никогда не рассматривал это как проблему, но создавать дополнительные сложности в разработке ради того, чтобы подсказки в IDE были другие, мне кажется неправильным подходом. Более того, если они являются публичным контрактом реализации, значит они зачем-то нужны, и есть вероятность, что могут понадобиться и при изменении вашего кода. А с интерфейсом в этом случае появляется то, о чем я говорю — надо лезть в интерфейс и пробрасывать.


                  Более того интерфейсом вы подчеркивает какая именно функциональность востребована конкретно тут в отрыве от реализации (которая может быть любой)

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


                  Это даёт нам право игнорировать эту самую реализацию и фокусировать внимание на текущем уровне абстракции вместо того чтобы исследовать конкретную реализацию.

                  Зачем нам исследовать конкретную реализацию, если мы смотрим вызывающий код? Есть конкретный код someVar.method(); someVar.method2();, ничего не мешает фокусировать внимание на этом уровне абстракции, независимо от того, какой там тип у someVar.


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

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