Re[15]: своё vs. сторонее
От: Ikemefula Беларусь http://blogs.rsdn.org/ikemefula
Дата: 25.10.13 10:01
Оценка:
Здравствуйте, ., Вы писали:

.>Что-то мне кажется, что все эти нападки направлены на что угодно, но не DI. DI это очень тривиальный и маленький паттерн, альтернативы ему — сложнее или кривее.

.>В общем вот почитай: http://code.google.com/p/google-guice/wiki/Motivation и выскажи что такого неправильного в предложенном решении c DI и как бы ты сам решал такую задачу.

Пример как то так себе, одна мешанина непойми чего заменилась на другую мешанину непойми чего, но на DI
Re[16]: своё vs. сторонее
От: AndrewJD США  
Дата: 25.10.13 10:20
Оценка:
Здравствуйте, Ikemefula, Вы писали:

.>>В общем вот почитай: http://code.google.com/p/google-guice/wiki/Motivation и выскажи что такого неправильного в предложенном решении c DI и как бы ты сам решал такую задачу.


I>Пример как то так себе, одна мешанина непойми чего заменилась на другую мешанину непойми чего, но на DI

А можно пример того как надо сделать? Т.е. задача описана по ссылке и есть несколько решений. Расскажите, как бы вы ее задизайнили?
"For every complex problem, there is a solution that is simple, neat,
and wrong."
Re[17]: своё vs. сторонее
От: Ikemefula Беларусь http://blogs.rsdn.org/ikemefula
Дата: 25.10.13 10:36
Оценка:
Здравствуйте, AndrewJD, Вы писали:

I>>Пример как то так себе, одна мешанина непойми чего заменилась на другую мешанину непойми чего, но на DI

AJD>А можно пример того как надо сделать? Т.е. задача описана по ссылке и есть несколько решений. Расскажите, как бы вы ее задизайнили?

Прежде всего я бы постарался избавиться от самого BillingService. В том виде, как он подан, вообще неочевидно, что он нужен как отдельный класс.
Вообще рассуждать про DI на примере одного метода с линейной логикой как то странно. Пример ничем не лучше уже показаного сделать суммирующего сервиса
Нужно как минимум смотреть что представляет из себя вызывающий код.
Re[16]: своё vs. сторонее
От: . Великобритания  
Дата: 25.10.13 10:43
Оценка:
Здравствуйте, IT, Вы писали:

.>>Давай вот тут: http://code.google.com/p/google-guice/wiki/Motivation как ты переделаешь "по правильному, без DI"?

IT>Вот! Молодец! Более тупейшего применения DI я в жизни не видел (хотя такое же видел не раз). Как ты думаешь, что тестирует этот якобы "тест"? Я тебе отвечу. А кроме эмоций есть что сказать?
Покажи класс.

IT>Он тестирует умение компилятора вызывать виртуальный метод. И больше НИЧЕГО.

Что мелочиться-то? Пиши сразу: "тестирует умение компьютера выполнять код. И больше НИЧЕГО".

IT>Вся бизнес логика, которую и нужно в конце концов тестировать тупо подменена фэйк-методами. Я фигею с вас, ребята

PaypalCreditCardProcessor скажем тупо посылает http запрос и читает результат. DatabaseTransactionLog делает insert в БД. Это что-ли бизнес-логика? Предлагаешь jdk http client потестировать или протокол http?
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Re[18]: своё vs. сторонее
От: . Великобритания  
Дата: 25.10.13 11:16
Оценка:
Здравствуйте, Ikemefula, Вы писали:

I>>>Пример как то так себе, одна мешанина непойми чего заменилась на другую мешанину непойми чего, но на DI

AJD>>А можно пример того как надо сделать? Т.е. задача описана по ссылке и есть несколько решений. Расскажите, как бы вы ее задизайнили?
I>Прежде всего я бы постарался избавиться от самого BillingService. В том виде, как он подан, вообще неочевидно, что он нужен как отдельный класс.
I>Вообще рассуждать про DI на примере одного метода с линейной логикой как то странно. Пример ничем не лучше уже показаного сделать суммирующего сервиса
I>Нужно как минимум смотреть что представляет из себя вызывающий код.
Это же минимальный пример. Не обращай внимание на маленькое количество кода. Представь что в этом сервисе есть ещё несколько методов, например "отмена заказа" и т.п., логика помещения заказа может быть сложнее, например, посылать email уведомления, проверять доступность скидок и т.п. Т.е. сервис нам нужен.
И обрати внимание что этот класс соответствует бизнес-сущности "биллинг". Можно, конечно, сказать что нафиг это всё и писать всё в main-методе или onclick_button, но вроде это не самая лучшая практика.
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Re[19]: своё vs. сторонее
От: Ikemefula Беларусь http://blogs.rsdn.org/ikemefula
Дата: 25.10.13 12:34
Оценка:
Здравствуйте, ., Вы писали:

AJD>>>А можно пример того как надо сделать? Т.е. задача описана по ссылке и есть несколько решений. Расскажите, как бы вы ее задизайнили?

I>>Прежде всего я бы постарался избавиться от самого BillingService. В том виде, как он подан, вообще неочевидно, что он нужен как отдельный класс.
I>>Вообще рассуждать про DI на примере одного метода с линейной логикой как то странно. Пример ничем не лучше уже показаного сделать суммирующего сервиса
I>>Нужно как минимум смотреть что представляет из себя вызывающий код.
.>Это же минимальный пример. Не обращай внимание на маленькое количество кода. Представь что в этом сервисе есть ещё несколько методов, например "отмена заказа" и т.п., логика помещения заказа может быть сложнее, например, посылать email уведомления, проверять доступность скидок и т.п. Т.е. сервис нам нужен.

С таким подходом все сводится к "предположим, DI нужен, следовательно, DI нужен"

.>И обрати внимание что этот класс соответствует бизнес-сущности "биллинг". Можно, конечно, сказать что нафиг это всё и писать всё в main-методе или onclick_button, но вроде это не самая лучшая практика.


Это неважно, чем он соответсвует. Важно как эта сущность используется с другими сущностями, т.е. как строится вызывающий код.
Re[12]: своё vs. сторонее
От: Baudolino  
Дата: 25.10.13 14:30
Оценка:
Здравствуйте, IT, Вы писали:

IT>Здравствуйте, Baudolino, Вы писали:


B>>Вы тесты в своей жизни когда-нибудь писали?

IT>Ты что-то хотел сказать?
Я задал вопрос. Ответ будет?
Re[14]: своё vs. сторонее
От: Baudolino  
Дата: 25.10.13 14:32
Оценка: :)
Здравствуйте, IT, Вы писали:

IT>Здравствуйте, Baudolino, Вы писали:


IT>>>Я уже устал об этом говорить. DI ломает навигацию по проекту

B>>В 2003 году? В 2013 не ломает, если вы используете правильные инструменты (при программировании в блокноте, согласен, навигация никакая, даже без DI).
IT>Сколько пафоса! Можно подумать ты пишешь программы с помощью мысленного инструмента, а не с помощью клавиатуры.
Интересно, каким образом здесь появилось слово "мысленный" и какое отношение к навигации по проекту имеет клавиатура?

IT>Если очень хочется пообсуждать криворукость архитектуры и архитекторов, то достаточно показать здесь свой наиправильнейший код. Или слабо?

Это аргумент из серии "сперва добейся"? Ну-ну. Я связан NDA с нынешним и бывшими своими работодателями. Хочешь посмотреть мой код — устройся для начала ко мне на работу.

IT>>>А всегда ли это нужно?

B>>В проекте объемом больше пары десятков тысяч строк — да, уже имеет смысл. На всяких мелких — не обязательно, но по рельсам паровоз обычно едет лучше, чем по говну.
IT>Ничего не понял Какой паровоз...
Это метафора, намекающая на то, что в небольших проектах DI тоже может быть полезным, позволяя сфокусироваться на бизнес-логике и не тратить время на инфраструктуру.
Re[15]: своё vs. сторонее
От: Abalak США  
Дата: 25.10.13 15:03
Оценка:
Здравствуйте, ., Вы писали:

.>Допустим, что было спроектировано криво, костылесовместимость решений от M$ всегда высока, поубивав бы. Но тебе понадобился лишь элементарный рефакторинг с минимальным импактом, чтобы это поправить. Если бы не было явного DI, то скорее всего была бы мешанина ещё более кривых архитектурных решений, такие как рассыпанные по всему коду класса операторы new, глобальные синглтоны, статические фабрики, магические контексты и прочий шлак, рефакторинг которого довльно сложная задача со слабо предсказуемыми последствиями.


Вот далеко не факт. Об этом сложно судить, но оправдывать существование одного говнокода вероятностью написания другого как минимум странно

.>Главное не избегать заранее кривое проектирование (это утопия), а минимизировать последствия неудачного проектирования. Судя по тому, что ты написал, DI в этом помогло отлично.


Нет, смотри, сущность объекта я описал — лезть в базу и исполнять код, который лежит тоже в базе и вернуть данные. Он не должен вообще не от чего зависить, его нужно создать и использовать, не заботясь в каком контексте он выполняется. DI тут ничем не помог, ну никак.

.>Я не знаю как там в C# мире, но в Яве чтобы минимально заюзать DI нужно добавить ~500k либу и написать пол дюжины строк кода. И объём этого кода будет явно меньше, чем если бы конфигурировал эти все вызовы вручную.


Ну в туториалах так же, а вот в реальном мире получается редкостная колбаса, подозреваю, что ява недалеко ушла.

.>Понимаю, можно что угодно нагородить где угодно, но DI в этом скорее будет мешать.

.>Не понимаю из твоего рассказа почему ты решил, что эта проблема вызвана DI.

Потому что, его туда притащили не понятно зачем и завязали объект на совсем независимые от него сущности.

.>Что-то мне кажется, что все эти нападки направлены на что угодно, но не DI. DI это очень тривиальный и маленький паттерн, альтернативы ему — сложнее или кривее.


Если бы так было в реале, я бы ничего не говорил. Но DI фреймворки это огромные монстры и шанс наступить на грабли стремится к единице. Если бы все писали иделаьный код, то и проблем бы не возникало, но DI дает в руки программистам отличный инструмент по наступлению на грабли. Усложнение там, где оно не нужно.

.>В общем вот почитай: http://code.google.com/p/google-guice/wiki/Motivation и выскажи что такого неправильного в предложенном решении c DI и как бы ты сам решал такую задачу.


Я бы решил на интерфейсах. Но опять же, это то, о чем я говорил выше — отличие теории от практики.
Re[20]: своё vs. сторонее
От: . Великобритания  
Дата: 25.10.13 15:16
Оценка:
Здравствуйте, Ikemefula, Вы писали:

I>Здравствуйте, ., Вы писали:


AJD>>>>А можно пример того как надо сделать? Т.е. задача описана по ссылке и есть несколько решений. Расскажите, как бы вы ее задизайнили?

I>>>Прежде всего я бы постарался избавиться от самого BillingService. В том виде, как он подан, вообще неочевидно, что он нужен как отдельный класс.
I>>>Вообще рассуждать про DI на примере одного метода с линейной логикой как то странно. Пример ничем не лучше уже показаного сделать суммирующего сервиса
I>>>Нужно как минимум смотреть что представляет из себя вызывающий код.
.>>Это же минимальный пример. Не обращай внимание на маленькое количество кода. Представь что в этом сервисе есть ещё несколько методов, например "отмена заказа" и т.п., логика помещения заказа может быть сложнее, например, посылать email уведомления, проверять доступность скидок и т.п. Т.е. сервис нам нужен.
I>С таким подходом все сводится к "предположим, DI нужен, следовательно, DI нужен"
Где это у меня было DI? Это ты всё в каждом предложении упоминаешь. Я лишь сказал, что нужен некий BillingService.

.>>И обрати внимание что этот класс соответствует бизнес-сущности "биллинг". Можно, конечно, сказать что нафиг это всё и писать всё в main-методе или onclick_button, но вроде это не самая лучшая практика.

I>Это неважно, чем он соответсвует. Важно как эта сущность используется с другими сущностями, т.е. как строится вызывающий код.
Да какая разница? Допустим он вызывается из servlet.onPost, парся всякие пост-параметры заказа. И, например, обрабатывает заказы из входящих емейлов/смскок.
Предложи свое видение.
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Re[16]: своё vs. сторонее
От: . Великобритания  
Дата: 25.10.13 15:32
Оценка:
Здравствуйте, Abalak, Вы писали:

A>Здравствуйте, ., Вы писали:


.>>Допустим, что было спроектировано криво, костылесовместимость решений от M$ всегда высока, поубивав бы. Но тебе понадобился лишь элементарный рефакторинг с минимальным импактом, чтобы это поправить. Если бы не было явного DI, то скорее всего была бы мешанина ещё более кривых архитектурных решений, такие как рассыпанные по всему коду класса операторы new, глобальные синглтоны, статические фабрики, магические контексты и прочий шлак, рефакторинг которого довльно сложная задача со слабо предсказуемыми последствиями.

A>Вот далеко не факт. Об этом сложно судить, но оправдывать существование одного говнокода вероятностью написания другого как минимум странно
Может не надо философстовать? А то тогда уж — весь код говно. И что же делать? Остаётся лишь надеется, что с каким-то говном проще возиться...

.>>Главное не избегать заранее кривое проектирование (это утопия), а минимизировать последствия неудачного проектирования. Судя по тому, что ты написал, DI в этом помогло отлично.

A>Нет, смотри, сущность объекта я описал — лезть в базу и исполнять код, который лежит тоже в базе и вернуть данные. Он не должен вообще не от чего зависить, его нужно создать и использовать, не заботясь в каком контексте он выполняется. DI тут ничем не помог, ну никак.
База — уже зависимость. А значит уже забота. Понимаю, что можно всё захардкодить... но нужно ли?

.>>Я не знаю как там в C# мире, но в Яве чтобы минимально заюзать DI нужно добавить ~500k либу и написать пол дюжины строк кода. И объём этого кода будет явно меньше, чем если бы конфигурировал эти все вызовы вручную.

A>Ну в туториалах так же, а вот в реальном мире получается редкостная колбаса, подозреваю, что ява недалеко ушла.
IoC-контейнеры в .net пришли из Явы...

.>>Понимаю, можно что угодно нагородить где угодно, но DI в этом скорее будет мешать.

.>>Не понимаю из твоего рассказа почему ты решил, что эта проблема вызвана DI.
A>Потому что, его туда притащили не понятно зачем и завязали объект на совсем независимые от него сущности.
Что значит "завязали"? И как его отвязать? Захардкодить new DependencyImpl в объект?

.>>Что-то мне кажется, что все эти нападки направлены на что угодно, но не DI. DI это очень тривиальный и маленький паттерн, альтернативы ему — сложнее или кривее.

A>Если бы так было в реале, я бы ничего не говорил. Но DI фреймворки это огромные монстры и шанс наступить на грабли стремится к единице. Если бы все писали иделаьный код, то и проблем бы не возникало, но DI дает в руки программистам отличный инструмент по наступлению на грабли. Усложнение там, где оно не нужно.
Наступают на грабли все, и со всеми фреймворками. Цель хорошего программиста — сделать это как можно менее болезненным. DI в этом помогает.

.>>В общем вот почитай: http://code.google.com/p/google-guice/wiki/Motivation и выскажи что такого неправильного в предложенном решении c DI и как бы ты сам решал такую задачу.

A>Я бы решил на интерфейсах. Но опять же, это то, о чем я говорил выше — отличие теории от практики.
Что значит "на интерфейсах"? Там как раз интерфейсы почти везде. Давай, показывай свою практику, хватит теоретизировать.
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Re[21]: своё vs. сторонее
От: Ikemefula Беларусь http://blogs.rsdn.org/ikemefula
Дата: 25.10.13 15:47
Оценка:
Здравствуйте, ., Вы писали:

.>>>Это же минимальный пример. Не обращай внимание на маленькое количество кода. Представь что в этом сервисе есть ещё несколько методов, например "отмена заказа" и т.п., логика помещения заказа может быть сложнее, например, посылать email уведомления, проверять доступность скидок и т.п. Т.е. сервис нам нужен.

I>>С таким подходом все сводится к "предположим, DI нужен, следовательно, DI нужен"
.>Где это у меня было DI? Это ты всё в каждом предложении упоминаешь. Я лишь сказал, что нужен некий BillingService.

Для BillingService не нужен никакой DI, максимум, что можно показать на ём, это Inversion of Control, т.е. вынести все лишние депенденсы куда нибудь наверх. Больше ничего на этом сервисе показать не удастся.
Потому с DI остаётся всё, как было сказано: "предположим, DI нужен, следовательно, DI нужен"

I>>Это неважно, чем он соответсвует. Важно как эта сущность используется с другими сущностями, т.е. как строится вызывающий код.

.>Да какая разница? Допустим он вызывается из servlet.onPost, парся всякие пост-параметры заказа. И, например, обрабатывает заказы из входящих емейлов/смскок.
.>Предложи свое видение.

Нужно смотреть вызывающий модуль, что в ём будет.
Re[17]: своё vs. сторонее
От: maxkar  
Дата: 25.10.13 16:00
Оценка:
Здравствуйте, AndrewJD, Вы писали:

I>>Пример как то так себе, одна мешанина непойми чего заменилась на другую мешанину непойми чего, но на DI

AJD>А можно пример того как надо сделать? Т.е. задача описана по ссылке и есть несколько решений. Расскажите, как бы вы ее задизайнили?

Задача отвратительно решена. И поставлена тоже.

Начнем с теста. Он бесполезен. Конкретно его функциональность проверяется в момент интеграционного тестирования. Мало ли какой-там CreditCardProcessor затесался в сборку. Может, там Fake лежит. А интеграционное тестирование это все покажет. Причем этот сценарий "все заказывается и работает" проверяться будет обязательно — это основной пользовательский сценарий. Ну а пицца — команде в качестве бонуса за релиз.

Далее. Тест не тестирует контракт! Тест каким-то сложным и запутанным способом проверяет, что именно написано в методе. Повторяю — не что метод делает, а что именно написано! Ну вот creditCardProcessor.getCardOfOnlyCharge() есть, для примера. А что, если потом будет сделана не одна, а несколько charge? Или сначала резервирование средств на счету, а только потом списание? Ведь "в реальной жизни" в этом creditCardProcessor куча разных методов. И некоторые вещи на самом деле можно реализовать несколькими способами. Ту же продажу, например. Так что корректно замокать этого монстрика — то еще приключение. Можно внедрить ревью кода для проверки подобных реализаций, это будет гораздо проще.

Еще стоит вспомнить, что для данного метода нужно бы какую-нибудь транзакционность проверять.

Сама сигнатура метода странная. Зачем там order, если используется только amount? И самая интересная вещь в виде Receipt не инжектится. Это вообще должна быть generic service, параметризованная типом Receipt.

Решение использовать Guice — тоже странное. Автор сам себя ведь выше опроверг. В пункте factory. Вы думаете, что Guice — делает что-то похожее на "dependency injection"? Нет. Это service locator в практически чистом виде, что ближе к factory. Смотрите, легкое движение руками и пример с инжектором превращается в
  public RealBillingService(Injector injector) {
    this.processor = injector.lookup(CreditCardProcessor.class);
    this.transactionLog = injector.lookup(TransactionLog.class, "realBillingTransactionLog");
  }

Второе поле — это эквивалент @Named. Я вот не понимаю, в чем крутость передавать парметры так, вместо ручной передачи через конструкторы? Как только возникает что-нибудь нетривиальное, подход с guice превращается в макароны. А что-нибудь нетривиальное возникает очень и очень легко. Например, у меня в приложении есть простенький интерфейс для кэша. Штуки три его реализации (с разными политиками). И десяток экземпляров (разные сущности, используются в разных местах). Вопрос — ну и как их инжектить? Можно взять что-нибудь менее service-locator-подобное. Какой-нибудь spring. И написать километры xml. А смысл? Можно написать те же километры кода (причем они будут короче, чем xml). А ведь можно взять scala, с помощью implicits делается аналог autowired. И при этом я в коде не ограничен условностями, могу обертки вокруг класса создать, могу метод написать (у меня сервлеты регистрируются с навеской нужных фильтров и конверторов подобным образом).

Ладно, переходим к самому интересному. BillingService не нужен. И вообще любые *Service не нужны. Да, именно не нужны. Если у нее действительно один метод, то это не service, это какой-нибудь банальный BuyProcessor. А если методов много, то службой никто в полной мере не пользуется. Она только создает лишние зависимости. Какой-то класс пользуется двумя методами из службы. Какой-то — другими тремя. И никто — всем набором методов. Зато мокать это безобразие с кучей методов — одно "удовольствие". А мокать нужно, ведь класс, зависящий от BillingService может потом изменить реализацию и использовать другие (эквивалентные) методы.

Обратите внимание, в предыдущем абзаце я описал интересный факт. Интерфейс служб зависит от нужд вышележащего уровня, а не нижележащего! Т.е. "низкоуровневая" служба может реализовывать несколько интерфейсов "более высокого" уровня. А иначе у вас какое-то безобразие получается. С одной стороны, вы хотите изолировать зависимость (и для этого как раз нормально вводить интерфейс). С другой, этот интерфейс для разделения определяется не "точкой разреза между слоями" ("cut point") а интерфейсом реализации. Те методы, которые реализует ваша RealBillingService и попадут в в BillingService. Это вообще что такое? Интерфейсы ради интерфейсов. И ведь не отрежешь — новая реализация BillingService будет похожа на RealBillingService, о какой слабой связности идет речь?

Теперь о том, как делать правильно. Где-то рядом уже правильно заметили, что интерфейс может быть и не нужен. Где пользователь этого интерфейса? Какой-нибудь обработчик веб-запросов или кликов на кнопку? А в чем великий смысл выделять интерфейс для BillingService и в нее выносить 10 тривиальных строчек? Я эти десять строчек в том фронт-контроллере размещу (в отдельном методе). Или тот обработчик будет еще с каким-то другим BillingService работать? Как-то я сомневаюсь пока, да и имя должно быть в этом случае другое (это buyProcessor). Для тестирования формата ответа фронтенда? Но, опять же, какой смысл в тестах? Интеграционное тестирование проверит и формат ответа, и работоспособность метода. Ну и пиццу даст в качестве бонуса.

Ладно, еще одно соображение. Есть несколько пользователей этого метода. Но, опять же, зачем выделять интерфейс? Можно статический метод сделать, куда передавать нужные зависимости. А можно и выделить интерфейс. И сделать одну большую службу, которая реализует много разных интерфейсов (BuyProcessor, OrderCancellingProcessor, и т.п.). Или можно сделать одну большую службу и передать ее в пользователей. Какая разница (если только не ставить задачу зачем-то протестировать все юниты)?

Итого. В самом простейшем случае вся реализация RealBillingService.chargeOrder уйдет во front controller. С большой вероятностью, туда же уйдет и TransactionLog.logChargeResult, хотя все зависит от того, как реализован этот метод. CreditCardProcessor останется интерфейсом (может быть, порезанным на мелочи вроде CreditCardAcquirer), потому как там функциональность большая, сложная и подверженная изменению. Инжектится он будет в конструкторе фронт-контроллера (самый простой и надежный способ). Учитывая, предыдущие упрощения, останется не так много сервисов. Придется инжектить: CreditCardProcessor, PizzaFactory, GlobalLog, UserManager, несколько справочников. Причем это список на все фронт-контроллеры, а не только на один конкретный.

P.S.
Я уже выше отмечал, что зависимость на Receipt в данном случае отвратительна. Поэтому небольшое упражнение на понимание DI и умение пользоваться ими. Поправим код метода.
  public Receipt chargeOrder(PizzaOrder order, CreditCard creditCard) {
    long receiptId = receiptIdGenerator.???; //Здесь
    transactionLog.logReceiptProcessing(receiptId, order, creditCard);
    try {
      ChargeResult result = processor.charge(creditCard, order.getAmount());
      transactionLog.logChargeResult(receiptId, result);

      return result.wasSuccessful()
          ? Receipt.forSuccessfulCharge(receiptId, order.getAmount())
          : Receipt.forDeclinedCharge(receiptId, result.getDeclineMessage());
     } catch (UnreachableException e) {
      transactionLog.logConnectException(e);
      return Receipt.forSystemFailure(receiptId, e.getMessage());
    }
  }

Вот такой новый код. Нужно дописать вызов метода для генерации Id, интерфейс/класс, который имеет receiptIdGenerator (название и члены класса), и то, как именно способом вы его хотите инжектить в BillingService.
Re[22]: своё vs. сторонее
От: . Великобритания  
Дата: 25.10.13 16:17
Оценка:
Здравствуйте, Ikemefula, Вы писали:

I>Для BillingService не нужен никакой DI, максимум, что можно показать на ём, это Inversion of Control, т.е. вынести все лишние депенденсы куда нибудь наверх. Больше ничего на этом сервисе показать не удастся.

Ну покажи, посмотрим.
DI это один из вариантов реализации IoC для зависимостей. Есть ещё SL, ну и глобальные переменные. Так что ты предлагаешь-то?

I>Потому с DI остаётся всё, как было сказано: "предположим, DI нужен, следовательно, DI нужен"

I>>>Это неважно, чем он соответсвует. Важно как эта сущность используется с другими сущностями, т.е. как строится вызывающий код.
.>>Да какая разница? Допустим он вызывается из servlet.onPost, парся всякие пост-параметры заказа. И, например, обрабатывает заказы из входящих емейлов/смскок.
.>>Предложи свое видение.
I>Нужно смотреть вызывающий модуль, что в ём будет.
class PizzaOrderServlet extends HttpServlet
{
    @Inject BillingService billingService;

    @Override public void onPost(HttpRequest request, HttpResponse response)
    {
       PizzaOrder order = parseOrder(request);
       CreditCard cc = parseCreditCard(request);
       Receipt recepit = billingService.chargeOrder(order, cc);
       writeRecepit(receipt, response);
    }
    private PizzaOrder parseOrder(HttpRequest request)
    {
      /// parse and validate data from request and return new PizzaOrder
    }
...parseCreditCard, writeRecepit....
}

аналогично для емейлов и т.п.
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Re[21]: своё vs. сторонее
От: maxkar  
Дата: 25.10.13 16:26
Оценка:
Здравствуйте, ., Вы писали:

.>>>И обрати внимание что этот класс соответствует бизнес-сущности "биллинг". Можно, конечно, сказать что нафиг это всё и писать всё в main-методе или onclick_button, но вроде это не самая лучшая практика.

I>>Это неважно, чем он соответсвует. Важно как эта сущность используется с другими сущностями, т.е. как строится вызывающий код.
.>Да какая разница? Допустим он вызывается из servlet.onPost, парся всякие пост-параметры заказа. И, например, обрабатывает заказы из входящих емейлов/смскок.
.>Предложи свое видение.

Это важно! Это очень важно. Если у вас только один транспорт, можно все заинлайнить. Это нормально.

Если же транспортов несколько, возможны варианты. Либо все ваши транспорты (http/email/sms) — придатки к одному приложению. В этом случае они зависят не от конкретных служб, а от большого фасада Application, который предоставляет все возможные функции. Это вполне конкретный класс, который внутри реализует все, что нужно. Возможно — вызывая другие службы. Возможно — своими методами.

А еще есть ненулевая вероятность, что в случае нескольких транспортов BillingService.chargeOrder будет заинлайнен в соответствующие фронтенды. По одной банальной причине — это слишком общий интерфейс для биллинга. Для "onClick" пользователь может определяться авторизацией на сайте. Для sms — номером телефона. Как вы их будете в transactionLog представлять? А еще, вероятно, в бизнес-требованиях будет задача разделять различные источники заказов для построения статистики. Кто их будет строить? Как? Вы научите BillingService/TransactionLog различать разные типы пользователей из order? Или это будут разные логгеры (и разные billing service)?

При инлайне в обработчики все станет гораздо лучше. Мы будем иметь нужный тип логгера в обработчике. Вот CreditCardProcessor может быть заинжектен в разные реализации, он имеет смысл. А BillingProcessor — слишком тривиален и слишком нестабилен по функциональности. А еще плохо расширяем. Что, если одним из способов платежа для sms будет списание с баланса телефона? Для sms-гейта это будет вызов другого "payment provider" (с другим интерфейсом), а основная обвязка вроде логирования останется общая.

И еще один интересный факт. Очень хорошо заметный на нескольких транспортах. Существующий код занимается переливанием из пустого в порожнее. Сначала упаковывает все коды в Receipt. Затем во frontend'е его if'ами/case'ами распаковывает. Если мы получили UnreacheableException (почему оно, кстати???), то на фронтенде мы можем сразу ответ начать выдавать, вместо заворачиванием этого исключения в красивую обертку. Ну и receipt в текущем виде становится странным. Там появляются хитрые варианты вроде "это вроде бы и receipt, но он не на покупку, а на системную ошибку". Его теперь приходится везде проверять на валидность.
Re[23]: своё vs. сторонее
От: Ikemefula Беларусь http://blogs.rsdn.org/ikemefula
Дата: 25.10.13 17:00
Оценка:
Здравствуйте, ., Вы писали:

.>Здравствуйте, Ikemefula, Вы писали:


I>>Для BillingService не нужен никакой DI, максимум, что можно показать на ём, это Inversion of Control, т.е. вынести все лишние депенденсы куда нибудь наверх. Больше ничего на этом сервисе показать не удастся.

.>Ну покажи, посмотрим.
.>DI это один из вариантов реализации IoC для зависимостей. Есть ещё SL, ну и глобальные переменные. Так что ты предлагаешь-то?

IoC это обрезание депенденсов, а не управление ими. А вот после обрезания ими можно управлять, хоть через локатор, хоть через DI, хоть через глобальные переменные, а можно и вовсе руками. Потому IoC != DI.

I>>Нужно смотреть вызывающий модуль, что в ём будет.

.>
.>class PizzaOrderServlet extends HttpServlet
.>{
.>    @Inject BillingService billingService;

.>    @Override public void onPost(HttpRequest request, HttpResponse response)
.>    {
.>       PizzaOrder order = parseOrder(request);
.>       CreditCard cc = parseCreditCard(request);
.>       Receipt recepit = billingService.chargeOrder(order, cc);
.>       writeRecepit(receipt, response);
.>    }
.>    private PizzaOrder parseOrder(HttpRequest request)
.>    {
.>      /// parse and validate data from request and return new PizzaOrder
.>    }
.>...parseCreditCard, writeRecepit....
.>}
.>

.>аналогично для емейлов и т.п.

Твой onPost объявлен, внимание, в конкретном модуле, то есть, прибит к нему гвоздями.

Вот, по ссылке
public class BillingModule extends AbstractModule {
  @Override 
  protected void configure() {
    bind(TransactionLog.class).to(DatabaseTransactionLog.class);  
    bind(CreditCardProcessor.class).to(PaypalCreditCardProcessor.class);
    bind(BillingService.class).to(RealBillingService.class);
  }
}


Все параметры биндов, внимание, это жосткие зависимости
То есть, модуль, гвоздями прибит к депенденсам.
Вот уже здесь произошел фейл — смысла в DI в данном примере нет и быть не может. Что вобщем то очевидно, у DI масштаб на порядок побольше, чем один твой onPost.
Re[15]: своё vs. сторонее
От: IT Россия linq2db.com
Дата: 25.10.13 17:41
Оценка: +2
Здравствуйте, Baudolino, Вы писали:

IT>>Если очень хочется пообсуждать криворукость архитектуры и архитекторов, то достаточно показать здесь свой наиправильнейший код. Или слабо?

B>Это аргумент из серии "сперва добейся"? Ну-ну. Я связан NDA с нынешним и бывшими своими работодателями. Хочешь посмотреть мой код — устройся для начала ко мне на работу.

Понятно. Стандартная отмазка. Впрочем, не очень то и хотелось ни твоего кода, ни уж тем более к тебе на работу.
... << RSDN@Home 1.2.0 alpha 5 rev. 69>>
Если нам не помогут, то мы тоже никого не пощадим.
Re[13]: своё vs. сторонее
От: IT Россия linq2db.com
Дата: 25.10.13 17:41
Оценка:
Здравствуйте, Baudolino, Вы писали:

B>>>Вы тесты в своей жизни когда-нибудь писали?

IT>>Ты что-то хотел сказать?
B>Я задал вопрос. Ответ будет?

Смысл отвечать на риторику. Тебя же мой ответ всё равно не интересует. Иначе бы ты внимательнее почитал бы топик и сам бы нашёл ответ.
... << RSDN@Home 1.2.0 alpha 5 rev. 69>>
Если нам не помогут, то мы тоже никого не пощадим.
Re[16]: своё vs. сторонее
От: fddima  
Дата: 25.10.13 20:20
Оценка:
Здравствуйте, IT, Вы писали:

IT>Понятно. Стандартная отмазка. Впрочем, не очень то и хотелось ни твоего кода, ни уж тем более к тебе на работу.

Тут пол форума NDA, так что +1. Если хочешь что-то доказать — показывай код.

PS: Я тут на днях пробовал к linq2db прикрутить csharp-sqlite (полностью мэнэджет/шарповый порт) — заработало блин, без модификации самого linq2db, с добавлением мелкого провайдера на 10 строк. Мега классная штука. Позже, возможно стоит добавть как-то в общую копилку. (поностью шарповый sqlite годен тем, когда ты на asp.net непонятно где и непонятно с кем и тупо для эмулятора сервиса например, по крайей мере так я его применяю). С нативом не так удобно (залочены длл, если приложение запущено).
Re[18]: своё vs. сторонее
От: . Великобритания  
Дата: 25.10.13 20:59
Оценка:
Здравствуйте, maxkar, Вы писали:

M>Задача отвратительно решена. И поставлена тоже.

M>Начнем с теста. Он бесполезен. Конкретно его функциональность проверяется в момент интеграционного тестирования.
Это называется юнит-тест. Спор юнит-тест vs инт-тест я затевать не хочу. Что такое юнит-тестирование я рассказывать не буду. Если хочется восполнить пробел в знаниях, материала в инете полно.

M>Мало ли какой-там CreditCardProcessor затесался в сборку. Может, там Fake лежит. А интеграционное тестирование это все покажет.

Жуть какая. Ну не используйте тогда "кривые" фреймворки от M$, или руки выпрямляйте. В guice это явно выражается ровно одной строчкой кода, нет никакой магии, всё выражается явно, читай ту ссылку:
    bind(CreditCardProcessor.class).to(PaypalCreditCardProcessor.class);


М>Причем этот сценарий "все заказывается и работает" проверяться будет обязательно — это основной пользовательский сценарий. Ну а пицца — команде в качестве бонуса за релиз.

Конечно. Но это не отменяет ценность юнит-теста.

M>Далее. Тест не тестирует контракт! Тест каким-то сложным и запутанным способом проверяет, что именно написано в методе. Повторяю — не что метод делает, а что именно написано! Ну вот creditCardProcessor.getCardOfOnlyCharge() есть, для примера. А что, если потом будет сделана не одна, а несколько charge? Или сначала резервирование средств на счету, а только потом списание? Ведь "в реальной жизни" в этом creditCardProcessor куча разных методов. И некоторые вещи на самом деле можно реализовать несколькими способами. Ту же продажу, например. Так что корректно замокать этого монстрика — то еще приключение. Можно внедрить ревью кода для проверки подобных реализаций, это будет гораздо проще.

Ну-ну. А потом неудачный рефакторинг, опечатка и посыпалось. Ревью — ручное, а юнит-тесты — автоматические.
Короче, рекомендую изучить юнит-тестирование.

M>Еще стоит вспомнить, что для данного метода нужно бы какую-нибудь транзакционность проверять.

Ага, и чтобы этот метод ещё и кофе варил.

M>Сама сигнатура метода странная. Зачем там order, если используется только amount? И самая интересная вещь в виде Receipt не инжектится. Это вообще должна быть generic service, параметризованная типом Receipt.

Ну вот такя имплементация, пользоваться уже можно. Потом можно дописывать, например по составу заказа сделать текстовое описание и послать как description для платежа.
Receipt это просто данные, зачем его инжектить?
DI плохо, а зато generic пихать где попало, так пожалуйста...
Это же просто демо-проект. В реальном коде там будет побольше строчек, конечно.

M>Решение использовать Guice — тоже странное. Автор сам себя ведь выше опроверг. В пункте factory. Вы думаете, что Guice — делает что-то похожее на "dependency injection"? Нет. Это service locator в практически чистом виде, что ближе к factory.

Так DI же и делает. Видимо ты не понял что-ли что там происходит, т.к. в классе RealBillingService что-то относящееся к guice это только @Inject (т.е. ровно одна лексема!).

M>Смотрите, легкое движение руками и пример с инжектором превращается в

M>
M>    this.processor = injector.lookup(CreditCardProcessor.class);
M>

Конечно можно превратить. guice предоставляет и SL, только здесь лучше всё-таки DI.

M>Второе поле — это эквивалент @Named. Я вот не понимаю, в чем крутость передавать парметры так, вместо ручной передачи через конструкторы? Как только возникает что-нибудь нетривиальное, подход с guice превращается в макароны.

Можно и вручную передавать, в главе "Dependency Injection" это и показано как. Но guice делается ровно то же, но автоматически.

M> А что-нибудь нетривиальное возникает очень и очень легко. Например, у меня в приложении есть простенький интерфейс для кэша. Штуки три его реализации (с разными политиками). И десяток экземпляров (разные сущности, используются в разных местах). Вопрос — ну и как их инжектить?

Это нетривиально потому что ты вручную делаешь. Для guice — тривиально. http://code.google.com/p/google-guice/wiki/BindingAnnotations

M> Можно взять что-нибудь менее service-locator-подобное. Какой-нибудь spring. И написать километры xml. А смысл? Можно написать те же километры кода (причем они будут короче, чем xml). А ведь можно взять scala, с помощью implicits делается аналог autowired. И при этом я в коде не ограничен условностями, могу обертки вокруг класса создать, могу метод написать (у меня сервлеты регистрируются с навеской нужных фильтров и конверторов подобным образом).

Ну собственно в guice как раз xml и не испольуется, и как IoC он на порядок лучше того же spring, так как продумывался именно для этого и с учётом ошибок спринга.
Скалу не обсуждаю... язык другой, там всё по другому может выглядеть. Guice писался для явы и на неё заточен с учётом её особенностей.

M>Ладно, переходим к самому интересному. BillingService не нужен. И вообще любые *Service не нужны. Да, именно не нужны. Если у нее действительно один метод, то это не service, это какой-нибудь банальный BuyProcessor. А если методов много, то службой никто в полной мере не пользуется. Она только создает лишние зависимости. Какой-то класс пользуется двумя методами из службы. Какой-то — другими тремя. И никто — всем набором методов.

Как интерфейс, скорее всего, он не нужен. Но сути не меняет. Пусть будет класс BillingService. И пусть он называется BuyProcessor. Что это меняет?

M> Зато мокать это безобразие с кучей методов — одно "удовольствие". А мокать нужно, ведь класс, зависящий от BillingService может потом изменить реализацию и использовать другие (эквивалентные) методы.

Не понял. Это тоже тривиально. Или в .net нет аналогов mockito/easymock/jmock/etc?

M>Обратите внимание, в предыдущем абзаце я описал интересный факт. Интерфейс служб зависит от нужд вышележащего уровня, а не нижележащего! Т.е. "низкоуровневая" служба может реализовывать несколько интерфейсов "более высокого" уровня. А иначе у вас какое-то безобразие получается. С одной стороны, вы хотите изолировать зависимость (и для этого как раз нормально вводить интерфейс). С другой, этот интерфейс для разделения определяется не "точкой разреза между слоями" ("cut point") а интерфейсом реализации. Те методы, которые реализует ваша RealBillingService и попадут в в BillingService. Это вообще что такое? Интерфейсы ради интерфейсов. И ведь не отрежешь — новая реализация BillingService будет похожа на RealBillingService, о какой слабой связности идет речь?

Забудь RealBillingService, для простоты можешь его переименовать в BillingService, а BillingService выкинуть, это же тривиальный рефакторинг "inline interface". Что изменится? Если же потом понадобится ещё одна реализация, сделаешь "extract interface" рефакторинг.

M>Теперь о том, как делать правильно. Где-то рядом уже правильно заметили, что интерфейс может быть и не нужен. Где пользователь этого интерфейса? Какой-нибудь обработчик веб-запросов или кликов на кнопку? А в чем великий смысл выделять интерфейс для BillingService и в нее выносить 10 тривиальных строчек? Я эти десять строчек в том фронт-контроллере размещу (в отдельном методе). Или тот обработчик будет еще с каким-то другим BillingService работать? Как-то я сомневаюсь пока, да и имя должно быть в этом случае другое (это buyProcessor). Для тестирования формата ответа фронтенда? Но, опять же, какой смысл в тестах? Интеграционное тестирование проверит и формат ответа, и работоспособность метода. Ну и пиццу даст в качестве бонуса.

Пользователей может быть несколько — один web-site, крутится в облаке, один обработчик смс — на специальном серваке с gsm-модемом, и т.п. Класс один — контексты очень разные.

M>Ладно, еще одно соображение. Есть несколько пользователей этого метода. Но, опять же, зачем выделять интерфейс?

Не хочешь — не выделяй, никто не заставляет.

M>Можно статический метод сделать, куда передавать нужные зависимости.

А там их откуда брать? Из тумбочки? А что если понадобится добавить ещё одну зависимость, например, отсылалку email-уведомлений в биллинг? В скольких местах придётся вызов этого статического метода менять, протаскивая EmailService в кучу мест?

M>А можно и выделить интерфейс. И сделать одну большую службу, которая реализует много разных интерфейсов (BuyProcessor, OrderCancellingProcessor, и т.п.). Или можно сделать одну большую службу и передать ее в пользователей. Какая разница (если только не ставить задачу зачем-то протестировать все юниты)?

Да пожалуйста, кто не даёт?

M>Итого. В самом простейшем случае вся реализация RealBillingService.chargeOrder уйдет во front controller. С большой вероятностью, туда же уйдет и TransactionLog.logChargeResult, хотя все зависит от того, как реализован этот метод. CreditCardProcessor останется интерфейсом (может быть, порезанным на мелочи вроде CreditCardAcquirer), потому как там функциональность большая, сложная и подверженная изменению.

M>Инжектится он будет в конструкторе фронт-контроллера (самый простой и надежный способ).
Опа! Т.е. если инжектить зависимость, но не называть это Dependency Injection, то это сразу становится хорошим и правильным способом? А ты случайно не заметил, что в главе "Dependency Injection with Guice" ровно это и сделано?

M>Учитывая, предыдущие упрощения, останется не так много сервисов. Придется инжектить: CreditCardProcessor, PizzaFactory, GlobalLog, UserManager, несколько справочников. Причем это список на все фронт-контроллеры, а не только на один конкретный.

И то же самое в SmsOrderProcessor... и везде. А инжектить будем магией, но главное всем молчать и не произносить "DI".

M>Вот такой новый код. Нужно дописать вызов метода для генерации Id, интерфейс/класс, который имеет receiptIdGenerator (название и члены класса), и то, как именно способом вы его хотите инжектить в BillingService.

Не понял. Что в этом коде нового?
receiptIdGenerator вообще новая сущность, будем считать что оно не нужно. Зачем ты её ввёл? В постановке задачи этого не было. Может быть оно будет браться из ChargeResult, какой-нибудь transaction reference.
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.