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 < [email protected]> 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 > [email protected] > https://lists.swift.org/mailman/listinfo/swift-users > -- Hello majk! das d asd as dasd
_______________________________________________ swift-users mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-users
