Таким образом, я пишу этот код для тестирования, но что-то не так с ним, что я пропускаю?

13

У меня интерфейс называется IContext. Для целей этого не имеет значения, что он делает, за исключением следующего:

T GetService<T>();

Этот метод просматривает текущий DI-контейнер приложения и пытается разрешить зависимость. Я думаю, что он довольно стандартный.

В моем приложении ASP.NET MVC мой конструктор выглядит следующим образом.

protected MyControllerBase(IContext ctx)
{
    TheContext = ctx;
    SomeService = ctx.GetService<ISomeService>();
    AnotherService = ctx.GetService<IAnotherService>();
}

Поэтому вместо того, чтобы добавлять несколько параметров в конструктор для каждого сервиса (потому что это будет очень раздражающим и трудоемким для разработчиков, расширяющих приложение), я использую этот метод для получения сервисов.

Теперь он чувствует себя не так . Однако, способ, которым я в настоящее время оправдываю это в своей голове, состоит в следующем - я могу издеваться над этим .

Я могу. Нетрудно создать макет IContextдля тестирования контроллера. Я должен был бы в любом случае:

public class MyMockContext : IContext
{
    public T GetService<T>()
    {
        if (typeof(T) == typeof(ISomeService))
        {
            // return another mock, or concrete etc etc
        }

        // etc etc
    }
}

Но, как я уже сказал, это неправильно. Любые мысли / оскорбления приветствуются.

LiverpoolsNumber9
источник
8
Это называется Service Locator, и мне это не нравится. Было много писем на эту тему - см. Martinfowler.com/articles/injection.html и blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern для начала.
Бенджамин Ходжсон
Из статьи Мартина Фаулера: «Я часто слышал жалобу о том, что такого рода локаторы сервисов - плохая вещь, потому что они не тестируемы, потому что вы не можете заменить их реализациями. Конечно, вы можете их плохо спроектировать, чтобы попасть в это проблемы, но вам не нужно. В этом случае экземпляр локатора сервиса - это просто простой держатель данных. Я могу легко создать локатор с помощью тестовых реализаций моих сервисов ». Не могли бы вы объяснить, почему вам это не нравится? Может быть в ответе?
LiverpoolsNumber9
8
Он прав, это плохой дизайн. Это легко public SomeClass(Context c). Этот код вполне понятен, не так ли? В нем говорится, that SomeClassзависит от Context. Эээ, но подождите, это не так! Он зависит только от зависимости, которую Xон получает из Context. Это означает, что каждый раз, когда вы вносите изменения, Contextони могут сломаться SomeObject, даже если вы изменили только Contexts Y. Но да, вы знаете, что вы только Yне изменились X, так что SomeClassвсе в порядке. Но написание хорошего кода - это не то, что вы знаете, а то, что знает новый сотрудник, когда он смотрит на ваш код в первый раз.
валентри
@DocBrown Для меня это именно то, что я сказал - я не вижу здесь никакой разницы. Не могли бы вы объяснить подробнее?
валентри
1
@DocBrown Теперь я понимаю вашу точку зрения. Да, если его Context - это просто набор всех зависимостей, то это неплохой дизайн. Это может быть, однако, плохое наименование, но это также только предположение. ОП должен уточнить, есть ли еще методы (внутренние объекты) контекста. Кроме того, обсуждение кода - это хорошо, но это - programmers.stackexchange, поэтому мне также следует постараться увидеть «позади» то, что должно улучшить OP.
валентри

Ответы:

5

Наличие одного вместо множества параметров в конструкторе не является проблемной частью этого проекта . Пока ваш IContextкласс - не что иное, как фасад службы , специально предназначенный для предоставления зависимостей, используемых MyControllerBase, а не общий локатор служб, используемый во всем коде, эта часть вашего кода, IMHO, в порядке.

Ваш первый пример может быть изменен на

protected MyControllerBase(IContext ctx)
{
    TheContext = ctx;
    SomeService = ctx.GetSomeService();
    AnotherService = ctx.GetAnotherService();
}

это не будет существенным изменением дизайна MyControllerBase. Хороший или плохой дизайн зависит только от того, хотите ли вы

  • убедитесь TheContext, SomeServiceи AnotherServiceвсегда все инициализируются фиктивными объектами, или все из них с реальными объектами
  • или, чтобы разрешить их инициализацию различными комбинациями 3 объектов (что означает, что в этом случае вам нужно будет передать параметры индивидуально)

Таким образом, использование только одного параметра вместо 3 в конструкторе может быть вполне разумным.

Проблема, которая является проблематичной, это IContextразоблачение GetServiceметода на публике. ИМХО, вам следует избегать этого, вместо этого оставляйте «фабричные методы» явными. Так будет нормально реализовать GetSomeServiceи GetAnotherServiceметоды из моего примера , используя локатор службы? ИМХО это зависит. Пока IContextкласс продолжает использовать простую абстрактную фабрику для конкретной цели предоставления явного списка сервисных объектов, это ИМХО приемлемо. Абстрактные фабрики, как правило, представляют собой просто «склеивающий» код, который не должен сам тестироваться модулем. Тем не менее, вы должны спросить себя, в контексте таких методов, как GetSomeService, действительно ли вам нужен локатор службы или явный вызов конструктора не будет проще.

Так что будьте осторожны, когда вы придерживаетесь дизайна, в котором IContextреализация представляет собой просто обертку вокруг общедоступного универсального GetServiceметода, позволяющего разрешать любые произвольные зависимости произвольными классами, тогда все применимо к тому, что @BenjaminHodgson написал в своем ответе.

Док Браун
источник
Я согласен с этим. Проблема с примером - общий GetServiceметод. Рефакторинг к явно названным и типизированным методам лучше. Еще лучше было бы IContextчетко изложить зависимости реализации в конструкторе.
Бенджамин Ходжсон
1
@BenjaminHodgson: иногда бывает лучше, но не всегда лучше. Вы замечаете запах кода, когда список параметров ctor становится длиннее и длиннее. Смотрите мой предыдущий ответ здесь: programmers.stackexchange.com/questions/190120/…
Док Браун
@DocBrown «запах кода» избыточной инъекции в конструктор указывает на нарушение SRP, что является реальной проблемой. Простое объединение нескольких служб в класс Facade только для того, чтобы представить их как свойства, никак не влияет на источник проблемы. Таким образом, Фасад не должен быть простой оберткой вокруг других компонентов, но в идеале должен предлагать упрощенный API, с помощью которого их можно использовать (или следует упростить их каким-либо другим способом) ...
AlexFoxGill
... Помните, что чрезмерный впрыск конструктора в виде «запаха кода» не является проблемой сам по себе. Это просто намек на то, что в коде есть более глубокая проблема, которая обычно может быть решена путем
разумного
Где вы были, когда Microsoft запекла это IValidatableObject?
RubberDuck
15

Этот дизайн известен как Service Locator *, и мне он не нравится. Есть много аргументов против этого:

Сервисный локатор соединяет вас с вашим контейнером . Используя регулярное внедрение зависимостей (где конструктор явно прописывает зависимости), вы можете напрямую заменить ваш контейнер на другой или вернуться к new-expressions. С тобой IContextэто не реально возможно.

Сервисный локатор скрывает зависимости . Как клиент, очень сложно сказать, что вам нужно для создания экземпляра класса. Вам нужно что-то вроде IContext, но вы также должны установить контекст, чтобы вернуть правильные объекты, чтобы сделать MyControllerBaseработу. Это совсем не очевидно из подписи конструктора. При обычном DI компилятор говорит вам именно то, что вам нужно. Если у вашего класса много зависимостей, вы должны чувствовать эту боль, потому что это подтолкнет вас к рефакторингу. Сервисный локатор скрывает проблемы с плохим дизайном.

Service Locator вызывает ошибки во время выполнения . Если вы позвоните GetServiceс неверным параметром типа, вы получите исключение. Другими словами, ваша GetServiceфункция не является полной функцией. (Итоговые функции - идея из мира FP, но в основном это означает, что функции всегда должны возвращать значение.) Лучше позволить компилятору помочь и сообщить вам, когда вы неправильно поняли зависимости.

Сервисный локатор нарушает принцип подстановки Лискова . Поскольку его поведение варьируется в зависимости от аргумента типа, сервисный локатор можно рассматривать так, как будто он имеет бесконечное количество методов в интерфейсе! Этот аргумент подробно изложен здесь .

Сервисный локатор сложно проверить . Вы привели пример подделки IContextдля тестов, и это хорошо, но, конечно, лучше не писать этот код в первую очередь. Просто внедрите свои поддельные зависимости напрямую, не заходя в локатор службы.

Короче, просто не делай этого . Это кажется соблазнительным решением проблемы классов с множеством зависимостей, но в конечном итоге вы просто сделаете свою жизнь несчастной.

* Я определяю Service Locator как объект с помощью универсального Resolve<T>метода, который способен разрешать произвольные зависимости и используется во всей кодовой базе (а не только в корне композиции). Это не то же самое, что Service Facade (объект, который связывает некоторый небольшой известный набор зависимостей) или Abstract Factory (объект, который создает экземпляры одного типа - тип Abstract Factory может быть универсальным, но метод - нет) ,

Бенджамин Ходжсон
источник
1
Вы собираете информацию о шаблоне Service Locator (с которым я согласен). Но на самом деле, в примере с OP, MyControllerBaseон не связан с конкретным DI-контейнером, и это не «настоящий» пример для анти-паттерна Service Locator.
Док Браун
@DocBrown Я согласен. Не потому, что это облегчает мою жизнь, а потому, что большинство приведенных выше примеров не имеют отношения к моему коду.
LiverpoolsNumber9
2
Для меня отличительной чертой анти-паттерна Service Locator является универсальный GetService<T>метод. Устранение произвольных зависимостей - это настоящий запах, который присутствует и корректен в примере ОП.
Бенджамин Ходжсон
1
Другая проблема с использованием Service Locator заключается в том, что он снижает гибкость: у вас может быть только одна реализация каждого интерфейса службы. Если вы создаете два класса, которые полагаются на IFrobnicator, но позже решите, что один должен использовать вашу оригинальную реализацию DefaultFrobnicator, но другой действительно должен использовать декоратор CacheingFrobnicator вокруг него, вам придется изменить существующий код, тогда как если вы Внедрение зависимостей напрямую, все, что вам нужно сделать, это изменить код установки (или файл конфигурации, если вы используете инфраструктуру DI). Таким образом, это нарушение OCP.
Жюль
1
@DocBrown GetService<T>()Метод разрешает запрашивать произвольные классы: «Что делает этот метод, так это просматривает текущий DI-контейнер приложения и пытается разрешить зависимость. Я думаю, это довольно стандартно». , Я отвечал на ваш комментарий вверху этого ответа. Это 100% сервисный локатор
AlexFoxGill
5

Марк Симанн ясно изложил лучшие аргументы против анти-паттерна «Сервисный локатор», поэтому я не буду слишком вдаваться в то, почему это плохая идея - это учебное путешествие, которое нужно потратить на то, чтобы понять это самостоятельно (я также рекомендую книгу Марка ).

Хорошо, чтобы ответить на вопрос - давайте переформулируем вашу актуальную проблему :

Поэтому вместо того, чтобы добавлять несколько параметров в конструктор для каждого сервиса (потому что это будет очень раздражающим и трудоемким для разработчиков, расширяющих приложение), я использую этот метод для получения сервисов.

Существует вопрос, который решает эту проблему в StackOverflow . Как уже упоминалось в одном из комментариев:

Лучшее замечание: «Одним из замечательных преимуществ использования Constructor Injection является то, что он делает нарушения принципа единой ответственности совершенно очевидными».

Вы ищете неправильное место для решения вашей проблемы. Важно знать, когда класс делает слишком много. В вашем случае я сильно подозреваю, что нет необходимости в «Базовом контроллере». На самом деле, в ООП почти всегда нет нужды в наследовании . Различия в поведении и общей функциональности могут быть полностью достигнуты за счет надлежащего использования интерфейсов, что обычно приводит к более качественному и инкапсулированному коду - и нет необходимости передавать зависимости конструкторам суперкласса.

Во всех проектах, над которыми я работал, где есть базовый контроллер, это было сделано исключительно для целей совместного использования удобных свойств и методов, таких как IsUserLoggedIn()и GetCurrentUserId(). СТОП . Это ужасное злоупотребление наследством. Вместо этого создайте компонент, который предоставляет эти методы, и возьмите зависимость от него там, где он вам нужен. Таким образом, ваши компоненты останутся тестируемыми, и их зависимости будут очевидны.

Помимо всего прочего, при использовании шаблона MVC я всегда рекомендовал бы тощие контроллеры . Вы можете прочитать больше об этом здесь, но суть паттерна проста: контроллеры в MVC должны делать только одно: обрабатывать аргументы, передаваемые инфраструктурой MVC, делегируя другие проблемы другому компоненту. Это опять-таки принцип единой ответственности в действии.

Это действительно помогло бы узнать ваш вариант использования, чтобы сделать более точное суждение, но, честно говоря, я не могу придумать ни одного сценария, в котором базовый класс предпочтительнее, чем обоснованные зависимости.

AlexFoxGill
источник
+1 - это новый взгляд на вопрос, на который ни один из других ответов на самом деле не адресован
Бенджамин Ходжсон
1
«Одним из замечательных преимуществ Constructor Injection является то, что он делает нарушения принципа единой ответственности совершенно очевидными». Действительно хороший ответ. Не соглашайтесь со всем этим. Мой случай использования, как ни странно, заключается в том, что мне не нужно дублировать код в системе, которая будет иметь более 100 контроллеров (по крайней мере). Однако в отношении SRP - каждая внедренная служба несет единую ответственность, как и контроллер, который использует их все - как бы один рефрактор это ??!
LiverpoolsNumber9
1
@ LiverpoolsNumber9 Ключ в том, чтобы переместить функциональность из BaseController в зависимости, пока в BaseController не останется ничего, кроме protectedоднострочных делегаций новым компонентам. Тогда вы можете потерять BaseController и заменить эти protectedметоды прямыми вызовами зависимостей - это упростит вашу модель и сделает все ваши зависимости явными (что очень хорошо в проекте с сотнями контроллеров!)
AlexFoxGill
@ LiverpoolsNumber9 - если бы вы могли скопировать часть вашего BaseController в вставку, я мог бы сделать несколько конкретных предложений
AlexFoxGill
-2

Я добавляю ответ на этот вопрос, основываясь на вкладе всех остальных. Огромное спасибо всем. Сначала вот мой ответ: «Нет, в этом нет ничего плохого».

Ответ Дока Брауна «Service Facade» Я принял этот ответ, потому что то, что я искал (если ответ был «нет»), было некоторыми примерами или некоторым расширением того, что я делал. Он представил это, предположив, что А) у него есть имя и Б), возможно, есть лучшие способы сделать это.

Ответ Бенджамина Ходжсона «Сервисный локатор» Как бы я ни ценил полученные здесь знания, я не являюсь «Сервисным локатором». Это «Фасад обслуживания». Все в этом ответе правильно, но не для моих обстоятельств.

Ответы USR

Я рассмотрю это более подробно:

Вы отказываетесь от большого количества статической информации таким образом. Вы откладываете решения на время выполнения, как это делают многие динамические языки. Таким образом вы теряете статическую проверку (безопасность), поддержку документации и инструментов (автозаполнение, рефакторинг, поиск использования, поток данных).

Я не теряю никаких инструментов и не теряю никакой «статической» печати. Фасад службы вернет то, что я настроил в своем контейнере DI, или default(T). И то, что он возвращает, «напечатано». Отражение заключено в капсулу.

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

Это конечно не "редкость". Поскольку я использую базовый контроллер, каждый раз, когда мне нужно изменить его конструктор, мне, возможно, придется менять 10, 100, 1000 других контроллеров.

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

Я использую инъекцию зависимости. В этом-то и дело.

И, наконец, комментарий Жюля об ответе Бенджамина: я не теряю никакой гибкости. Это мой служебный фасад. Я могу добавить столько параметров, GetService<T>сколько захочу, чтобы различать разные реализации, так же, как это делается при настройке контейнера DI. Так, например, я мог бы изменить, GetService<T>()чтобы GetService<T>(string extraInfo = null)обойти эту «потенциальную проблему».


В любом случае, еще раз спасибо всем. Это было действительно полезно. Приветствия.

LiverpoolsNumber9
источник
4
Я не согласен, что у вас есть Сервисный Фасад здесь. GetService<T>()Метод способен (попытка) Решимость зависимостей произвольной . Это делает его Сервисным Локатором, а не Фасадом Сервиса, как я объяснил в сноске моего ответа. Если бы вы заменили его небольшим набором GetServiceX()/ GetServiceY()методов, как предлагает @DocBrown, то это был бы Фасад.
Бенджамин Ходжсон
2
Вы должны прочитать и обратить пристальное внимание на последний раздел этой статьи о Абстрактном сервисном локаторе , который, по сути, является тем, что вы делаете. Я видел, как этот анти-шаблон испортил весь проект - обратите особое внимание на цитату «это ухудшит вашу жизнь как разработчика технического обслуживания, потому что вам нужно будет использовать значительное количество умственных способностей, чтобы понять последствия каждого внесенного вами изменения». "
AlexFoxGill
3
При статической типизации - вы упускаете суть. Если я попытаюсь написать код, который не предоставляет класс, использующий DI с необходимым ему параметром конструктора, он не скомпилируется. Это статическая защита от проблем во время компиляции. Если вы напишите свой код, предоставив вашему конструктору IContext, который не был правильно сконфигурирован для предоставления аргумента, который ему действительно нужен, он скомпилируется и завершится с ошибкой во время выполнения
Бен Ааронсон,
3
@ LiverpoolsNumber9 Ну, тогда зачем вообще строго типизированный язык? Ошибки компиляции - это первая линия защиты от ошибок. Модульные тесты являются вторыми. Ваши также не будут восприняты модульными тестами, потому что это взаимодействие между классом и его зависимостью, поэтому вы окажетесь на третьей линии защиты: интеграционных тестах. Я не знаю, как часто вы их запускаете, но сейчас вы говорите об обратной связи порядка минут или более, а не миллисекунд, которые требуются вашей IDE, чтобы подчеркнуть ошибку компилятора.
Бен Ааронсон
2
@ LiverpoolsNumber9 неверно: теперь ваши тесты должны заполнять IContextи внедрять это, и что особенно важно: если вы добавите новую зависимость, код все равно будет скомпилирован - даже если тесты не пройдут во время выполнения
AlexFoxGill