Здравствуйте, stump, Вы писали:
S>Поэтому, с точки зрения здравого смысла (который у некоторых не в почете) сумму ордера конечно же надо хранить. S>Как это может повлиять на дизайн? Или никак не повлияет, руководствуемся "четким принципом" и все.
Мои решения вобщем-то не особо зависят от того, планируется ли менять систему подсчета в течении жизенного цикла программы
// вариант когда есть шансы подсчитывать разные ордеры по разномуpublic class Order1
{
decimal? total;
public decimal Total
{
get
{
if (total == null)
throw new InvalidOperationException();
return total.Value;
}
}
public void CalculateTotal(IOrderTotalCalculator calc)
{
calc.Calcuate(this);
}
}
// вариант когда таких шансов практически нет, подсчет тривиаленpublic class Order2
{
decimal? total;
public decimal Total
{
get
{
if (total == null)
total = CalculateTotal();
return total.Value;
}
}
private decimal CalculateTotal()
{
return 5;
}
}
// вариант когда таких шансов практически нет, подсчет нетривиаленpublic class Order3
{
IOrderTotalCalculator calc;
decimal? total;
public decimal Total
{
get
{
if (total == null)
total = calc.CalculateTotal(this);
return total.Value;
}
}
}
И наконец четвертый вариант — хранение Total отдельно в классе содержащем Order и его аггрегаты.
И все эти решения могут кардинально измениться в зависимости от множества неуказанных требований.
CRA>>void Unknown()
CRA>>{
CRA>> decimal total = OrderHelper.CalaTotal(order); //получение значение
CRA>> order.Total = total; //изменить состояние
CRA>>}
CRA>>
A>Несогласен! Никакой это не бред. A>Он действительно нарушает SRP в случае, если изменить состояние order это не единственная его задача. Т.о. если этот код попадает в контроллер UI, например, мы получаем нарушение SRP: следить за Total никак не входит в его обязанность. A>Если же это его единственная задача, то возникает вопрос: зачем нам третий ( ) класс, у которого все обязанностей -- обновить Total у order.
Посмотри на метод, который я специально не стал удалять.
Еще раз — есть две задачи: вычислить значение и изменить состояние персистентного объекта. Это разные задачи, объединение их в одном флаконе есть классическое нарушение SRP
Подскажи пожалуйста, как решить задачу перерасчета Total не нарушая твоего принципа SRP
Хочу обратить внимание, что Unknown — это метод.
Здравствуйте, AndrewVK, Вы писали:
A>>В общем, в библиотеках от extension methods проблем больше, чем пользы. AVK>Расскажи это МС, которые, вот идиоты, класс Enumerable в библиотеку воткнули.
Он не расширяет классы той же библиотеки. По существу (где именно существо я указал выше, напомню это про LeftSubstring и RightSubstring) тебе, видимо, сказать нечего и ты придираешься к мелочам. А жаль.
Здравствуйте, C...R...a...S...H, Вы писали:
CRA>Получается интересная ситуация, либо Вы все забыли пока обсуждали переименование метода UpdateTotal в CalcTotal. CRA>Либо Ваша любимая методика подмены понятий дает о себе знать.
Ничего я не подменяю. Речь о неправильности изменений шла именно в контексте исходного примера. Т.е. я предполагал, что твой код является логическим продолжением того, исходного примера. Т.е. никаких хитрых внутренних методов Calc там нет, иначе бы было бы логично их сразу и продемонстрировать.
Я вижу, аргументы по делу уже вообще даже не упоминаются. Думаю, на сем стоит закончить.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, adontz, Вы писали:
A>(где именно существо я указал выше, напомню это про LeftSubstring и RightSubstring) тебе, видимо, сказать нечего и ты придираешься к мелочам. А жаль.
Это ты какой то левый пример придумал и теперь пытаешься втиснуть его в качестве аргумента.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, AndrewVK, Вы писали:
A>>(где именно существо я указал выше, напомню это про LeftSubstring и RightSubstring) тебе, видимо, сказать нечего и ты придираешься к мелочам. А жаль. AVK>Это ты какой то левый пример придумал и теперь пытаешься втиснуть его в качестве аргумента.
Андрей, Рома действительно написал большой пост, многое в нем действительно по делу, но ты самое главное, относящееся к теме дискуссии, оставил без комментариев, а начал писать замечания по мелочам.
Здравствуйте, MozgC, Вы писали:
MC>Андрей, Рома действительно написал большой пост, многое в нем действительно по делу, но ты самое главное, относящееся к теме дискуссии, оставил без комментариев, а начал писать замечания по мелочам.
Я ответил на то, что посчитал самым важным. Если тебе интересны мои комментарии к чему то конкретно — приводи цитаты, постараюсь ответить.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, AndrewVK, Вы писали:
A>>(где именно существо я указал выше, напомню это про LeftSubstring и RightSubstring) тебе, видимо, сказать нечего и ты придираешься к мелочам. А жаль. AVK>Это ты какой то левый пример придумал и теперь пытаешься втиснуть его в качестве аргумента.
Есть ты не видишь проблемы в том, что метод LeftSubstring описан в классе, так как для его эффективной реализации необходим доступ к приватным полям, а метод RightSubstring описал в хелпере, потому что его эффективная реализация не требует доступа к внутренним полям, то я тоже не вижу предмета для обсуждения с тобой. Как можно обсуждать с человеком то, что он не видит?!
Здравствуйте, adontz, Вы писали:
A>Есть ты не видишь проблемы в том, что метод LeftSubstring описан в классе, так как для его эффективной реализации необходим доступ к приватным полям, а метод RightSubstring описал в хелпере, потому что его эффективная реализация не требует доступа к внутренним полям, то я тоже не вижу предмета для обсуждения с тобой.
Нет, я не вижу, с какой это стати LeftSubstring требует доступа, а RightSubstring нет. Кроме того, по вопросам производительности и дизайна я в этом топике уже высказывался. Re[16]: Куда девать ф-ции внешние для класса
Здравствуйте, AndrewVK, Вы писали:
AVK>Ничего я не подменяю. Речь о неправильности изменений шла именно в контексте исходного примера. Т.е. я предполагал, что твой код является логическим продолжением того, исходного примера. Т.е. никаких хитрых внутренних методов Calc там нет, иначе бы было бы логично их сразу и продемонстрировать.
Продублирую свой первый пост (и добавлю комментарий):
public class Order
{
public IList<OrederLine> OrderLines {get;set;}
public decimal Total{get;set;}
}
///Может быть Хелпером либо Иньекциейpublic class OrderTotalCalculator
{
public decimal UpdateTotal(Order order)
{
//метод Calc тут
}
}
Из первых ваших постов я сделал вывод что, вам это понятно.
Даже Вы сами так написали
я логично предположил, что, если у тебя нигде собственно вычислений не заявленно, значит они внутри Update
Теперь я так понимаю, у Вас новая зацепка,
никаких хитрых внутренних методов Calc там нет
Похоже, и для этой ветки разговра у Вас не осталось аргументов, и как правильно заметил adontz Вы начинаете придираться к мелочам.
Понятно, аргументов по прежнему нет, попытка вывернуть все на изнанку наличествует. Продолжать мне не интересно. Будут вопросы по делу, а не очередные попытки поймать на какой нибудь ерунде — обращайтесь.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, AndrewVK, Вы писали:
AVK>Нет, я не вижу, с какой это стати LeftSubstring требует доступа, а RightSubstring нет.
Например, LeftSubstring повторно использует тот же массив char[], просто указывая новую длину. RightSubstring создаёт новый.
По существу есть что сказать или "я не верю, что LeftSubstring может требовать доступ, а RightSubstring нет" это всё что ты можешь ответить?
Глубинную проблему, которую я пытался доходчиво проиллюстрировать, а именно, раскидывание по разным классами логически связанных, в том числе парных, методов при использовании правила "Если достаточно публичного контракта, метод переносится из основного класса в хелпер" ты не видишь?
Z>А мне кажется вы оба не правы. Z>Если подсчет Total выносить из класса Order, то в нем не должно остаться ни Total, ни CalcTotal ни других упоминаний о том, что у него есть Total. Это уже ответственность OrderTotalCalculator.
Ты будешь смеяться, но я считаю что правы все Каждый по своему и каждый в своем контексте
Давай от теории перейдем к практике: I Анемичная модель:
а) Класс Order нужен для бизнесс логики.
Бизнесс логика знает о том, кого нужно спросить, чтобы подсчитать Total. Total здесь действительно ни к чему:
public class Order
{
public IList<OrderLine> OrderLines {get;}
}
б) Класс Order нужен для показа пользователю (UI) и для сохрания в базе (DAO)
ДАО совершенно не должна знать про то где и как достать Total. C UI не так все категорично, но все же желательно, чтобы UI тоже не знал где брать Total (меньше знаешь -- крепче спишь ).
Поэтому в этом случае мы имеем следующую реализацию:
public class Order
{
public IList<OrderLine> OrderLines {get;}
public decimal Total {get; set;}
}
II Rich модель:
Скорее всего тут тоже будет несколько вариантов, но я покажу тот, что больше всего нравится мне (именно его я бы и использовал в своем приложении):
Класс Order сам ответственнен за свою целостность и согласованность. Он контролирует добавление и удаление строк заказа и инициирует пересчет стоимости при изменении.
public class Order
{
private readonly IList<OrderLine> _orderLines = new List<OrderLine>();
private decimal _total = 0M;
public IEnumerator<OrderLine> GetOrderLinesEnumerator()
{
return _orderLines.GetEnumerator();
}
public void AddOrderLine(OrderLine orderLine)
{
_orderLines.Add(orderLine);
updateTotal();
}
public void RemoveOrderLine(OrderLine orderLine)
{
_orderLines.Remove(orderLine);
updateTotal();
}
public decimal Total
{
get { return _total; }
}
private void updateTotal()
{
// тут может быть DI, ServiceLocator или статический хелпер как ниже
_total = OrderTotalCalculator.CalculateTotalFor(this);
}
}
Судить о достоинствах и недостатках каждого подхода предоставляю вам.
CRA>Посмотри на метод, который я специально не стал удалять. CRA>Хочу обратить внимание, что Unknown — это метод. Чей метод? Вот в чем вопрос
Вот именно об этом я и говорил:
Он действительно нарушает SRP в случае, если изменить состояние order это не единственная его задача. Т.о. если этот код попадает в контроллер UI, например, мы получаем нарушение SRP: следить за Total никак не входит в его обязанность.
Если же это его единственная задача, то возникает вопрос: зачем нам третий класс, у которого все обязанностей -- обновить Total у order.
Заметь, я нигде не говорю какой класс содержит этот метод. Я рассматриваю два возможных варианта: у класса нет больше обязаностей кроме как следить за Total для Order и у класса есть такие обязанности.
CRA>
CRA>Еще раз — есть две задачи: вычислить значение и изменить состояние персистентного объекта. Это разные задачи, объединение их в одном флаконе есть классическое нарушение SRP
CRA>Подскажи пожалуйста, как решить задачу перерасчета Total не нарушая твоего принципа SRP
Никакой он не мой. Он общий и в большинстве случаев интуитивно понятный.
Пример кода здесь: Re[25]: Куда девать ф-ции внешние для класса
Здравствуйте, stump, Вы писали:
S>Поэтому, с точки зрения здравого смысла (который у некоторых не в почете)
Кто бы это мог быть?
S>сумму ордера конечно же надо хранить.
С точки зрения здравного смысла, надо хранить не сумму (алгоритм рассчета которой так же может поменяться), а ставку налога на момент формирования заказа. Впрочем, для финансов, наверное, и сумму тоже имеет смысл хранить.
S>2. Может быть несколько способов расчета суммы, они могут меняться в течении жизненного цикла прогаммы (например завтра будет програссивная шкала налога). S>Как это может повлиять на дизайн?
Повлиять на дизайн может так:
1. Метод рассчета суммы находится внутри класса. При появлении нового алгоритма необходимо модифицировать код класса Order вписав туда еще одну функцию рассчета с реализацией нового алгоритма.
2. Метод рассчета суммы находится вне класса. При появлении нового алгоритма достаточно просто добавить еще одну функцию снаружи, не модифицируя класс Order.
При появлении нового функционала намного предпочтительнее добавлять новый код, а не модифицировать имеющийся.
S>З.Ы. Однако какой знатный холивар пошел
Отож.
Здравствуйте, adontz, Вы писали:
A>Никакое усердно использование правила не заменит усердное использование мозга.
Кто бы спорил. Главное только пользовать его в правильном направлении..
A>Правило — это всего лишь правило и его повсеместное необдуманное применение принесёт больше вреда чем пользы, как и использование любого другого правила.
Вот stump бы на такое обиделся и сказал бы, что он с хамами не разговаривает и что ты вообще любитель передергивать. Но я не гордый, я отвечу.. С чего ты взял, что я за "необдуманное использование правила"? Я где-то призывал не думать? Если так, то тебе наверняка будет не сложно привести нужную цитату.
Проблема ровно в том, что так и не смогли привести ни одного сценария, где бы это правило не работало, и я не вижу ни одной разумной причины его не применять (ну, разьве что по быстрому надо пару классов набросать и забыть).
A> У тебя внешний интерфейс библиотеки — контракт для общения с другими компонентами, построен не на основе понятий предметной области, здравого смысла, и т.п., а на основе исключительно деталей реализации.
Почему исключительно? Вот сейчас ты опять передергиваешь.. Тот самый "здравый смысл", на который вы так все любите ссылаться подсказывает, что при всем желании от деталей реализации никуда не уползешь. Дизайн библиотеки и ее API естественно строится из соображений здравого смысла, понятий предметной области и прочих других красивых слов. Но, к сожалению, иногда, требования производительности идут в разрез с правильным и удобным дизайном и мы вынуждены помещать метод внутрь класса. (Собсственно, Андрей об этом уже писал). Но, при этом, заметь, правило, мной озвученное, по прежнему работает.
A> Всё же специалист твоего уровня должен бы уметь видеть не только отдельные кирпичи но и стену в целом.
Ну вот я и пытаюсь, поднять вас до своего уровня, чтобы вы увидели стену в целом, а вы уперлись тут.. в кирпичик...
Откапываете всякие частности, и пытаетесь придумать сценарий при котором правило не работает — успехов..
A>Понизить инкапсуляцию внутри библиотеки, но предоставить наружу интерфейс не зависящирй от деталей реализации задача более приоритетная.
В том что "зато пользоваться удобно и абстрактно", будешь убеждать пользователя своей библиотеки, когда она начнет считать не правильно.
С высоты моего уровня, приоритеты несколько другие..
A>Если выясниться что метод можно более эффективно реализовать за счёт обращения к внутренним полям класса, то он прыгнет в класс.
Не можно, а нужно. Должны быть очень веские причины, чтобы при возможности использовать только внешний контракт, открывать доступ к внутреннему состоянию.
A> А когда смотришь на всё это снаружи, не углубляясь в детали реализации, то не наблюдаешь абсолютно никакой системы (что не удивительно), сплошной бардак.
Я правильно понимаю, что из за невозможности вынести один метод за пределы класса из соображений производительности, ты предлагаешь все остальные методы так же пихать в класс и получать один большой бардак?
A> Метод LeftSubstring() может быть в строке, а метод RightSubstring() в хелпере. Потому что правило! Твоя цель — логический бардак снаружи, за счёт только тебе, как автору, видной внутренней красоты?
Не угадал. Моя цель — минимизировать бардак, даже в том случае, если мы вынуждены реализовывать несколько методов не там где хотелось бы.
К тому же ты опять передергиваешь, речь не в абстрактной "внутренней красоте", а в удобстве поддержки, даже на твоем уровне это должно быть очевидно..
A>Хелперы имеют один большой недостаток следующий из размазанности функционала по классам — необходимо помнить (sic!) или постоянно искать в документации названия классов для выполнения операций с данным классом.
<...ужасы поскипаны...>
Рома, не придумывай, на практике это не проблема. Для статических хелперов есть методы расширения, для избавления от этой проблемы, а с IoC и так все понятно. Более того, даже без методов расширения это не было большой проблемой.
A> Юзабилити оно бывает не только в интерфейсах. Тебе может шашечки, рейтинги связности считать, другой абстракционистикой маятся, а народу ехать, сейчас и с комфортом. Рейтинги и правила это, конечно, познавательно, но голову никто не отменял.
Гы. То есть, контраргументов на приведенные формальные метрики, прнципы уменьшения связности и прочие best-practice найти не удалось, значит надо обвинить оппонента в тяге к абстрактной красивости..
Ром, я тоже так умею, давай не начинать? Если есть что по делу сказать — излагай, с удовольствием выслушаю.
A> Они по твоей логике вообще не должны существовать,
Давай я в своей логике сам разберусь? Тебя не затруднит привести цитату, где бы я клеймил Extension Methods?
A> ведь они не могут обращаться к внутренним полям класса, а значит должны быть вынесены в хелпер.
А по твоему класс с Extension Method-ами — не абстрактный хелпер? А что же он тогда?
A> Но нет, придумали специальный синтаксис, расширили язык именно для удобного использования методов.
Для удобного использования абстрактных хелперов, Рома. Именно для них..
A>Как видишь, никаких рейтингов связности.
Как это никаких? Как раз специально, для самых ленивых придумали специальную плюшку, позволяющую выносить методы их класса в абстрактные хелперы. Именно для этого, и ни для чего другого. Это именно тот подход, который я отстаиваю...
Ты бы разобрался в вопросе предварительно, прежде чем клеймить..
A> Открой MSDN и прочитай "Implementers of class libraries should not use extension methods".
Андрюша тебя уже вывел на чистую воду, так что не буду тратить время...
В твоей рич модели есть одна проблема — отслеживание изменения суммы ордера. В примере ты отслеживаешь только факты добавления и удаления позиций. Но сами позиции могут измениться (другой товар, другая цена, другое количество) и сумму ордера надо пересчитывать. Тут в рич моделях обычно начинает работать "тяжелая артилерия" вроде патерна обозреватель или АОП. Но в любом случае код сильно усложняется.
Дальше — хуже. Сумма ордера зависит и от продукта (ставка налога). При изменении ставки налога, надо бы пересчитать суммы по ордерам, да не просто так, а по какому нибудь хитрому правилу (например, с определенной даты).
В общем тут пересчет суммы ордера, вынесенный в отдельный сервисный класс, представляется более предпочтительным.
Так и знал, что будут додумывания, появятся вопросы по отсутствующим классам...
S>В твоей рич модели есть одна проблема — отслеживание изменения суммы ордера. В примере ты отслеживаешь только факты добавления и удаления позиций.
Да.
S>Но сами позиции могут измениться (другой товар, другая цена, другое количество) и сумму ордера надо пересчитывать.
Могут, но мы не допустим: сделаем OrderLine immunable.
S>Тут в рич моделях обычно начинает работать "тяжелая артилерия" вроде патерна обозреватель или АОП. Но в любом случае код сильно усложняется.
События тут явно лишние.
Тут лишние. Можно привести примеры задачи когда child-объектам нужно уведомить о чем нибудь своего парента. Но в этом случае лучше использовать цепочку обязанностей, так как инициаторы события (дети) знают о его получателях (родители).
А АОП совсем не причем. Где ты тут разглядел аспект?
S>Сумма ордера зависит и от продукта (ставка налога). При изменении ставки налога, надо бы пересчитать суммы по ордерам, да не просто так, а по какому нибудь хитрому правилу (например, с определенной даты).
Ой, а как это все получилось? Откуда такое бизнесс требование? Неужели ставка налога меняется каждые 3 минуты, час?
Конечно же нет. Каждая ставка утверждается государством. Изменения происходят не чаще раза год. Ну не чаще раза в месяц. Самое главное, что в момент формирования заказа и в момент его оплаты об изменении этой ставки можно забыть!
S>В общем тут пересчет суммы ордера, вынесенный в отдельный сервисный класс, представляется более предпочтительным.
В явно зверской версии ТЗ -- да. Но в реальной ситуации -- нет.
) класс, у которого все обязанностей -- обновить Total у order.
Каждый по своему и каждый в своем контексте 
Мы уже победили, просто это еще не так заметно...
