Если мне не изменяет память, проблема схожести имен также рассматривается как у Мартина, так и у Макконнела. Лекарства от схожести: применять осмысленные формулировки и отказаться от аббревиатур: checkPontentialForLegalEntityRequest(), checkPotentialForIndividualRequest().
Возможно, я неправильно понимаю вашу претензию к предложенным @Rsa97 именам, но я вообще не вижу, как они могут способствовать превращению кода в дремучее легаси. По-моему, с этими именами всё в порядке, в особенности в сравнении с A, B, C, D.
Поскольку я в данный момент работаю как раз на таком долгоживущем проекте, могу сказать с уверенностью: неактуальные комментарии составляют значительный процент неудобств, возникающих при работе над легаси. Если не больше, чем другие причины, то явно они входят в тройку «лидеров».
Коллега тут ссылался на способ, предложенный С. Макконнелом, когда ты вначале описываешь метод псевдокодом (в комментариях), а затем по намеченному плану пишешь фактическую реализацию.
Но псевдокод не обязательно писать в виде комментариев. Можно описать верхний уровень абстракции в виде вызовов несуществующих методов, а затем эти методы объявить и реализовать. Такой подход весьма способствует тому, что ваш код будет правильно разбит на уровни и будет выглядеть осмысленно.
Про ревью действительно хорошая мысль. Вообще, если тебе на ревью задают вопрос, это четкий индикатор, что не всем понятно, что ты тут понаписал. И тогда однозначно необходимо одно из двух: либо рефакторить код, чтобы он стал самоочевидным, либо добавлять комментарий. (Ну, либо и то и то вместе.)
И если ревьюер сказал, что не понятно, не надо пытаться доказать, что всё в твоем коде хорошо — это не так. Надо просто взять и попытаться сделать понятнее.
Речь не идет о том, чтобы просто понавыносить как можно больше методов. Это вопрос грамотного проектирования.
Извлечение методов должно способствовать основной задаче — разделению кода на уровни абстракции.
Если на верхнем уровне присутствуют только вызовы методов, отражающие общий смысл алгоритма, а в подметодах идет детализация, то это то, к чему нужно стремиться. Если же методы вытащены из основного алгоритма случайным образом, то, конечно, код не станет от этого понятнее.
Все верно. Можно указать на необходимость соблюдать порядок вызовов в комментарии. Я лишь пытаюсь донести мысль, что комментарии и в этом случае — не единственный выход.
Например, можно сделать, чтобы вызов одного метода зависел от результатов работы другого:
Так становится очевидна не только сама последовательность вызовов, но и причина, по которой она необходима.
Можно организовать флюентный интерфейс, в котором метод, который должен вызываться первым, возвращает интерфейс, в котором есть вызов второго метода, который зависит от результатов работы первого:
Именно. У Макконнела в "Совершенном коде" эта тема раскрыта на 42 страницах (считал по оригиналу), у Мартина в "Чистом коде", как я уже упоминал, ей тоже уделено внимание.
Если кто-то потратил 3 минуты на прочтение моей статьи и заинтересовался этой темой, вы знаете, что делать дальше.
Понятно, что логика в вашем реальном приложении сильно сложнее моих упрощенных примеров. На то они и упрощенные. При всем при этом я не вижу проблемы с тем, чтобы уложить это все в рамки того же паттерна строителя.
можно чуть-чуть упростить логику - сделать эту выборку первой, когда список пустой (тогда не придется проверять заполнен у него ДУЛ или нет - просто если нет - добавляем нового клиента с ДУЛ, если есть - новую запись с другим ДУЛ для существующего клиента). Но тогда придется опять писать комментарий почему именно она должна идти первой, а все остальное потом.
Кстати, одна из прелестей этого паттерна как раз заключается в том, что он позволяет отвязаться от порядка выполнения методов-конфигураторов. Накидывать конфигурацию мы можем в любом порядке, а алгоритм сборки будет выполнять действия в том порядке, в котором нужно.
Да, без номеров, но с максимально близко к тексту ТЗ. Чтобы было можно связать кодл с ТЗ.
Под этим утверждением подпишусь. Если уж нужно ссылаться на ТЗ, то не по номерам, а по формулировкам или заголовкам.
В данном случае нет. Потому что вся выборка делается одним SQL запросом
Плюс две дополнительных проверки (включение их в запрос сильно его замедляет, быстрее проверить отдельно) - неподтвержденный ИНН (там по двум таблицам проверка) и тип идентификации клиента (одна таблица) - она вообще отдельный пунктом в ТЗ прописана для всех.
Так предложенный мной подход и не протвиоречит этим требованиям. Применение любого из условий, заданных строителем, может выполняться как на уровне SQL-запроса, так и на уровне фильтра постобработки — это уже исключительно вопрос реализации. Дополнительные данные для проверок (например, данные из других таблиц), если они нужны, можно загрузить заблаговременно и передать их в строитель тем или иным способом.
Интересный подход, хотя это, пожалуй, получается не паттерн спецификации, поскольку объект, передаваемый вами в метод репозитория, содержит не логику, а дополнительные критерии.
Задачу, похожую на вашу, я на своем проекте решал несколько иным способом:
interface QueryFilter
{
/**
* Применяет условия фильтрации к переданному объекту запроса.
*/
public function applyTo(Query &$query);
}
class DoctrineLoyaltyCardRepository extends DoctrineRepository implements LoyaltyCardRepositoryInterface
{
// ...
private function findAllByFilter(QueryFilter $filter): LoyaltyCardCollection
{
// Применяем базовые параметры фильтрации
$query = $this->cardRepository->createQueryBuilder('c')
->andWhere('c.programId = :programId')
->andWhere('c.levelId = :levelId')
->andWhere('c.profileFilled = :profileFilled')
->getQuery();
// Применяем фильтр с дополнительными параметрами
$filter->applyTo($query);
$cards = $query->getResult();
return new LoyaltyCardCollection(...$cards);
}
}
Пример несложный, это верно. Я его привел, чтобы проиллюстрировать, что "говорящий" код вполне может передавать мысль о соответствии реализации формулировкам в ТЗ.
Как вы верно отметили, в ТЗ всех тонкостей не передашь, поэтому, как я писал выше, реализация редко повторяет ТЗ один в один. Но мы можем разбить реализацию на уровни абстракции. И было бы неплохо (и это вполне реально реализовать), чтобы на верхнем уровне она сохраняла соответствие с оглавлением ТЗ.
Ваш пример постановки задачи вполне хорош, но я не могу сказать, чтобы он был ощутимо сложнее примера, который я привел выше. Задача решается, например, применением строителя условий:
Можно, конечно, сослаться в комментариях на пунктты ТЗ, но тут кроется мина: чтобы это было надежно, нужно, чтобы либо ТЗ было окончательным (что бывает редко), либо ссылаться нужно на конкретную версию ТЗ (чего я тоже не встречал), иначе любое изменение в ТЗ, влияющее на нумерацию, прведет к тому, что комментарии начнут вводить в заблуждение. Так что лучше, чтобы можно было сориентироваться без ссылок на номера пунктов.
В любом случае, лучше, чтобы комментарии дополняли самодокументируемый код (когда это действительно необходимо), а не заменяли его.
плохо, когда устаревает коментарий, но когда устаревает имя метода - еще хуже
Согласен полностью. Поэтому самодокументируемый код — это лишь часть общей культуры программирования, которой неплохо было бы придерживаться.
Сюда можно включить, например, принципы SOLID. Буква O из этой аббревиатуры, напомню, — Open-Closed Principle. В контексте поднятой вами проблемы его можно понимать так, что не следует менять имеющееся поведение системы. Если кратко и грубо — не меняй старый метод — пиши новый. То есть, поведение метода не должно меняться никогда. Поэтому и его имя не устареет.
Вы предлагаете в код в виде комментариев вносить требования (ТЗ)? Ну это такое.
Про ТЗ соглашусь. Попытка вывести прямое, однозначное соответствие между ТЗ и кодом может привести к сомнительным решениям. ТЗ лучше проецируется на приемочные тесты, чем на код.
Спасибо за аргументированный комментарий. Со многими мыслями в нём я согласен.
Про напоминания — да, полностью поддерживаю. Это действительно хорошая и уместная роль комментариев: сохранить знание, которое сейчас вроде бы ни на что не влияет, но может оказаться критичным при изменениях. Такие вещи в код "не впишешь", и комментарий тут вполне уместен.
Что касается встроенной документации — я предпочитаю отделять её от обычных комментариев в коде. То, что оформляется через Javadoc, PHPDoc, Doxygen и т. п., — это всё-таки документация к интерфейсу, и она действительно должна описывать семантику, ограничения, контракты, примеры использования и прочее. У себя на проекте я как раз продвигаю такую практику, и она обязательна для публичных методов.
Про комментарии "почему" — тоже согласен, что полностью без них обойтись не получится. Но я бы сузил область их применения: речь именно о ситуациях, где нужно зафиксировать обоснование выбора между несколькими вариантами, особенно если выбранный вариант не самый очевидный. Такие случаи, на мой взгляд, не то чтобы очень частые, и потому не требуют, чтобы комментарии были "везде на всякий случай". Но когда они действительно нужны — лучше написать.
Если же в функции большое количество совместно используемых переменных, это может оказаться индикатором, что функция перегружена смыслами (нарушает принцип единой ответственности, если вам угодно). Тогда она как раз таки нуждается в разбивке на подфункции — это упростит код и будет способствовать пониманию.
Спасибо, отличный комментарий — с многим можно согласиться. Вместе с тем, к нему есть что добавить.
Многое, с чем я могу согласиться, например, есть в этом комментарии: https://habr.com/ru/articles/929600/#comment_28604372 — очень точно сказано и про приоритет читаемости, и про то, что в большинстве проектов производительность — не первое, о чём стоит беспокоиться.
От себя добавлю, что самодокументируемый код и комментарии, по-хорошему, должны работать вместе. Когда код можно выразить понятными именами — это стоит делать. А если нужны пояснения к логике, особенно завязанной на предметную область или поведение вне кода — тут комментарий вполне уместен.
Про производительность: в подавляющем большинстве случаев на неё гораздо больше влияют не структура кода или количество методов, а операции ввода-вывода — файловые операции, работа с базой, сетевые задержки и т. п. Разбиение метода на 2–3 вспомогательных почти никогда не станет узким местом.
Что касается приведенного вами примера кода — это не моя предметная область и не тот язык, с которым я работаю, поэтому детали комментировать не возьмусь. Но как пример из жизни — он отлично показывает, в каких ситуациях комментарий помогает понять, что происходит, без необходимости вникать в физическую структуру данных. Опять же, не знаю особенностей этого языка, но рискну предположить, что и в нем найдутся выразительные средства, позволяющие сделать код более читаемым и сократить необходимость в комментариях или дополннить их.
Да, оба варианта возможны, тут нужно смотреть по контексту.
От себя дополню, что предложенная вами формулировка NotPercent может оказаться не вполне однозначной. Я бы предлжил fractionalTaxRate.
Плюс самодокументируемого кода в вашем кейсе становится понятен, если тело метода с расчетом оказывается относительно большим. Тогда расстояние от места объявления аргумента или локальной переменной (там, где она описана комментарием) до места его использования может оказаться достаточно большим, чтобы вызвать непонимание у человека, читающего код. В случае самодокументируемого кода имя переменной говорит само за себя, в каком бы месте оно вам ни встретилось.
Всё это так. Смысл в том, что во всех этих случаях комментариям есть достойные альтернативы. По опыту, почти в 100 % случаев, когда есть желание написать комментарий, можно извлечь соотвествующий фрагмент кода в метод, и имя этого метода будет заменой комментарию.
И как правило, это будет лучшей альтернативой, потому что имя метода (более-менее) неразрывно связано с его назначением с одной стороны и с реализацией — с другой. О комментарии такого не скажешь.
Да.
Да.
Да.
Да (видите опечатки в предложенных вами примерах?).
Обратите внимание, предложенные мной варианты как раз исправляют все указанные вами проблемы, не сильно добавляя к длине имен.
Длина имен
checkPotentialForRequestClerntsFLи
checkPotentialForIndividualRequest,например, отличается всего на один символ.
Если мне не изменяет память, проблема схожести имен также рассматривается как у Мартина, так и у Макконнела. Лекарства от схожести: применять осмысленные формулировки и отказаться от аббревиатур:
checkPontentialForLegalEntityRequest(),checkPotentialForIndividualRequest().Возможно, я неправильно понимаю вашу претензию к предложенным @Rsa97 именам, но я вообще не вижу, как они могут способствовать превращению кода в дремучее легаси. По-моему, с этими именами всё в порядке, в особенности в сравнении с A, B, C, D.
Поскольку я в данный момент работаю как раз на таком долгоживущем проекте, могу сказать с уверенностью: неактуальные комментарии составляют значительный процент неудобств, возникающих при работе над легаси. Если не больше, чем другие причины, то явно они входят в тройку «лидеров».
Коллега тут ссылался на способ, предложенный С. Макконнелом, когда ты вначале описываешь метод псевдокодом (в комментариях), а затем по намеченному плану пишешь фактическую реализацию.
Но псевдокод не обязательно писать в виде комментариев. Можно описать верхний уровень абстракции в виде вызовов несуществующих методов, а затем эти методы объявить и реализовать. Такой подход весьма способствует тому, что ваш код будет правильно разбит на уровни и будет выглядеть осмысленно.
Про ревью действительно хорошая мысль. Вообще, если тебе на ревью задают вопрос, это четкий индикатор, что не всем понятно, что ты тут понаписал. И тогда однозначно необходимо одно из двух: либо рефакторить код, чтобы он стал самоочевидным, либо добавлять комментарий. (Ну, либо и то и то вместе.)
И если ревьюер сказал, что не понятно, не надо пытаться доказать, что всё в твоем коде хорошо — это не так. Надо просто взять и попытаться сделать понятнее.
Речь не идет о том, чтобы просто понавыносить как можно больше методов. Это вопрос грамотного проектирования.
Извлечение методов должно способствовать основной задаче — разделению кода на уровни абстракции.
Если на верхнем уровне присутствуют только вызовы методов, отражающие общий смысл алгоритма, а в подметодах идет детализация, то это то, к чему нужно стремиться. Если же методы вытащены из основного алгоритма случайным образом, то, конечно, код не станет от этого понятнее.
Все верно. Можно указать на необходимость соблюдать порядок вызовов в комментарии. Я лишь пытаюсь донести мысль, что комментарии и в этом случае — не единственный выход.
Например, можно сделать, чтобы вызов одного метода зависел от результатов работы другого:
Так становится очевидна не только сама последовательность вызовов, но и причина, по которой она необходима.
Можно организовать флюентный интерфейс, в котором метод, который должен вызываться первым, возвращает интерфейс, в котором есть вызов второго метода, который зависит от результатов работы первого:
Так вы физически не сможете вызвать
getPermissions(), не загрузив предварительно список пользователей.Со сложными алгоритмами: их (почти?) всегда можно разложить на несколько уровней простых.
Да, это так. Роберт Мартин довольно эпатажный автор. Но смысл его высказывания, тем не менее, понятен.
Именно. У Макконнела в "Совершенном коде" эта тема раскрыта на 42 страницах (считал по оригиналу), у Мартина в "Чистом коде", как я уже упоминал, ей тоже уделено внимание.
Если кто-то потратил 3 минуты на прочтение моей статьи и заинтересовался этой темой, вы знаете, что делать дальше.
Понятно, что логика в вашем реальном приложении сильно сложнее моих упрощенных примеров. На то они и упрощенные. При всем при этом я не вижу проблемы с тем, чтобы уложить это все в рамки того же паттерна строителя.
Кстати, одна из прелестей этого паттерна как раз заключается в том, что он позволяет отвязаться от порядка выполнения методов-конфигураторов. Накидывать конфигурацию мы можем в любом порядке, а алгоритм сборки будет выполнять действия в том порядке, в котором нужно.
Под этим утверждением подпишусь. Если уж нужно ссылаться на ТЗ, то не по номерам, а по формулировкам или заголовкам.
Так предложенный мной подход и не протвиоречит этим требованиям. Применение любого из условий, заданных строителем, может выполняться как на уровне SQL-запроса, так и на уровне фильтра постобработки — это уже исключительно вопрос реализации. Дополнительные данные для проверок (например, данные из других таблиц), если они нужны, можно загрузить заблаговременно и передать их в строитель тем или иным способом.
Интересный подход, хотя это, пожалуй, получается не паттерн спецификации, поскольку объект, передаваемый вами в метод репозитория, содержит не логику, а дополнительные критерии.
Задачу, похожую на вашу, я на своем проекте решал несколько иным способом:
Пример несложный, это верно. Я его привел, чтобы проиллюстрировать, что "говорящий" код вполне может передавать мысль о соответствии реализации формулировкам в ТЗ.
Как вы верно отметили, в ТЗ всех тонкостей не передашь, поэтому, как я писал выше, реализация редко повторяет ТЗ один в один. Но мы можем разбить реализацию на уровни абстракции. И было бы неплохо (и это вполне реально реализовать), чтобы на верхнем уровне она сохраняла соответствие с оглавлением ТЗ.
Ваш пример постановки задачи вполне хорош, но я не могу сказать, чтобы он был ощутимо сложнее примера, который я привел выше. Задача решается, например, применением строителя условий:
Можно, конечно, сослаться в комментариях на пунктты ТЗ, но тут кроется мина: чтобы это было надежно, нужно, чтобы либо ТЗ было окончательным (что бывает редко), либо ссылаться нужно на конкретную версию ТЗ (чего я тоже не встречал), иначе любое изменение в ТЗ, влияющее на нумерацию, прведет к тому, что комментарии начнут вводить в заблуждение. Так что лучше, чтобы можно было сориентироваться без ссылок на номера пунктов.
В любом случае, лучше, чтобы комментарии дополняли самодокументируемый код (когда это действительно необходимо), а не заменяли его.
Согласен полностью. Поэтому самодокументируемый код — это лишь часть общей культуры программирования, которой неплохо было бы придерживаться.
Сюда можно включить, например, принципы SOLID. Буква O из этой аббревиатуры, напомню, — Open-Closed Principle. В контексте поднятой вами проблемы его можно понимать так, что не следует менять имеющееся поведение системы. Если кратко и грубо — не меняй старый метод — пиши новый. То есть, поведение метода не должно меняться никогда. Поэтому и его имя не устареет.
Пример по поводу соответствия реализации требованиям ТЗ (псевдокод).
=== ТЗ ===
Функция
calculateFinalAmount(order)должна реализовывать следующий алгоритм:Подсчитать сумму всех товаров в заказе, исключая товары с пометкой "подарок".
Применить персональную скидку клиента, если она указана.
Применить промокод, если он присутствует и действителен.
Если сумма после скидок превышает 10 000, применить дополнительную скидку 5%.
Добавить налог на добавленную стоимость (НДС) в размере 20% от суммы после всех скидок.
Если заказ содержит хотя бы один товар с категорией "алкоголь", применить акциз 10% к их стоимости до скидок.
Вернуть итоговую сумму и расшифровку примененных скидок и налогов.
=== Реализация ===
Про ТЗ соглашусь. Попытка вывести прямое, однозначное соответствие между ТЗ и кодом может привести к сомнительным решениям. ТЗ лучше проецируется на приемочные тесты, чем на код.
Спасибо за аргументированный комментарий. Со многими мыслями в нём я согласен.
Про напоминания — да, полностью поддерживаю. Это действительно хорошая и уместная роль комментариев: сохранить знание, которое сейчас вроде бы ни на что не влияет, но может оказаться критичным при изменениях. Такие вещи в код "не впишешь", и комментарий тут вполне уместен.
Что касается встроенной документации — я предпочитаю отделять её от обычных комментариев в коде. То, что оформляется через Javadoc, PHPDoc, Doxygen и т. п., — это всё-таки документация к интерфейсу, и она действительно должна описывать семантику, ограничения, контракты, примеры использования и прочее. У себя на проекте я как раз продвигаю такую практику, и она обязательна для публичных методов.
Про комментарии "почему" — тоже согласен, что полностью без них обойтись не получится. Но я бы сузил область их применения: речь именно о ситуациях, где нужно зафиксировать обоснование выбора между несколькими вариантами, особенно если выбранный вариант не самый очевидный. Такие случаи, на мой взгляд, не то чтобы очень частые, и потому не требуют, чтобы комментарии были "везде на всякий случай". Но когда они действительно нужны — лучше написать.
Если же в функции большое количество совместно используемых переменных, это может оказаться индикатором, что функция перегружена смыслами (нарушает принцип единой ответственности, если вам угодно). Тогда она как раз таки нуждается в разбивке на подфункции — это упростит код и будет способствовать пониманию.
Спасибо, отличный комментарий — с многим можно согласиться. Вместе с тем, к нему есть что добавить.
Многое, с чем я могу согласиться, например, есть в этом комментарии: https://habr.com/ru/articles/929600/#comment_28604372 — очень точно сказано и про приоритет читаемости, и про то, что в большинстве проектов производительность — не первое, о чём стоит беспокоиться.
От себя добавлю, что самодокументируемый код и комментарии, по-хорошему, должны работать вместе. Когда код можно выразить понятными именами — это стоит делать. А если нужны пояснения к логике, особенно завязанной на предметную область или поведение вне кода — тут комментарий вполне уместен.
Про производительность: в подавляющем большинстве случаев на неё гораздо больше влияют не структура кода или количество методов, а операции ввода-вывода — файловые операции, работа с базой, сетевые задержки и т. п. Разбиение метода на 2–3 вспомогательных почти никогда не станет узким местом.
Что касается приведенного вами примера кода — это не моя предметная область и не тот язык, с которым я работаю, поэтому детали комментировать не возьмусь. Но как пример из жизни — он отлично показывает, в каких ситуациях комментарий помогает понять, что происходит, без необходимости вникать в физическую структуру данных. Опять же, не знаю особенностей этого языка, но рискну предположить, что и в нем найдутся выразительные средства, позволяющие сделать код более читаемым и сократить необходимость в комментариях или дополннить их.
Да, оба варианта возможны, тут нужно смотреть по контексту.
От себя дополню, что предложенная вами формулировка NotPercent может оказаться не вполне однозначной. Я бы предлжил fractionalTaxRate.
Плюс самодокументируемого кода в вашем кейсе становится понятен, если тело метода с расчетом оказывается относительно большим. Тогда расстояние от места объявления аргумента или локальной переменной (там, где она описана комментарием) до места его использования может оказаться достаточно большим, чтобы вызвать непонимание у человека, читающего код. В случае самодокументируемого кода имя переменной говорит само за себя, в каком бы месте оно вам ни встретилось.
Всё это так. Смысл в том, что во всех этих случаях комментариям есть достойные альтернативы. По опыту, почти в 100 % случаев, когда есть желание написать комментарий, можно извлечь соотвествующий фрагмент кода в метод, и имя этого метода будет заменой комментарию.
И как правило, это будет лучшей альтернативой, потому что имя метода (более-менее) неразрывно связано с его назначением с одной стороны и с реализацией — с другой. О комментарии такого не скажешь.