[swift-users] Feedback for Dependency Injection Framework?

[Tadeas Kriz]

Hi Ian,

Shouldn't the `ViewController_B` be a dependency and not be instantiated
inside a `ViewController_A`? Or ViewControllers/Controllers are not
considered dependencies?

Thanks,
Tadeas

On Thu, Jun 16, 2016 at 10:50 PM Ian Terrell via swift-users <
swift-users at swift.org> wrote:

> Hi Mike,
>
> Thanks for your response. :) Let me address your points. I apologize for
> the length; this is a topic I care about and I know my co-workers are
> reading it. :)
>
> I'm going to edit the message history for brevity.
>
> My first point of confusion is that these libraries seem to be less about
>>> dependency injection and more about service location. For instance, in your
>>> GitHub example in the playground, we see the following:
>>>
>>> struct GithubListMembersServiceImpl : GithubListMembersService {
>>>>     let githubURL: TaggedProvider<GithubBaseURL>
>>>
>>>
>>>>
>>>     func listMembers(organizationName: String, handler: [String] -> ()) {
>>>>         let url = githubURL.get()
>>>
>>>
>>> To my mind it doesn't look like githubURL was injected. It looks like it
>>> was looked up in a service locator.
>>>
>> I'm not very familiar with the service locator pattern to be honest,
>> however, I can assure you that the base URL is being injected. I think
>> there may be some confusion on either how the GithubListMembersServiceImpl
>> is being constructed, or tagged providers in general.
>>
>
> I don't believe I'm confused over those features, and I do understand that
> githubURL is being injected from your point of view. What I mean though is
> that it's being injected from a global location, not from the main call
> site.
>
> In service location code that needs a service asks for it, and the service
> locator provides it. In this case you are asking for a GithubBaseURL. The
> (in my opinion problematic) pattern occurs even if you rewrite the service
> implementation as you did:
>
> struct GithubListMembersServiceImpl : GithubListMembersService {
>>     let githubURL: *NSURL*
>
>
>>
>     func listMembers(organizationName: String, handler: [String] -> ()) {
>>         let url = githubURL
>>
>> .URLByAppendingPathComponent("orgs/\(organizationName)/public_members")
>>         let dataTask = urlSession.dataTaskWithURL(url) ...
>
>
> Definitely from the perspective of GithubListMembersServiceImpl the
> githubURL is injected. That makes it nicely testable and well designed.
> However in this case the service location is merely once removed (and is
> already present in the prior version as well): now your code is asking a
> service locator for the GithubListMembersService (client code will now have
> to call GithubListMembersService.get()).
>
> I see dependency injection as being given your dependencies. I see service
> location as asking for your dependencies. In Cleanse and Dagger and Guice I
> feel like the dependencies are being asked for (the get() method is *asking
> something else* to be provided the call site with an instance).
>
> In this case, I believe your "component" is the service locator.
>
> and then change the binding statement to be
>>
>>
>>>       binder
>>>        .bind()
>>>        .to(value: NSURL(string: "https://api.github.com")!)
>>
>>
>> The downside of this in an actual application is that you may want to
>> inject other URLs. These "Type Tags"  TaggedProviders are essentially
>> just a ...
>>
>
> And this is why I think Dagger and Guice and Cleanse are improperly
> categorized as dependency injection frameworks. I think in reality they are
> service locators and factories. Dependency injection needs no framework!
>
> With manual dependency injection, when different services want different
> URLs you simply inject them normally.
>
> call site A: MembersServiceImpl(baseURL: productionURL)
> call site B: MembersServiceImpl(baseURL: stagingURL)
>
> That requires that the call sites manage that dependency in order to know
> what to pass (feature not a bug).
>
>
>> I've always thought of dependency injection as being a tool for avoiding
>>> global state. But in this case it seems to me like the service locator is
>>> globally managed. The language of service locators is even present in
>>> Guice's excellent motivation page
>>> <https://github.com/google/guice/wiki/Motivation> (my emphasis):
>>>
>>>
>> The only "global" state are things that are declared singletons. (which
>> are still only singletons in the scope of each time one calls build). They
>> have very similar semantics to singletons in Guice
>> <https://github.com/google/guice/wiki/Scopes> (however no custom scopes
>> yet as mentioned above).
>>
>
> Global is not quite accurate, but it is not terribly close. Your Component
> object tree is what I mean, and I view it (perhaps wrongly) as
> "semi-global." I get the sense that in most applications there will end up
> being exactly one. A better word is "external". Now your objects rely on
> external state, whether it is global or semi-global.
>
>
>> For the former, there was feedback regarding the constructors being
>> implicitly made for structs in this reddit thread
>> <https://www.reddit.com/r/swift/comments/4o2lno/squarecleanse_swift_dependency_injection/d49pu6c>
>> .
>>
>
> You pointed out in that thread that these frameworks make it easier to
> keep up with new dependencies, reordered initializer parameters, etc — but
> all that hiding I fear would lead to poor architectural decisions in
> practice. Too many objects end up depending on too many things, because
> those dependencies are hidden from them. That means that real refactors
> (trying to use these objects in un-forethought contexts, etc) become more
> difficult, and your app actually becomes more tightly coupled (it's so easy
> to .get() that other object!).
>
>  Guice will inspect the annotated constructor, and *lookup* values for
>> each parameter.
>>
>> Cleanse actually achieves the same thing, but without reflection. I
>> attempted to explain it in this part of the README
>> <https://github.com/square/Cleanse/blob/master/README.rst#terminating-methods-top1factory-p1---e-1st-arity>,
>> but I'll be honest, I struggled while attempting to make it both simple and
>> detailed enough to convey what's going on.
>>
>
> I think I get what's going on. I only meant that the words "look up" imply
> service location to me. In my mind you're registering (via bind()) service
> factory instructions to the service locator. At runtime, the service
> locator/factory *looks up *how to build it and returns you an instance.
>
> Big snippet below for all the context:
>
> Another point I've heard people talk about is that these libraries help as
>> dependencies grow. I hope this isn't a straw man, as I interpret the
>> following line in your README in that way.
>>
>>>
>>> As the functionality of this app grows, one may add arguments to
>>>> RootViewController and its dependencies as well as more modules to satisfy
>>>> them.
>>>>
>>>
>>> The argument seems to go that this is easy to maintain:
>>>
>>> init(a: A)
>>>
>>>
>>> But this is difficult:
>>>
>>> init(a: A, b: B, c: C, d: D, e: E, ...)
>>>
>>>
>>> The argument goes further by saying that it becomes especially difficult
>>> as you may need to pass the dependencies through the some nodes of the
>>> object graph that seem not to need them. For instance, in the object graph
>>> A -> B -> C, if C has a dependency on Foo but B does not, why should B know
>>> about Foo?
>>>
>>> But I find that argument unconvincing on two grounds. First, I believe
>>> an object's dependencies are the union of its and its children's
>>> dependencies. In the example above, B has an implicit dependency on Foo.
>>> "Skipping" passing them around or instantiating them in the owning class is
>>> actually only hiding an object's true dependencies by using global state.
>>> Second, as a single object's dependencies become more unwieldy it seems in
>>> practice to indicate an architectural problem where objects are doing too
>>> much. These problems are related, as the architectural problem may not be
>>> as visible when the true dependencies are hidden!
>>>
>>
>> So this is an interesting point. I'm a strong believer that DI solves
>> this issue really well.
>>
>> Let's take this example (I think its similar to the straw man you are
>> referring to):
>>
>> https://gist.github.com/mikelikespie/c54a017677265322df7eb785d9f36345
>>
>> Basically, VC_A, needs serviceA, and a VC_B to do its job, however, for
>> it to create VC_B it needs VC_B's dependencies + its transitive
>> dependencies which happen to be serviceA, serviceB, serviceC.
>>
>> So I agree with the arguments made in the straw man, e.g. don't need to
>> change the constructor hierarchy all the way down when VC_D needs a new
>> service or needs to start depending on something. It takes a pretty large
>> example to start to see these benefits, but its kind of like how you don't
>> really see a benefit from a quicksort over a bubble sort until you have a
>> lot of items to sort.
>>
>> So if we refactor as is (without turning anything into protocols) to use
>> Cleanse, we come up with something like:
>>
>> https://gist.github.com/mikelikespie/3abe371bf7f7ab67f71d6cfa22c0145d
>>
>> Now, say serviceD incurred a new dependency, one would just add it to its
>> constructor and make sure the dependency is configured... don't have to
>> change any of the other view controllers in the hierarchy.
>>
>
> Ok! So, we're on the exact same page with the code and the problem, and
> perhaps even the same conclusion, but the exact opposite solutions.
>
> I'm a strong believer that DI solves this issue really well. ... It takes
>> a pretty large example to start to see these benefits
>>
>
> I believe that true dependency injection is the solution, but I think that
> Cleanse/Dagger/Guice aren't really providing that in a useful way. I think
> the refactored version using Cleanse is significantly worse than the
> non-refactored version. Additionally, I think the larger the project (in
> your view where it shines), the worse the problem gets (in my view where it
> hinders).
>
> Concretely: In both examples, ViewController_A has dependencies on
> services A, B, C, and D. But only in the non-Cleanse version is that
> visible. This means that it has hidden, non-explicit dependencies, that
> have to be looked up somewhere else. And they're looked up from the service
> locator/factory VC_A_Component. We cannot use ViewController_A outside of
> the context of that special knowledge and setup.
>
> These frameworks aren't removing dependencies, they're hiding them. This
> makes it less easy to reason about the code.
>
> As an example of reasoning, a reader of the component/service locator
> setup in the Cleanse example has to wonder: Why am I binding Services A, B,
> C, and D and VCs A, B, C, and D just to create a single VC_A? Now the
> reader has to inspect all of the view controllers to find out! After which
> she learns, well, VC_A uses S_A and can create a VC_B, which uses S_B and
> can create a VC_C, which uses S_C and can create a VC_D, and VC_D uses S_D.
> All of that information must be discovered and tied together just to get()
> a single VC_A!
>
> In the non-Cleanse example, the question is: Why do I need to instantiate
> VC_A with Services A, B, C, and D? And the answer is simple and comes from
> a single view controller (the one we care about): because VC_A uses S_A can
> create a VC_B and VC_B uses services B, C, and D.
>
> That's a huge contrast in reasoning, and it's because Cleanse/Dagger/Guice
> are hiding information in external service locators.
>
> I suppose I might phrase my point of view as saying that I don't believe
> in transitive dependencies; and definitely not in treating them
> differently. You have your dependencies, and by depending on something else
> that has dependencies, those transitive dependencies are de facto yours,
> directly. They get no special treatment, because they cannot; treating them
> specially relies on external state. That is breaking encapsulation.
>
>
>> This gets me to the next point, testing. VC_A's purpose is to call
>> serviceA and present a view controller. To properly unit test its
>> functionality, it may not be necessary to provide a concrete implementation
>> of the VC it wants to present. In general if one can test a component and
>> its functionality correctly with less dependencies that's better.
>>
>
> I would never advocate anything that wasn't properly and nicely testable.
> I take your point that it seems simpler to test in this setup, but I don't
> really believe it's simpler. Again, that's because the true dependencies
> are hidden.
>
> In the case of manual dependency injection as I advocate, the solution is
> protocols and empty implementations. I agree that empty implementations of
> protocols can be annoying to maintain, but I find they solve the problem
> very well in a clean and "dumb" manner.
>
> For instance, to test VC_A, assuming all the services were protocols, you
> could create an instance solely to test its service A functionality like so:
>
> let testedVC = ViewController_A(serviceA: TestingServiceA(), serviceB:
> DummyServiceB(),
>                                                      serviceC:
> DummyServiceC(), serviceD: DummyServiceD())
>
> https://gist.github.com/ianterrell/c86aa1ad64453a214966ac81b31f75e9
>
> I do think there is value in aiding with property injection for the cases
>>> where we can't use initializer injection (we do often use storyboards at my
>>> company, although we try to keep them small and unwieldy!), but I think
>>> those cases are solved with a few clever extensions. See post-script if
>>> you're curious.
>>>
>>
>> Cleanse does support property injection
>> <https://github.com/square/Cleanse#property-injection>. My examples in
>> the readme only really demonstrate using it for the AppDelegate, but it
>> also makes sense for storyboard injection. It is very explicit though (less
>> magic than other DI frameworks). I also think there would be room for a
>> cleanse extension that specifically deals with storyboards. (I kinda like
>> what RxSwift did with RxCocoa).
>>
>
> Yes, I saw it. I think I only meant that I do see manual property
> injection with UIKit classes as sometimes cumbersome, but I believe there
> are better solutions than frameworks like these.
>
> I don't believe I've misinterpreted your project or it's documentation. I
> anticipate that we may simply believe in different solutions to the hard
> problem or dependency management. In that case I leave this conversation
> happily agreeing to disagree, just as I do with some of my other colleagues
> here. :) But at least I hope I've well explained why I personally view
> these solutions as detrimental.
>
> Thanks,
> Ian
>
> _______________________________________________
> swift-users mailing list
> swift-users at swift.org
> https://lists.swift.org/mailman/listinfo/swift-users
>
-- 
Hello majk!
das
d
asd
as
dasd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-users/attachments/20160616/7a579280/attachment.html>