Комментарии 68
Если вы имеете ввиду, что «Пользователь отдающий Комментарии из своего последнего Поста» нарушает правило «единой ответственности», то это не так, если вы соблюдаете дерево / иерархия зависимостей сущностей: если Пользователь будет родителем Комментария и Поста в вашей приложении, то никакого нарушения не произойдет.
НО если Пользователь правда не ответственен за Пост или Комментарии, а просто с ними связан, то да, будет нарушение (что часто можно увидеть в коде стандартных MVC проектов).
DRY
Наоборот, LoD как раз-таки полностью поддерживает DRY, поскольку снижает кол-во переиспользуемого кода (вам не придется постоянно писать цепочки запросов, достаточно будет использовать 1 метод)
KISS
Не вижу ничего сложного)
+ Я не предлагаю использовать «везде и всегда», нет, важно знать когда это имеет смысл и для этого есть глава «Когда использовать»
+ Вы сейчас предъвляете принципу, который не я придумал, так что можете сходить поискать основоположника и высказать ему)
Соответственно, если эта иерархия меняется / расширяется, везде, где был вызвана вот эта цепочка, придется вносить изменения (рефакторить код + тесты).
Для того, чтобы этого не происходило, достаточно выносить в отдельные методы повторяющийся код (в соответствии с DRY).
Я думаю описанный метод имеет право на жизнь в определенных условиях. А именно когда нужно обеспечить приемлемое качество кода молодых программистов и нет желания их учить и развивать. Или если в команде есть любители стандартизировать все на свете.
- Тон статьи глупо-категоричный и пренебрежительный.
- Почему предложенный метод
User::getLastPostComments()
нарушает закон Деметры? Почемуthis.posts.getLastPost().getComments()
, а не, скажем,this.posts.getLastPostComments()
? User::addComment()
– плохое название. Это что, добавление комментария к пользователю?- Так "нерушимый закон" или "основываясь на здравом смысле"?
- Что такого постыдного в 3-ей хромосоме?
«User::getLastPostComments() нарушает закон Деметры» – нет, он не нарушает, наоборот это пример применения LoD к коду, мы тут друг друга не поняли
«this.posts.getLastPost().getComments()» – согласен, так будет корректнее
«User::addComment()» – согласен, спасибо, поправлю
«основываясь на здравом смысле» – кто бы и когда не говорил о «догмах», всегда нужно помнить о здравом смысле, поэтому я люблю использовать такого рода противоречие
«Что такого постыдного в 3-ей хромосоме?» – абсолютно ничего, исключительно сатирический оборот
Когда я узнал об этих принципах, качество моего кода выросла х2, а скорость принятия решения х5.
… и как вы это объективно померяли? Или это просто такое приятное чувство подтверждения?
user.getLastPostComments()
И теперь каждый уровень (вложенности) знает обо всех сценариях использования, ага. А уж как на этом удобно писать запросы...
Объективно никак, это клибейт.
Как я говорил, есть варианты, когда это выгодно и один из лучших это именно трансформация данных.
Объективно никак, это клибейт.
Не надо так.
«Кстати, я вот даже про «х5 скорости...» написал именно потому что благодаря знанию данного приема я могу объективно сделать выбор, зная минусы и плюсы, и например спокойно писать «response.Body.ReadStream()» если это подходит ситуации лучше, чем LoD.»
Кстати, чисто из любопытства вопрос появился: чейнинг нарушает LoD?
class Burger {
addCheese() {
...
return this
}
addSalad() {
...
return this
}
addBacon() {
...
return this
}
}
const burger = new Burger().addCheese().addSalad().addBacon()
Боже мой. Куда мы катимся?
… нет, я все-таки не понимаю.
Вот у меня есть очень-очень-очень простая вещь: HTTP-сообщение (то, которое ответ), с заголовками и телом. Ну то есть response.Headers
и response.Body
. И вы мне хотите сказать, что response.Body.ReadStream()
— это неправильный код, и надо писать response.ReadBodyStream()
? А почему? А что делать авторам бесчисленного количества расширений, который добавляют что-нибудь навроде body.ReadAs<T>
? Добавлять еще и response.ReadBodyAs<T>
, который все равно будет внутри делать response.Body.ReadAs<T>
— просто потому, что Body
— это публичный интерфейс, и ничего другого нет? Но зачем? Какая от этого польза?
А с заголовками что делать? Вместо response.Headers.ContentType.Encoding
добавлять response.GetContentTypeHeaderEncoding()
? Это уже прямое нарушение SRP, потому что у вас контракт Response
начинаем иметь больше одной причины для изменения.
Так зачем все эти пляски? Может быть, пора уже начать разделять публичный контракт и внутреннюю структуру?
По-человечески задача звучит: «Я хочу получить Encoding ответа» – поэтому вместо «response.GetContentTypeHeaderEncoding()» должно быть «response.getEncoding()», чтобы как раз таки скрыть что он находиться в Headers -> ContentType, поскольку это лишняя информация.
Тоже самое с «response.ReadBodyStream()», который должен быть «response.ReadStream()», потому что: «Я хочу прочитать ответ», а «response» должен сам определить из какого stream читать. Так у нас рождается интерфейс «StreamReader{ ReadStream(): T }».
Если же звучало: «Хочу прочитать тело ответа» – а не что-то другое ответа, тогда «response.ReadBodyStream()» был бы корректен, но это зависит от конкретной ситуации.
Я понимаю почему могло создаться подобное впечатление, потому что в моих примерах человеческая фраза звучит: «Хочу комментарии последнего поста» – поэтому получается «getLastPostComments» но это не означает что там внутри обязательно будет post -> comments, так может быть мердж нескольких свойств archivePosts + posts -> comments + ratings + reviews, если так требует бизнес-логика.
Короче, да, я привел недостаточно грамотные примеры, подумаю как это поправить, чтобы не происходило путаницы.
+ Пример с Request уже настолько понятная сущность, что она вряд ли сильно измениться и чаще всего будет одинаковая даже между библиотек, поэтому да, не нужен LoD для Request, это нужно более комплексных сущностей (например, Моделям и Агрегатам вашего приложения), в которых могут быть сложные выборки и взаимосвязи.
Кстати, я вот даже про «х5 скорости...» написал именно потому что благодаря знанию данного приема я могу объективно сделать выбор, зная минусы и плюсы, и например спокойно писать «response.Body.ReadStream()» если это подходит ситуации лучше, чем LoD.
должно быть минимальное кол-во указаний на то, откуда берутся данные:
Так почему должно-то? Я точно знаю, откуда я хочу читать данные, и мне нужно, чтобы они были именно оттуда, а не откуда-то еще.
По-человечески задача звучит: «Я хочу получить Encoding ответа»
Неа. Я хочу получить значение, указанное в соответствующем поле заголовка Content-Type
. Потому что я работаю с имплементацией стандарта.
Тоже самое с «response.ReadBodyStream()», который должен быть «response.ReadStream()»,
Нет, не должен быть. Мне не нужен поток, который покрывает собой заголовки (а именно это даст response.ReadStream
).
не нужен LoD для Request
Значит, нет никакого "нерушимого закона", и каждый раз, глядя на отношения, надо думать.
Ну да, надо думать.
я могу объективно сделать выбор
Объективно? Расскажите мне, как вы объективно сделаете выбор в данной ситуации.
И вы так и не ответили, что делать с гигантским дублированием кода для ReadAs
по телу ответа. Или с переносом функциональности всех заголовков (рекомендую посмотреть, сколько их).
Если задача звучит: «Я хочу получить Encoding ответа» – меня не интересует где и как он лежит, может, вообще reponse должен сам решить в какой ситуации какой Encoding отдать, значит «response.getEncoding»
Если задача звучит: «Я хочу получить Encoding типа-контента из заголовка ответа» – значит «response.getContentTypeHeaderEncoding», НО встает проблема с тем, что придется придумывать сотни таких методов прокси для каждого свойства и при этом ни один из них: (1) не мерджит несколько свойств, (2) каждый метод отражает полную линейку зависимости – значит применение LoD не дает никаких преимуществ, значит делаем «response.Headers.ContentType.Encoding»
+ Еще раз: response не нужен LoD, потому что response (1) стандартизирован, (2) не имеет внутри комплексных связей, (3) чаще всего идет как библиотека сразу для всех. LoD хорош для Моделей, Сущностей, Агрегатов или классов, которые имеют глубокие / сложные / множественные связи.
Если задача звучит: «Я хочу получить Encoding ответа» – меня не интересует где и как он лежит, [...] значит «response.getEncoding» [...] Если задача звучит: «Я хочу получить Encoding типа-контента из заголовка ответа» – значит «response.getContentTypeHeaderEncoding»,
Нет, значит response.Headers.ContentType.Encoding
.
И, собственно, это приводит нас к альтернативе:
- я хочу получить кодировку ответа —
response.Encoding
- я хочу получить свойство "кодировка" заголовка
Content-Type
ответа —response.Headers.ContentType.Encoding
Видите, как ответ соответствует задаче? Это "просто" моделирование предметной области. Никакой закон Деметры, внезапно, здесь не задействован.
Еще раз: пример с response некорректен
Да нет, он в полный рост корректен, потому что это код, с которым я работаю каждый день, и если есть какой-то "нерушимый закон крутого кода", он должен к этому коду применяться. А если не применяется, значит, это не "нерушимый закон", а так, рекомендация.
он в полный рост корректен, потому что это код, с которым я работаю каждый день
Я, пожалуй, еще немного здесь разверну. Вот вы пишете "пример с response некорректен, потому что он (1) стандартизирован". Знаете, бизнес-области тоже бывают стандартизированы. Но дело, на самом деле, не в этом. Дело в публичном API.
Вернемся к примеру с постами и комментами. Вопрос, на который надо было отвечать с самого начала — это, есть ли, собственно, свойство Comments
у сущности Post
, или нет.
Если его нет, если нет валидного сценария, при котором пользователь (программист) может получить все комментарии конкретного поста и работать с ними самостоятельно, то и не должно быть этого свойства. А тогда и написать post.Comments
просто нельзя, не о чем говорить.
А вот если такое свойство есть, то программист вполне может написать post.Comments.LastBy(c => c.TimeStamp)
, и будет полностью прав, потому что это, внезапно, публичный API. А если вы, как разработчик сущности Post
решили добавить post.GetLastCommentByTimeStamp
, вы должны гарантировать, что эти две имплементации консистентны — потому что принцип наименьшего удивления.
«post.GetLastCommentByTimeStamp» – позволит вам внести коррективу с «активными» постами только в рамках Post => меньше рефакторинга и опасности для оставшегося кода.
«А что если нам нужно где-то все получать, а где-то только активные?» – тогда появляются «post.GetLastActiveCommentByTimeStamp» и «post.GetLastCommentByTimeStamp», да, больше методов и большие названия (что точно минус), но они человекочитабельные и хорошо поддаются рефакторингу (что плюс).
Если же там дико сложные выборки с ужасными названиями, то просто используем паттерн Спецификация, который очень сильно уменьшит названия и даст больше гибкости, оставив преимущества LoD.
«post.Comments.LastBy(c => c.TimeStamp)» – если когда-нибудь в вашем коде окажется, что вы не можете брать все комментарии, а только «активные», вам придется везде переписать на «post.Comments.LastBy(c => c.TimeStamp && c.isActive)» и тоже самое в любом месте, где есть «post.Comments...»
Если вы имеете в виду, что Comments
теперь обозначает "активные" (потому что у вас написано "в любом месте, где есть «post.Comments...»"), то надо просто поправить Comments
, и все.
А в противном случае, если вы поправите только GetLastCommentByTimeStamp
, что он берет активные, то вы нарушаете тот самый принцип наименьшего удивления, потому что вы меняете существующий публичный API в неожиданную сторону.
тогда появляются «post.GetLastActiveCommentByTimeStamp» и «post.GetLastCommentByTimeStamp»,
Намного проще сделать post.Comments.Active().LastBy(...)
и posts.Comments.LastBy(...)
, потому что надо добавить один метод.
И только когда "последний комментарий" становится бизнес-сценарием со своей выделенной логикой, вы его выносите в спецификацию, и, внезапно, это не обязательно логика поста, это вполне может быть логикой самостоятельного кейса.
просто используем паттерн Спецификация, который очень сильно уменьшит названия и даст больше гибкости, оставив преимущества LoD.
Внезапно, posts.Comments.Active().OnlyByUser(...).Today()
— это и есть спецификация. Но LoD она нарушает.
posts.Comments.Active().OnlyByUser(...).Today()
Спецификация не должна быть частью сущности post, вот здесь есть хороший пример Specification blog.byndyu.ru/2014/07/command-and-query-responsibility.html
И опятьже ".Active()" это раскрытие подробностей, о которых внешнему коду может быть (зависит от бизнес-требований, о чем я писал выше) абсолютно не нужно знать.
Нужно, потому что это случай "нам нужно где-то все получать, а где-то только активные".
А если нам не нужно об этом знать, то это будет спрятано в comment.Posts
, и ничего снаружи трогать будет не надо.
Спецификация не должна быть частью сущности post
А она ей и не является.
А если нам не нужно об этом знать, то это будет спрятано в comment.Posts, и ничего снаружи трогать будет не надо.
Как именно будет спрятано?
А она ей и не является.
Вы выстаиваете цепочку методов от post, а спецификация должна существовать как отдельный класс с chain methods или как command.
Как именно будет спрятано?
Ну как-как. Раньше post.Comments
(извините, я выше перепутал) возвращал все комментарии, станет возвращать только "активные". И все.
Вы выстаиваете цепочку методов от post
Ну да, потому что это читается лучше, чем post.Where(ActiveSpecification())
. Но работает это одинаково.
спецификация должна существовать как отдельный класс с chain methods
А имплементация и лежит в отдельном классе. То, что вы видите как методы на post
— это методы-расширения.
Ну как-как. Раньше post.Comments (извините, я выше перепутал) возвращал все комментарии, станет возвращать только «активные». И все.
Через переопределение getter?
post.Where(ActiveSpecification())
А, вы в формате Active Record пишите, понял. Я использую Repository, чтобы больше сохранять чистоту Сущностей.
Через переопределение getter?
Ну да.
Я использую Repository, чтобы больше сохранять чистоту Сущностей.
А в репозитории ничего не изменится: было repository.Where(ActiveSpecification())
, станет repository.Active()
.
Просто не везде бывают getter (например в Golang в любом случае пришлось бы писать метод getComments)
repository.Active()
Это как-то слишком похоже на QueryBuider, поэтому я скорее за «repository.Where(ActiveSpecification())», но это лично моя вкусовщина.
Переопределение getter – это один из приемов, который позволяет придерживаться LoD.
Нет, с точки зрения LoD ничего не поменялось.
Просто не везде бывают getter (например в Golang в любом случае пришлось бы писать метод getComments)
Ну да, вместо property getter у вас был бы method getter, и… что? Число точек одно и то же.
Нет, не доволен. Ваши утверждения о "крутости кода" как были голословными, так и остались.
Короче, не я это придумал, я просто рассказал простым языком как это работает
Ну, вы сделали какое-то количество утверждений, вот и приходится с этими утверждениями разбираться.
И это утверждение неверно. Если то, с чем надо работать, является публичным API этого объекта, не важно, пользуетесь ли вы LoD.
Если у вас (одна из причин описанных выше) тогда LoD даст положительный эффект, если нет, тогда LoD не поможет.
LoD помогает ответить на вопрос: как вы сами в своем уникальном коде приложения определите публичный API
Нет, не помогает. LoD стимулирует запихнуть весь API в один объект, а это совершенно не обязательно правильно.
Публичный API должен расти из (а) домена и (б) использования.
LoD стимулирует запихнуть весь API в один объект
В объект, который из логики самого домена должен управлять вложенными объектами.
Я думаю, что вам стоит почитать про Агрегаты из DDD, скорее всего, вам они тоже не понравятся, они тоже направлены на то, чтобы управлять всеми операциями, которые относятся к подчиненным / вложенным Сущностям.
а это совершенно не обязательно правильно
Да, именно так, поэтому я и написал в статье, что надо использовать «здравый смысл»)
В объект, который из логики самого домена должен управлять вложенными объектами.
Неа, LoD ничего не говорит про логику домена. Он говорит только про объект, доступный потребляющему коду.
Я думаю, что вам стоит почитать про Агрегаты из DDD
Прежде чем рекомендовать кому-то что-то почитать, стоит задуматься, может быть, ваша рекомендация опоздала. Лет на десять, в данном случае.
Агрегаты мне как раз очень нравятся, другое дело, что в сложных доменах они совсем не так распространены, как хотелось бы думать.
LoD ничего не говорит про логику домена.
Я не говорил что LoD это говорит, наоборот, if бизнес-логика требует управлять вложенными сущностями -> можно использовать LoD
Агрегаты мне как раз очень нравятся
Это супер и они как раз являются отличными примером того, как LoD помогает структурировать код.
Я не говорил что LoD это говорит, наоборот, if бизнес-логика требует управлять вложенными сущностями
Если бизнес-логика требует управлять вложенными сущностями, надо управлять вложенными сущностями, LoD не возникает (потому что снаружи нельзя получить доступ к вложенным сущностям).
они как раз являются отличными примером того, как LoD помогает структурировать код.
И снова нет. Это не LoD помогает структурировать код. Это агрегаты, как концепция, которая растет не из LoD, а из понятия границ консистентности, помогают структурировать код.
- Each unit should have only limited knowledge about other units: only units "closely" related to the current unit.
- Each unit should only talk to its friends; don't talk to strangers.
- Only talk to your immediate friends.
[...]
In particular, an object should avoid invoking methods of an object returned by another method. For many modern object oriented languages that use a dot as field identifier, the law can be stated simply as "use only one dot". That is, the codea.m().n()
breaks the law wherea.m()
does not.
То, что он противоречит куче примеров выше, и просто говорит "нельзя использовать объект, возвращенный другим методом", безотносительно сценариев использования.
Любой адекватный программист понимает, что правила – это ориентир, а не неприкосновенная истина.
Ими можно пользоваться, а можно этого не делать. Выбор за каждым.
Вам не нравится неполнота текста в описании правила на Википедии? Да, ребята ограничились описанием самого правила.
Мне не нравится неполнота правила.
Любой адекватный программист понимает, что правила – это ориентир
Неа. "Любой адекватный программист" понимает, что есть правила, а есть рекомендации, и одно от другого надо отличать.
Мне не нравится неполнота правила.
Можете привести пример полноценного правила?
Неа. «Любой адекватный программист» понимает, что есть правила, а есть рекомендации, и одно от другого надо отличать.
А можете привести пример «правил», которые нарушать нельзя нигде и никогда?
Можете привести пример полноценного правила?
Конечно. "жи/ши пиши с буквой и". Или, если вам нужно правило в программировании, "запрещено обращение к неинициализированной локальной переменной" (в C#).
А можете привести пример «правил», которые нарушать нельзя нигде и никогда?
Ну вот, например, про неинициализированную переменную выше. Да, на самом деле, оба правила выше: если ваш текст на русском языке, в нормативном контексте, будет нарушать жи/ши, он будет неграмотным.
Так вот, хоть и дословный перевод «Law of Demeter» получается «Правило / Закон Деметры», но, конечно же, на деле, это «рекомендация», потому что соблюдение / не соблюдение данного «правила» не повлияет на runtime или компиляцию кода.
Поэтому, какое бы не было название у термина / паттерна, если его несоблюдение не мешает скомпилировать / воспроизвести программу с требующейся логикой это всегда «рекомендация».
Так вот, хоть и дословный перевод «Law of Demeter» получается «Правило / Закон Деметры», но, конечно же, на деле, это «рекомендация»
Так я об этом вам и говорю. А теперь (мысленно) поправим заголовок с учетом этого (и еще пары) выводов: ""Закон Деметры": рекомендация по уменьшению связности кода". Правда же, совсем иначе звучит?
Поэтому, какое бы не было название у термина / паттерна, если его несоблюдение не мешает скомпилировать / воспроизвести программу с требующейся логикой это всегда «рекомендация».
Неа. Потому что еще есть внутренние регламенты и другие веселые вещи, когда скомпилировать можно, и запустить можно, и работать будет, а вмержить все равно не дадут, потому что правила.
Правда же, совсем иначе звучит?
Согласен, но вы очень категоричны, мне нравится мое название, оно завлекает и заставляет людей написать почти 40 комментариев)
… а вмержить все равно не дадут, потому что правила.
Мы говорим про терминологию, а не устав процесса разработки компании, это абсолютно разные вещи. Это как соотносить «замок» на двери, и «замок» при вязке собак.
Поэтому, какое бы не было название у термина / паттерна, если его несоблюдение не мешает скомпилировать / воспроизвести программу с требующейся логикой это всегда «рекомендация». А если вы решили добавить его в устав, тогда становиться «правилом», но только в определенных рамках.
Согласен, но вы очень категоричны, мне нравится мое название, оно завлекает и заставляет людей написать почти 40 комментариев)
Ну то есть ваша цель — она "получить побольше комментариев"? Ок.
Мы говорим про терминологию, а не устав процесса разработки компании, это абсолютно разные вещи. Это как соотносить «замок» на двери, и «замок» при вязке собак.
Вот как раз с точки зрения терминологии и то, и другое — "правило".
А если вы решили добавить его в устав, тогда становиться «правилом», но только в определенных рамках.
А "правила" вообще существуют в определенных рамках.
Ну то есть ваша цель — она «получить побольше комментариев»?
Начать диалог с аудиторией, потому что в процессе диалога рождается много полезных мыслей и идей. Например, наша с вами ветка – прекрасный тому пример.
А «правила» вообще существуют в определенных рамках.
Оно называет «Правило / Закон Деметры», вы хотите чтобы я перевел это как-то по-другому? Кажется, вы не заметили, что лично я использую в статье слово «принцип»
наша с вами ветка – прекрасный тому пример.
Наша ветка — прекрасный пример тому, что несмотря на приличное количество контрпримеров, основной текст (и тон) вашего поста остался без изменений.
Оно называет «Правило / Закон Деметры», вы хотите чтобы я перевел это как-то по-другому?
Я хочу, чтобы вы не пропагандировали идею, что что-то, называющееся "законом Деметры" на самом деле является сколько-нибудь объективным правилом, ведущим к хорошему коду.
Наша ветка — прекрасный пример тому, что несмотря на приличное количество контрпримеров, основной текст (и тон) вашего поста остался без изменений.
Да, потому что это мой пост и мое мнение, а есть ваше мнение.
Я хочу, чтобы вы не пропагандировали идею, что что-то, называющееся «законом Деметры» на самом деле является сколько-нибудь объективным правилом, ведущим к хорошему коду.
Для вас нет, а мне и моим коллегам знание этого принципа полезно для принятия решения использовать или не использовать его в той или иной ситуации.
У всех есть свои догмы, у меня свои. Я, например, считаю, что типизация приносит исключительно положительный эффект, а есть люди, которые против использования типизации. Я с ними дел вести не буду.
У вас есть позиция против LoD, ок, это ваша позиция.
Посмотрел википедию, ссылки в ней. Создаётся впечатление, что первый пример совсем не в тему, второй частично в тему.
Плохо — post.comments().push(newComment)
, хорошо — post.addComment(newComment)
.
Прокидывать это всё на уровень класса User
смотрится как рецепт создания god object'а.
И, как добрая половина принципов ООП, этот закон лучше подходит к ФП (причём не обязательно даже в чистых функциях). Потому что там, действительно, лучше написать функцию getLastPostComments(user)
и тестировать её как отдельную сущность. И при этом не приходится ломать копья насчёт того, посты пользователя — это должны быть user.posts
или allPosts.filterByUser(user)
.
Мне больше нравится другое определение Закона Деметры – через пример.
Когда вы хотите что-то купить, надо дать деньги, а не, например, кошелёк, из которого можно взять деньги.
То есть в методы надо передавать минимально, только то, что им нужно, а не какой-то объект-контейнер, из которого это что-то можно достать.
(Не)рушимые законы крутого кода: Law of Demeter (с примерами на TypeScript)