Comments 62
Почему IUsersRepository.User(int Id) вместо IUsersRepository.Get(int Id) по аналогии с add?
Потому что Get — это глагол, а никакого значимого действия мы указывать не хотим, только то, что вот пользователь с идентификатором ID.
Но и User — не предел. Я улучшу это имя, развив ряд соображений из этой статьи, в статье следующей.
Почему же
а никакого значимого действия мы указывать не хотим?
Я хочу именно что указать, что мне нужно получить пользователя, разве нет?
Просто в Вашем случае интерфейс получается примерно такой
Add()
Update()
User()
т.е. неконсистентный, что по мне гораздо хуже для читабельности.
Но посмотрим, что будет в следующей статье
Загрузка объекта из хранилища — тоже действие, притом дорогое (по сравнению с работой с объектами в памяти).
Загрузка объекта из хранилища — тоже действие, притом дорогое (по сравнению с работой с объектами в памяти).
Загрузка — это Load, а не Get.
Кстати, даже тут можно уйти от Load. По контексту хранилища обычно понятно, будет это дорогостоящая операция или нет. Например, database.User(id) — мы видим, что обращение User происходит на объекте базы, поэтому понимаем: операция дорогостоящая.
Кроме этого, если контракт (интерфейс) обобщает способ получения и, вообще, конкретные детали, то уточнение стоимости получения — нарушение инкапсуляции.
Сильнее было бы пойти от самой сущности — Discount с методом static decimal Of(Customer customer, Order order), чтобы вызывать Discount.Of(customer, order) — проще, чем _discountCalculator.CalculateDiscountBy(customerId)
А как этот статический класс инджектить в IoC — container? DDD часто используется в вебе, где без этого никуда (да и не только в вебе без этого никуда)
Если нужно инжектить, то пожалуйста:
public interface IDiscount
{
decimal Of(Customer customer, Order order);
}Вызов тогда:
var discount = _discount.Of(customer, order);
// Но никак не:
var discount = _discountCalculator.CalculateDiscountBy(customerId, order);Суть осталась та же, изменилась только форма.
Т.е. в достаточно сложном коде появляются односмысленные (но принципиально разные) действия (вычисление/установка и т.д.) над разнотипными объектами. А если сверху это всё и иерархией с дженериками накрыть — так вообще уже через несколько часов сам автор перестанет понимать .of() чего он делает.
А так у нас есть CalculatDiscount, CalculateNDS (о, ужас, я знаю — просто не помню как это по английски — тоже проблема, кстати), CalculateTotalSum и т.д. И уже код читается почти как английский текст и от точек в глазах не рябит.
Я бы не хотел отходить от темы, размышляя "а что если". Боюсь, тогда мы тут надолго застрянем.
Совершенно точно, что простое IDiscount имеет свои пределы, но пределы эти — часть другого обсуждения, мне же оно нужно было для того, чтобы показать идею.
Кстати, если взять CalculateDiscount(), CalculateNDS() и CalculateTotalSum(), то проще — Discount(), NDS() и TotalSum(). Что оно Calculate, и так понятно.
К тому же английский язык бывает коварен. TotalSum() может и понятен, а вот просто Sum() — уже нет. Это глагол (меняющий внутреннее состояние объекта) или существительное? Тоже самое: Add это совсем не то же самое что Push (в первом случае непонятно что в конец).
Как вы должно быть поняли — пишу с точки зрения джависта и джавовский гайдлайнов, где это вопрос именования расписан и стандартный вариант именно глагол-существительное (что и над чем).
Не всегда понятно, когда работа идёт с этими методами внутри класса, где, скорее всего уже есть приватные или локальные переменные с такими названиями.
Да, существуют ситуации, когда уже есть какие-то переменные или поля с такими же именами. Но и их можно обойти, всё зависит от конкретного случая, главное понимать, зачем глагол, а зачем существительное. В случае с Calculate глагол уж точно ни к чему.
Как вы должно быть поняли — пишу с точки зрения джависта и джавовский гайдлайнов, где это вопрос именования расписан и стандартный вариант именно глагол-существительное (что и над чем).
В C# для методов существует такое же правило. Интересно, что если метод, то Get (или какой-нибудь Calculate) пишется, а вот если свойство (синтаксически это метод без параметров и без скобочек) — то нет. Как если бы от физического воплощения (свойство или метод) одного и того же понятия мы бы по разному говорили о нём между собой.
Думается мне, критерий всегда один: говорят "скидка", значит Discount, и т.д., тут в комментарии ниже описывал.
Код тогда строится не по действиям, императивно, а описательно: не "скидка считается считателем скидок по формуле", а "скидка — это...". Разумеется, там где действие, там остаётся царствие глагола.
Интересно, что если метод, то Get (или какой-нибудь Calculate) пишется, а вот если свойство (синтаксически это метод без параметров и без скобочек) — то нет. Как если бы от физического воплощения (свойство или метод) одного и того же понятия мы бы по разному говорили о нём между собой
Ну да, это потому что в случае свойства действие (гет или сет) уже передается синтаксисом и дублировать его словесно не надо. order.Discount = 5 — свойство слева от присваивания, поэтому сет.
К тому же у метода множество различных возможных вариантов действия, а у свойства только два, поэтому нет неоднозначности, в отличие от метода
Ну, название интерфейса IDiscount очевидно не хорошее, с точки зрения естественного языка, так как не передает совершаемого им действия
Совершаемое действие не имеет значения. Имеет значения результирующее понятие.
Думаю, просто следует проявить изобретательность.
public interface IDiscount
{
(decimal Value, decimal AdditionalValue) Of(Customer customer, Order order);
}Всегда можно поиграться и найти то, что подходит. Если на каждое улучшение находится "если", это не значит, что нужно вообще ничего не улучшать и писать повсюду: _discountCalculator.CalculateDiscount (ещё утверждается, что это DDD).
Попробуйте поискать FindUsages такого AdditionalValue — найдутся обращения к Item2 всех кортежей.
Попробуйте поискать FindUsages такого AdditionalValue — найдутся обращения к Item2 всех кортежей.
И вот мы начинаем обсуждать, хороши ли именованные кортежи, как часто их приходится рефакторить, искать использования полей...
Изначальный посыл: использовать понятия из предметной области, избегать повторений. В предметной области вычислителей скидок нет, а _discountCalculator.CalculateDiscount содержит слова discount и calculator по два раза. Поэтому нужно думать.
Кстати, есть ещё вариант:
public interface IDiscount
{
void Apply(Customer customer, Order order);
}Но и тут можно придраться к тому, менять Order нельзя. Как и к тому, что скидка с несколькими полями — вещь сомнительная. Разумеется, Apply можно переделать на Apply(ref price, Customer customer, Order order), но и тут можно найти минусы.
Без контекста все эти конкретные размышления парируются каким-нибудь "а что если", при этом изначальный посыл от этого не меняется.
Кстати, есть ещё вариантПлохой, потому что опять не решает проблему управления скидками как сущностями. После модификации Order этим методом теряется информация, сколько заказ стоил до скидки. Также легко себе представить справочник скидок, где у скидки есть сроки действия, или они подключаются списком детализации к строке заказа. Тут и вырисовывается DiscountManager как сервис, и Discount как сущности, с которыми он работает.
Как и к тому, что скидка с несколькими полями — вещь сомнительнаяВовсе нет, потому что типичная скидка в сложных системах — алгебраический тип-сумма, может быть либо процентной, либо абсолютной. Также неплохо бы хранить основание для скидки (постоянному клиенту, или по акции №55).
После модификации Order этим методом теряется информация, сколько заказ стоил до скидки
Зависит от реализации.
Тут и вырисовывается DiscountManager как сервис, и Discount как сущности, с которыми он работает.
Боюсь, вы от плохо тогда перешли в чудовищно плохо. Это неподдерживаемый, непонятный код. Применение и подсчёт скидки должен быть в скидке, а не в каком-то менеджере. Всегда можно обойтись без него.
Кстати, вот ещё в голову пришёл вариант:
public interface IDiscounts
{
IDiscount One(Customer for, Order and);
}
public interface IDiscount
{
public int Value { get; }
public DiscountType Type { get; }
public DateTime WillBeAvailableTo { get; }
}
// Вызываем так:
Order order = ...;
Customer customer = ...;
IDiscounts discounts = ...; // Инжектим и т.д.
var price = Price(of: order); // Как-то считается цена.
var discount = discounts.One(for: customer, and: order);
discount.Apply(ref price); // `Apply` или внутрь `IDiscount`, или методом расширений.Даже такой скорый вариант сильнее всякого менеджера или хелпера хотя бы тем, что он не размазывает ответственности. Думаю, можно намного лучше, если иметь хотя бы какое-то представление о конкретной задаче и её особенностях.
Кстати, вот ещё в голову пришёл вариант:Ну, замечательно. Вы изобрели DiscountService/DiscountManager под именем Discounts.
Если вы собираетесь отстаивать своё наименование, то представьте, что ваш коллега Вася из соседнего отдела напилил кучу интерфейсов: IDiscount, IOrder, IOrders, IShop. Что из них репозиторий, что из них сущность, что сервис — непонятно.
Можно экономить на названиях, но через 3 года сам же откроешь свой код, и будешь вспоминать, почему
IOrders — это репозиторий, а IDiscount — сервис с методом void Apply(Customer customer, Order order);Применение и подсчёт скидки должен быть в скидке, а не в каком-то менеджереТакой подход убивает абстракцию. Если у нас несколько каналов продаж, можно было бы сделать объект Discount, содержащий лишь информацию о значениях скидок, а их вычисление отдать сервисам, которые будут писать разные люди.
У вас получается, что скидка должна знать про каждый канал продаж, появление нового канала потребует внесения изменений в код Discount. Это нормально, когда пилишь сам себе простой проект, в котором сам всё контролируешь. Но плохо для промышленной разработки.
Ну, замечательно. Вы изобрели DiscountService/DiscountManager под именем Discounts.
Имя — это всё.
Что из них репозиторий, что из них сущность, что сервис — непонятно.
Это избыточные детали. Приложение-то по работе с заказами, а не с сервисами заказов.
Можно экономить на названиях
Но ведь это не экономия на названиях, а использование слов из предметной области.
Можно экономить на названиях, но через 3 года сам же откроешь свой код, и будешь вспоминать, почему IOrders — это репозиторий, а IDiscount — сервис с методом
void Apply(Customer customer, Order order);
Единственное, что можно подумать: IDiscount — это скидка, а IOrders — заказы.
Такой подход убивает абстракцию.
Боюсь, DiscountManager — это не абстракция...
Если у нас несколько каналов продаж, можно было бы сделать объект Discount, содержащий лишь информацию о значениях скидок, а их вычисление отдать сервисам, которые будут писать разные люди.
Это новое "если".
У вас получается, что скидка должна знать про каждый канал продаж, появление нового канала потребует внесения изменений в код Discount.
Если я верно вас понял, вы предлагаете переиспользовать "лишь информацию о скидке". А если в каждом из каналов скидки выражаются разными данными? А если где-то выдаётся только процентная скидка? Если она не всегда ограничена по времени?
На мой взгляд, вот уж что точно не получится масштабировать и удобно использовать, так это переиспользование информации. Ведь переиспользуются объекты.
А если в каждом из каналов скидки выражаются разными данными? А если где-то выдаётся только процентная скидка? Если она не всегда ограничена по времени?Придётся данные скидки привести к общему знаменателю, под максимальный случай. Ведь, как это часто бывает, захотят видеть агрегированную статистику, а агрегировать можно однородные данные.
Thread.Sleep(5.Minutes())Как раз об этом и будет заключительная статья!
5.Minutes()Как финтифлюшка в HelloWorld забавно смотрится, но не масштабируется для большого проекта.
Потому как в
DispatcherTimer(5.Minutes()) Minutes должен вернуть TimeSpan, а в контексте, когда нужны тики (FileTime) — Minutes должен вернуть тоже целое число, но не в миллисекундах, а в других единицах, сотнях наносекунд. И что с этим Minutes делать, когда единственное применение намертво прибито к типу int?Как финтифлюшка в HelloWorld забавно смотрится, но не масштабируется для большого проекта.
Хм. Сперва постулируется: "не масштабируется для большого проекта", а затем как подтверждение — довольно частный пример, который может касаться лишь одной из подсистем этого самого большого проекта.
Кроме этого, надо понимать, 5.Minutes() работает там, где работает, а там, где не работает, ищется другое.
Положим, описываю я дебаффы в игре:
Debuff(CriticalChanceDecrease, power: 10, duration: 5.Minutes());Теперь масштабируем эту строчку в десятки раз, и получаем вполне приличный, читаемый код. Система, которой нужны тики, а также иные, сложные, требующие технической сноровки, не пострадали.
Не получается единообразия.
что этот подход нельзя перенести на остальные подсистемы
Зачем это может быть нужно? Каждая подсистема имеет свой ограниченный контекст. Жертвовать читаемостью кода из-за того, что он не масштабируется на разработку операционных систем для ракетостроения, — не самое лучшее решение.
Простой пример, который доставлял бы мне дискомфорт в программировании если бы английский язык был бы для меня родной. Door->open() vs Open->door().
Да, в ООП предполагается такое незначительное нарушение грамматики, но из него никак не следует, что нужно идти в диаметрально противоположное направление и писать хелперы.
К сожалению, не могу подтвердить или опровергнуть это мнение. В основном наблюдаю пренебрежение к языку и словам, из-за которого код получается неопрятным, неаккуратным, но зато рабочим, как если бы его работоспособность была конечным продуктом.
Разве что, случается (ведь я сам не носитель): некоторые слова кажутся вычурными и неестественными, потому что редко их встречаешь или до этого не знал.
QueueWorkItem vs Execute — имхо, первое намного лучше: оно даёт понимание того, что ThreadPool исполнит задачу «когда-нибудь», не обязательно сейчас. Execute такого понимания не даёт и это достаточно больная проблема, имхо
На мой взгляд, сама семантика вызова: ThreadPool.Execute предполагает, что действие не будет выполнено мгновенно, поскольку мы работаем в контексте ThreadPool.
Никогда бы не подумал что-то вызвать на тредпуле, не понимая, что это в действительности значит: что там есть потоки, они управляются, что не нужно их надолго задерживать. Тип ThreadPool сообщает вполне достаточно.
Но аргумент понятен. Как вам: ThreadPool.Queue(action)? Зачем WorkItem? Что это значит?
WorkItem это объект доменной модели если можно так выразится. Например, в приложениях под IIS WorkItem не просто становится в очередь и выполняется когда-то, но и также не даёт IIS pool уйти в recycle до тех пор пока он не выполниться до конца. Да это не очевидно, но в MSDN про это пишут.
Вообще, в целом я с Вами согласен (особенно надеюсь на статью код-как-английский-язык),
но! ИМХО Вы слишком категоричны, рассмотрите варианты типа threadPool.QueueWorkItem(() => {… }) — вполне возможно что внутри скобочек не
будет слова action => неискушённому взгляду не будет понятно что там ставят в очередь…
Вообще, в целом я с Вами согласен (особенно надеюсь на статью код-как-английский-язык)
Эта часть тоже уже вышла, но, судя по комментариям, она вас разочарует.
но! ИМХО Вы слишком категоричны
Того требует форма. Если бы на каждый пример я дописывал "вообще, бывают вот такие исключения, а вот в этих ситуациях всё-таки лучше так, а ещё случается...". Категоричность как раз была целью, поскольку её различаемость выше, чем у средних рассуждений (не в ущерб оным).
рассмотрите варианты типа threadPool.QueueWorkItem(() => {… }) — вполне возможно что внутри скобочек не
будет слова action => неискушённому взгляду не будет понятно что там ставят в очередь…
И всё же, давайте представим сценарий. Разработчик, имеющий представление о потоках и о том, как они управляются в пуле (а это почти обязательное требование, чтобы им пользоваться), смотрит на запись ThreadPool.Run( () => { ... } ) и такой думает: "Не, бред какой-то. Что тут происходит?". Потом встречает ThreadPool.QueueUserWorkItem, и тотчас восклицает: "А, так это ж UserWorkItem! Теперь ясно!".
Боюсь, разработчики просто привыкают к QueueUserWorkItem, и для них это просто alias выражения "Вызвать метод на тредпуле".
Боюсь, разработчики просто привыкают к QueueUserWorkItem, и для них это просто alias выражения «Вызвать метод на тредпуле»Просто обманывать не надо. Если после завершения метода Run фактически выполнение не началось — это наименование врёт.
Просто обманывать не надо
Вы используете слово выполнить без контекста, тогда как в действительности это будет выполнить на тредпуле. Выполнение чего-то на тредпуле имеет другую семантику, чем просто выполнение чего-то. Почему бы тогда не писать ThreadPool.FindFreeThreadAndExecuteUserWorkItem(action)? А то QueueUserWorkItem не сообщает о том, что выполнение действия будет совершено только тогда, когда в тредпуле есть свободный поток.
Если после завершения метода Run фактически выполнение не началось — это наименование врёт.
Не имеет значения, началось ли оно фактически. Это инкапсулировано. Наименование врало бы, если бы разработчик ожидал от него мгновенного выполнения, а он этого не ожидает.
Почему бы тогда не писать ThreadPool.FindFreeThreadAndExecuteUserWorkItem(action)?Потому что это неверно. Метод не ищет свободный поток. Он добавляет в очередь, и на этом его ответственность заканчивается. Если в имя метода добавить «Find», может показаться, что от наличия свободного потока зависит результат (если такого нет, метод вернёт false, например).
Потому что это неверно. Метод не ищет свободный поток.
Это потому что вы знаете, как устроен ThreadPool внутри. А метод, меж тем, не должен сообщать, что он делает внутри.
Мне, как клиенту ThreadPool.QueueUserWorkItem совершенно безразлично, будет там внутри очередь или Scheduler. У меня есть действие, и я хочу выполнить его на потоке из пула. Поэтому: "Пул, подыщи-ка мне поток, и выполни эту задачу" — вот вам и FindFreeThreadAndExecuteUserWorkItem получился.
Если в имя метода добавить «Find», может показаться, что от наличия свободного потока зависит результат (если такого нет, метод вернёт false, например).
Если прочитать только Find, но там есть ещё AndExecute.
В общем, на мой взгляд, всё это несущественные споры. Основная претензия была не Queue (хотя и к нему есть), а к UserWorkItem.
Как вам ThreadPool.Queue?
Если прочитать только Find, но там есть ещё AndExecute.«And» можно понять так, что действия выполняются последовательно. Если Find не успешен, Execute не выполняется.
Как вам ThreadPool.Queue?Для данного класса — подходит, потому что ThreadPool с другими сущностями не работает. Но Microsoft можно понять: есть классы (например, Directory), которые работают с разными сущностями. Поэтому не просто Directory.Enumerate, а EnumerateFiles и EnumerateDirectories, и вообще, глагол+существительное как стандарт.
Тут консистентность наименований во всех классах важнее, чем экономия на одном простом классе.
Тут консистентность наименований во всех классах важнее, чем экономия на одном простом классе.
Мне так не кажется. Простой и читаемый код важнее, чем внутренняя согласованность библиотеки. Но речь, разумеется, не про Microsoft конкретно, а про общий подход.
Поэтому не просто Directory.Enumerate, а EnumerateFiles и EnumerateDirectories, и вообще, глагол+существительное как стандарт.
Зачем так, если можно хотя бы Directories.Of(path) и Files.Of(path)? Глагол не нужен совершенно. При этом лучше вообще использовать методы расширений, чтобы передать "субъектный" оттенок: path.AsDirectory().Files и path.AsDirectory().Directories. Тогда и расширяемость выше.
IMHO, вызывает только желательные коннотации и в достаточном количестве.
Не согласен. Ассоциация только у тех, для кого английский не родной и кто работал только со стеком.
В C++, например, у deque есть push_front, push_back, pop_front, pop_back, а у queue только push (в конец) и pop (из начала). То есть уже понимание не такое, как вам кажется.
У vector и многих прочих — только push_back.
Тот, кто их знает, уже не считает, что push всегда положить туда, откуда его же возьмёт pop.
А у нейтива вообще понимание, что push может быть какой угодно и где угодно, и понимать его надо по смыслу области применения.
Код живой и мёртвый. Часть вторая. Действия и свойства