CRA>По вашим словам получается что клиет делающий так: CRA>
CRA>void Unknown()
CRA>{
CRA> decimal total = OrderHelper.CalaTotal(order); //получение значение
CRA> order.Total = total; //изменить состояние
CRA>}
CRA>
CRA>Нарушает SRP, согласитесь это бред
Несогласен! Никакой это не бред.
Он действительно нарушает SRP в случае, если изменить состояние order это не единственная его задача. Т.о. если этот код попадает в контроллер UI, например, мы получаем нарушение SRP: следить за Total никак не входит в его обязанность.
Если же это его единственная задача, то возникает вопрос: зачем нам третий ( ) класс, у которого все обязанностей -- обновить Total у order.
P.S. Вспомнился анекдот:
Утро, паковая зона. Работают два мужика: первый копает яму, второй закапывает. Подходит к ним прохожий:
-- Что вы делаете? Зачем один копает яму, а второй закапывает? Это ведь глупо
-- Ничего не глупо. Нас всего трое. Второй должен садить деревья. Он заболел.
Смех смехом, но при увеличении количества классов уменьшается надежность системы.
Здравствуйте, Aikin, Вы писали:
CRA>>По этому получается такая вот загагулина: A>В которой вы упустли несколько вариантов:
A>И это ничем не хужу гуда: A>
A>public class Order
A>{
A> public decimal GetTotal()
A> {
A> return OrderCalculator.GetTotalFor(this);
A> }
A>}
A>
Любой вариант может существовать, кому то нравиться даже так:
public decimal GetTotal(IOrderCalculator calc)
{
return calc.GetTotalFor(this);
}
Проблема не в реализации, а в концепции.
Повторюсь:
Свойство Total может существовать в Order (оно может наже наззывать GetTotal), но фукции аля CalcTotal etc в Order не место.
Пусть даже GetTotal==CalcTotal но концептуально CalcTotal — это явно говорит пользователю о том, что CalcTotal, это спец функция для расчета total, а вот GetTotal пользователю сообщает только, то, что свойство Total ReadOnly
A>P.S. Надоело как-то выкать, может на "ты" перейдем?
ОК
Здравствуйте, Aikin, Вы писали:
A>Стэйт -- внутреннее состояние объекта, день недели -- состояние внешней среды! A>От внешней среды может зависить Стратегия (подсчета чего-то там), но не внутреннее Состояние самого объекта. A>Только из-за того что настал понедельник заказ не перейдет в состояние Заказан, вторник -- Оплачен, среда -- Отгружен, четверг -- Доставлен.
Я тут допустил оплошность, переформулирую требование В определенные дни создания заказа, но должн вести себя по разному
Хотя странно почему ты, считешь что заказ не может перейти в определенное состояние из-за даты, а как же состояние Просрочен
A>>Вы немного сгущаете краски в 2). Он не выполняет операции расчета, он знает кто может ему помочь их выполнить. В этом подходе не вижу ничего плохого CRA>Стоп. CRA>Если в даже в инетрфейста Order есть куча методов Calc*** — это это не гуд, даже если они используют state, strategy, etc.
Боже упаси. С чего вы это взяли? Я первый оторву руки программисту/архитектору предложившему это. В Order есть всего один метод-геттер GetTotal() (get_Total()). Не CalculateTotal(), а именно GetTotal()
CRA>>>А вот во 2-м случае, только ответственность за хранение аттрибутов заказа. A>>Да, про анемичную модель мы слышали, но не прониклись Боюсь Order это не тот случай когда она хороша. CRA>На вкус и цвет... CRA>Но посмотрите что может случиться:
CRA>Вот такая может получить Rich model и в конечном итоге придется все Calc*** методы куда-нить переносить
Не, это не рич модель, а страшний сон архитектора.
Рич модель будет такая: CRA>
CRA>public class Order
CRA>{
CRA> public IList<OrederLine> OrderLines {get;set;} // У меня есть вопросы по публичной коллекции, но для простоты пусть будет так
CRA> public decimal Total{get;}
CRA>}
CRA>
CRA>переформулирую требование В определенные дни создания заказа, но должн вести себя по разному
Таки Cтратегия
CRA>Хотя странно почему ты, считешь что заказ не может перейти в определенное состояние из-за даты, а как же состояние Просрочен
Да, тут есть доля правды. Но дело в том, что не сам объект должен перевевести себя в состояние Просрочен, а внешняя к нему функциональность.
...Хм, хотя в состояние Невалиден он может перевести себя сам... Но, с другой стороны, перевод этот осуществляется после вызова Validate()...
Скользко, очень скользко. Хорошо хоть у нас не эта задача сейчас стоит, а вычисление стоимости.
CRA>Любой вариант может существовать, кому то нравиться даже так: CRA>Свойство Total может существовать в Order (оно может наже наззывать GetTotal), но фукции аля CalcTotal etc в Order не место. CRA>Пусть даже GetTotal==CalcTotal но концептуально CalcTotal — это явно говорит пользователю о том, что CalcTotal, это спец функция для расчета total, а вот GetTotal пользователю сообщает только, то, что свойство Total ReadOnly
Полностью с этим согласен и даже отстаивал эту позицию, но спор начался из-за того что не сошлись во мнении куда помещать метод GetTotal(): в сам класс или в Хелпер
CRA>>Вы опять пытаетесь за меня написать метод UpdateTotal.
AVK>Я тебе попытаюсь пояснить ситуацию в последний раз. В контексте топика речь шла именно о методе вычисления, нигде в исходном примере никаких изменений не было. Глядя на твое решение, я логично предположил, что, если у тебя нигде собственно вычислений не заявленно, значит они внутри Update. Если же у тебя там целая инфраструктура — то весь твой пример идет в лес и обсуждать я его не хочу, потому что на обсуждаемый здесь вопрос он никак не отвечает, самое главное осталось за кадром.
Да, метод вычисления находится в Update, как это меняет ситуацию?
Предвосхищая ваши заявления, SRP для каждого метода это бред, так как вы перестали поддерживать дискуссию по этому вопросу, думаю что вы со мной согласны.
. Я до сих пор речь веду о нем. Естественно, твои умопостроения, которые ты по ходу разговора додумываешь, я не учитывал. Но твоя позиция абсолютно неконструктивна — всегда ведь можно сказать, на любое утверждение, что оно неверно, потому что у нас за кадром целое стадо коров спрятано. А конструктивно — это когда говорится что, а, главное, почему надо делать.
Видимо пытаясь бороться с методом UpdateTotal вы совершенно забыли мой пример, я специально для вас его повторю (изменив метод Update на Calc)
public class Order
{
public IList<OrederLine> OrderLines {get;set;}
public decimal Total{get;set;}
}
public class OrderTotalCalculator
{
public decimal CalcTotal(Order order){}
}
AVK>То есть ты признаешь, что ты написал сообение, которое заведомо не проясняет ситуацию с обсуждаемым здесь вопросом, так? Тогда зачем ты его написал?
Посмотрите на мой пост от 23.07.08 17:39 (С него и началось наше с Вами обсуждение), я в нем предлагаю способ и полностью его обосновываю, то что вы предлажили назвать метод не UpdateTotal, а CalcTotal ситуации не меняет, а сейчас вы пытаетесь вообще мое решение засудить.
AVK>давай тогда и я додумаю, а то несправедливо получается: исходная задача не твоя и не моя, ты додумываешь, а мне нельзя. А вдруг унутре Update так: AVK>
AVK>var total = order.GetTotal();
AVK>order.Total = total;
AVK>
AVK> AVK>Поэтому давай не будем тут сочинять по ходу разговора, а просто демонстрировать максимально ясно и понятно ту мысль, которую хочется продемонстрировать, и тогда все у нас получится.
В моем примере я имел ввиду следующее
Что то вы перегибаете палку.
Вы хотите что бы я полностью написал метод UpdateTotal, но это бред.
Обговорю^ в UpdateTotal ведется расчет Total и оно записывается в Order.
Здравствуйте, C...R...a...S...H, Вы писали:
CRA>Да, метод вычисления находится в Update, как это меняет ситуацию?
Возвращаемся к началу топика и читаем по новой, на этот вопрос я уже отвечал.
CRA>Предвосхищая ваши заявления, SRP для каждого метода это бред
Я тоже так умею. Предвосхищая твои заявления — не применять SRP для методов это бред.
CRA>Вы хотите что бы я полностью написал метод UpdateTotal, но это бред.
Нет, я хочу чтобы в твоем примере не было вещей, важных для обсуждаемого вопроса, которые по ходу обсуждения начинают всплывать.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
А мне кажется вы оба не правы. Если подсчет Total выносить из класса Order, то в нем не должно остаться ни Total, ни CalcTotal ни других упоминаний о том, что у него есть Total. Это уже ответственность OrderTotalCalculator.
Здравствуйте, IB, Вы писали:
IB>Это не "прыжек в сторону". Андрюша уже писал об этом, как, впрочем, и я. Принцип сформулирован очень четко. Если метод использует только публичный контракт для работы с классом, значит он должен находиться вне класса.
Никакое усердно использование правила не заменит усердное использование мозга. Правило — это всего лишь правило и его повсеместное необдуманное применение принесёт больше вреда чем пользы, как и использование любого другого правила. Об чём я ниже и напишу.
IB>Уже использует, понимаешь? Это данность, его так задизайнили, не важно по каким причинам. Если по каким либо причинам методу нужно использовать приватные данные, не важно по каким, дизайн ли, производительность ли, еще что-то, то его место внутри. IB>Все очень четко, конкретно, понятно и прозрачно. В данном случае части методов, из соображений производительности, нужно иметь доступ к внутреннему состоянию, значит их место внутри класса.
Ага, теперь давай рассмотрим последствия. У тебя внешний интерфейс библиотеки — контракт для общения с другими компонентами, построен не на основе понятий предметной области, здравого смысла, и т.п., а на основе исключительно деталей реализации. Причём это поведение, о чудо, вызванно побуждениями к повешению инкапсуляции! Всё же специалист твоего уровня должен бы уметь видеть не только отдельные кирпичи но и стену в целом. Понизить инкапсуляцию внутри библиотеки, но предоставить наружу интерфейс не зависящирй от деталей реализации задача более приоритетная.
То есть у тебя если вдруг станет ясно, что метод можно реализовать только за счёт публичного контракта, то он прыгнет в хелпер. Если выясниться что метод можно более эффективно реализовать за счёт обращения к внутренним полям класса, то он прыгнет в класс. А когда смотришь на всё это снаружи, не углубляясь в детали реализации, то не наблюдаешь абсолютно никакой системы (что не удивительно), сплошной бардак. Метод LeftSubstring() может быть в строке, а метод RightSubstring() в хелпере. Потому что правило! Твоя цель — логический бардак снаружи, за счёт только тебе, как автору, видной внутренней красоты?
IB>С твоей точки зрения, общая помойка в одном классе лучше? Как раз классический пример "плохой кохессии" в терминологии stump-а. Не знаю, я по помойкам не специалист, я больше аккуратненькие клумбочки сервисов выращиваю..
Ты не специалист? Не верю! Неужели никогда на Си++ не писал? Там этот принцип доведён до точки, STL называется. Данные отдельно, алгоритмы-манипуляторы-хелперы отдельно.
Что мы имеем? Вот я хочу найти элемент в сортированном массиве. Ищу find, search, index_of и естественно ничего не нахожу. Есть binary_search, но он проверят наличие элемента не возвращая его индекса. Автор библиотеки (привет тебе автор!) называл функцию lower_bound. Слабо угадать такое название? Оно абсолютно никак не связано с классом vector.
Хелперы имеют один большой недостаток следующий из размазанности функционала по классам — необходимо помнить (sic!) или постоянно искать в документации названия классов для выполнения операций с данным классом. Даже при всём уважении к MSDN, не надо лукавить. Необходимо помнить, искать бесполезно. Не надо впаривать что это будут классы StringCase и типа набери string, ctrl+space и будет счастье. Не будет, у нас нет StringCase, есть CultureInfo. Это что, можно угадать или быстро найти, что для перевода строки в верхний регистр надо использовать класс CultureInfo?!
Интерфейс подобной библиотеки становиться полным кошмаром, его надо держать постоянно в голове, помня семантические связи между классами. Почему ToUpper не просто внесли в string, а продублировали там? Подублировали, функция уже была в хелпере! Да потому что не на правила полагались, а хорошенько подумали головой и не об абстрактной крутости подсчитаной нарошной утилиткой, а о людях, живых людях, которым надо будет каждый день пользоваться классом String и которые не хотят что-то помнить и лазать по документации, а хотят нажать точку и увидеть список того, что можно с этим стрингом произвести. Юзабилити оно бывает не только в интерфейсах. Тебе может шашечки, рейтинги связности считать, другой абстракционистикой маятся, а народу ехать, сейчас и с комфортом. Рейтинги и правила это, конечно, познавательно, но голову никто не отменял.
Более того, пока всё прогрессивное сообщество развивается в одну сторону, ты умудряешься развиваться в прямо противоположную. Зачем по твоему придумали extension methods? Они по твоей логике вообще не должны существовать, ведь они не могут обращаться к внутренним полям класса, а значит должны быть вынесены в хелпер. Но нет, придумали специальный синтаксис, расширили язык именно для удобного использования методов. Чтобы нажать точку и выплыли методы. Вот на таком примитивном уровне мотивации всё и зиждется. Как видишь, никаких рейтингов связности.
Казалось бы вот оно счастье, писать хелперы как extension methods, но суть как раз в другом. Открой MSDN и прочитай "Implementers of class libraries should not use extension methods". Суть в том, чтобы иметь возможность расширять классы от которых нельзя наследоваться и иметь возможность удобно использовать эти расширения. Ты не можешь вставить класс в иерархии между IEnumerable<T> и List<T>, не можешь добавить метод в string. Не можешь, но хочешь. И вот тебе, пожалуйста extension methods. Удобство использования ставиться во главу. Linq от которого все тащаться это же чистое удобство. Всё что там есть замечательно реализуется и без extension methods, но кто этим будет пользоваться в таком виде?
Здравствуйте, stump, Вы писали:
S>Хочется, конечно подиспутировать по существу, но совершенно надоело терпеть твое хамство.
У тебя восхитительная особенность, при недостатке аргументов обвинять оппонента в некорректности ведения дискуссии. И ты меня после этого в хамстве обвиняешь? Ты вообще внимательно читал, что сам написал?
Здравствуйте, adontz, Вы писали:
A>Что мы имеем? Вот я хочу найти элемент в сортированном массиве. Ищу find, search, index_of и естественно ничего не нахожу. Есть binary_search, но он проверят наличие элемента не возвращая его индекса. Автор библиотеки (привет тебе автор!) называл функцию lower_bound. Слабо угадать такое название? Оно абсолютно никак не связано с классом vector.
A>Хелперы имеют один большой недостаток следующий из размазанности функционала по классам — необходимо помнить (sic!) или постоянно искать в документации названия классов для выполнения операций с данным классом. Даже при всём уважении к MSDN, не надо лукавить. Необходимо помнить, искать бесполезно. Не надо впаривать что это будут классы StringCase и типа набери string, ctrl+space и будет счастье. Не будет, у нас нет StringCase, есть CultureInfo. Это что, можно угадать или быстро найти, что для перевода строки в верхний регистр надо использовать класс CultureInfo?!
Рома, с этой ужасной проблемой прекрасно справляются extension методы.
A> Зачем по твоему придумали extension methods? Они по твоей логике вообще не должны существовать
Ты, видимо, читал что то свое. Я не припоминаю чтбы Иван говорил что они не нужны. Как мне помнится, он говорил обратное. Так что неясно, с кем ты споришь.
A> Открой MSDN и прочитай "Implementers of class libraries should not use extension methods".
Этот приём называется "художественная резьба по цитатам". Вот полная цитата:
Implementers of class libraries should not use extension methods to avoid creating new versions of assemblies.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, AndrewVK, Вы писали:
A>> Открой MSDN и прочитай "Implementers of class libraries should not use extension methods". AVK>Этот приём называется "художественная резьба по цитатам". Вот полная цитата: AVK>
Implementers of class libraries should not use extension methods to avoid creating new versions of assemblies.
И в чём принципиально поменялся смысл? Extension methods в библиотеках запрещены и служат лишь для удобного расширения чужих библиотек. Свои собственные библиотекии не расшииряют через extension methods. Ты не прыгай несущественной части сообщения, выдёргивая самое, на твой взгляд, сомнительное, ты отвечай на сущесвенную часть. Я тебе подскажу, существенная часть, это например: "Метод LeftSubstring() может быть в строке, а метод RightSubstring() в хелпере".
Здравствуйте, adontz, Вы писали:
AVK>>Этот приём называется "художественная резьба по цитатам". Вот полная цитата: AVK>>
Implementers of class libraries should not use extension methods to avoid creating new versions of assemblies.
A>И в чём принципиально поменялся смысл?
В том, что следует не всегда избегать использования extension методов в библиотеке, а с одной, вполне конкретной целью.
A> Extension methods в библиотеках запрещены
В цитате этого нет. В цитате сказано что следует избегать использования extension методов с целью не создавать новую версию сборки. С другими целями использовать можно.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, Ziaw, Вы писали:
Z>Здравствуйте, Aikin, Вы писали:
Z>А мне кажется вы оба не правы. Если подсчет Total выносить из класса Order, то в нем не должно остаться ни Total, ни CalcTotal ни других упоминаний о том, что у него есть Total. Это уже ответственность OrderTotalCalculator.
Нет так не пойдет. В первоначальном моем примере был только код, демонстрирующий два подхода к размещению методов с логикой класса, и ничего о предметной области. Но, как я вижу примерчик оказался необычайно жизненным
С точки зрения предметной области в нем был один существенный изъян. Если посмотреть на формулу расчета суммы ордера, то мы заметим что там присутствует ставка налога из класса Product. Представте, что у нас есть тысяча ордеров (наверное часть из них оплачена), и в один прекрасный момент мы изменяем ставку налога в Product (Дума так решила...). Если после этого мы начнем сверять суммы по ордерам с суммами по платежам, то обнаружим что суммы теперь "не бьют". Зато за дверями стоят юзера намереньем побить уже нас.
Поэтому, с точки зрения здравого смысла (который у некоторых не в почете) сумму ордера конечно же надо хранить.
Ага это сразу заметил , но тогда я уклонился от дискусии, потому что она уводила в сторону от первоначальной цели. Но дискуссия таки все равно поперла.
С другой строны в первоначальной постановке ничего не говорится о том, должен ли меняться способ расчета суммы или нет.
Давайте рассмотрим два крайних случая:
1. Способ расчета никогда не будет менятся, потому что мы пишем утилиту для разовой конвертации данных.
2. Может быть несколько способов расчета суммы, они могут меняться в течении жизненного цикла прогаммы (например завтра будет програссивная шкала налога).
Как это может повлиять на дизайн? Или никак не повлияет, руководствуемся "четким принципом" и все.
Большой +
Все эти аргументы и многие другие уже приводились здесь. Ты действительно хорошо пишешь, но сомневаюсь, что тебе удастся в чем то убедить Ивана и Андрея.
Здравствуйте, adontz, Вы писали:
A>Начиная с "However, many of the features that make extension methods so useful for library consumers can be problematic for class library authors".
In particular, there are 2 primary issues ...
... if an instance member is ever added to a type with the same name as an extension method, then the extension method can be rendered uncallable.
Во-первых это вранье, потому что остается возможность вызова в классическом виде. Во-вторых единственный способ добавить к типам готовой библиотеки метод экземпляря — сделать наследника. Однако, если прокастить наследника к базовому типу — extension метод можно звать и нужным способом. В-третьих если пользюк называет методы наследников библиотеки так же, как существующий метод в той же библиотеке, то он сам себе злобный буратин.
The second issue is that an extension method author cannot prevent other programmers from writing conflicting extensions.
А это вообще специально сделано, чтобы как раз таки я мог подменять один функционал другим на уровне статического полиморфизма. Например, заменять лямбды на expression tree.
Вобщем, слабенькая статейка, неубедительно.
A>В общем, в библиотеках от extension methods проблем больше, чем пользы.
Расскажи это МС, которые, вот идиоты, класс Enumerable в библиотеку воткнули.
... << RSDN@Home 1.2.0 alpha 4 rev. 1095 on Windows Vista 6.0.6001.65536>>
Здравствуйте, AndrewVK, Вы писали:
AVK>Возвращаемся к началу топика и читаем по новой, на этот вопрос я уже отвечал. AVK>Я тоже так умею. Предвосхищая твои заявления — не применять SRP для методов это бред. AVK>Нет, я хочу чтобы в твоем примере не было вещей, важных для обсуждаемого вопроса, которые по ходу обсуждения начинают всплывать.
Признаюсь, вам все же удалось меня провести, а я стормозил из-за того что поторопился.
Смотрим внимательно:
Ваш комментарий к моему решению:
Первый пост:
CRA> public decimal UpdateTotal(Order order){}
Лучше все же CalcTotal,а не UpdateTotal.
Второй пост:
Вот как раз я и хотел обратить внимание на то, что лучше бы этот метод ничего сам не менял, потому что это более универсальное решение. К примеру, сумму может быть нужно рассчитать предварительно, без изменения персистентного класса.
А теперь вскрываем карты
После этих постов, что мол лучше функцию обозвать CalcTotal, а не UpdateTotal и заявлений про SRP для методов (которые Вы потом прекратили, из-за того, что это полный бред. И скрытое изменение состояния, которое я так же опроверг)
Вы выдаете вот такое сообщение:
В контексте топика речь шла именно о методе вычисления, нигде в исходном примере никаких изменений не было. Глядя на твое решение, я логично предположил, что, если у тебя нигде собственно вычислений не заявленно, значит они внутри Update. Если же у тебя там целая инфраструктура — то весь твой пример идет в лес и обсуждать я его не хочу, потому что на обсуждаемый здесь вопрос он никак не отвечает, самое главное осталось за кадром.
Получается интересная ситуация, либо Вы все забыли пока обсуждали переименование метода UpdateTotal в CalcTotal.
Либо Ваша любимая методика подмены понятий дает о себе знать.
Третьего уж простите не дано
) класс, у которого все обязанностей -- обновить Total у order.
Мы уже победили, просто это еще не так заметно...
