Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-injection #14

Closed
ilyapuchka opened this issue Nov 24, 2015 · 6 comments
Closed

Multi-injection #14

ilyapuchka opened this issue Nov 24, 2015 · 6 comments

Comments

@ilyapuchka
Copy link
Collaborator

Sometimes it can be useful to register several implementations for the same protocol and resolve them as an array. Imagine that you have different authorisation services, for instance via Facebook, Twitter and Google, that all implement a common protocol, and you want to present the list of all possible services so that user can choose one of them but your code does not care what concrete service he chose, you just work with it via that common protocol.

With Dip we can do that only with tags, associating different implementations of the same protocol with different tags and resolve them using the same tags. Though it works it introduces coupling between independent parts of the system - point of registration and points of resolution.

protocol OAuthService {
    func doLogin()
}

class FacebookOAuth: OAuthService {
    func doLogin() {
        print("login with facebook")
    }
}

class TwitterOAuth: OAuthService {
    func doLogin() {
        print("login with twitter")
    }
}

class GoogleOAuth: OAuthService {
    func doLogin() {
        print("login with google")
    }
}

let container = DependencyContainer { container in
    container.register(tag: "facebook") { FacebookOAuth() as OAuthService }
    container.register(tag: "twitter") { TwitterOAuth() as OAuthService }
    container.register(tag: "google") { GoogleOAuth() as OAuthService }
}

let services: [OAuthService] = [
    container.resolve(tag: "facebook"),
    container.resolve(tag: "twitter"),
    container.resolve(tag: "google")
]

services[Int(arc4random_uniform(UInt32(3)))].doLogin()

Other IoC containers (like Ninject for .Net) has this feature. The obvious consequence of allowing multiple bindings to the same protocol is that how you resolve them not as an array but as a single instance, what binding (definition in case of Dip) you should choose. Ninject offers different styles of attributes or runtime context checks that are very specific for .Net. If there is no constraints that resolution result is ambiguous and exception is thrown.

Do we need such feature in Dip and if yes than how we should solve the ambiguity problem?

@ilyapuchka
Copy link
Collaborator Author

Looks like the best way is to continue to use tags to register individual definitions, but to ignore tags in separate definitions pool. When client resolves and instance he should provide tag, to resolve all registered definitions it uses separate method resolveAll (can't use resolve<T>()->[T] cause it will lead to ambiguity) that ignores tag. Here is the implementation - 4e01d85

@AliSoftware
Copy link
Owner

I'm not sure that feature is worth implementing, it feels like making the code more complex (I really want to keep the codebase simple at least have a good tradeoff between useful features and simple implementation) for little use.

In your use case example, I'm not sure I see how the current implementation is not capable of handling this, e.g. for that use case I'd probably do something like this:

protocol OAuthServicesListAPI {
  var services: [OAuthService] { get }
}
struct OAuthServicesList: OAuthServicesListAPI {
  let services: [OAuthService] = [
    container.resolve(tag: "facebook"),
    container.resolve(tag: "twitter"),
    container.resolve(tag: "google")
  ]
}
container.register(.Singleton) { OAuthServiceList() as OAuthServiceListAPI }

Then when I need the list simply (container.resolve() as OAuthServiceListAPI).services

@ilyapuchka
Copy link
Collaborator Author

Yes, with current implementation it's possible to achieve the same results but that will require either additional definition as you pointed out or to call resolve several times manually. I was thinking about making it possible to do this automatically without any changes in the way how you register components (you still need to use tags to be able to resolve them individually)

Why do you think this will make syntax more complicated? That only adds resolveAll method family and it looks absolutely the same way as resolve. Also implementation wise it does not change anything in standard register/resolve flow except adding new definitions pool.

I squashed those changes in one commit to see diff more clearly - 6b64a2d

@AliSoftware
Copy link
Owner

Well actually now that I think about it my concern is not really about implementation being complicated but more the API getting too rich, like for any new user seeing all those resolveXXX methods, they may feel lost and not know which to use. That started to be a concern for me with the circular dependency PR as for a new comer to DI that might frighten them, and I wanted to keep the API simple. Maybe my concern is not about just this PR but that I don't want Dip to end up being a huge machinery for the user with an API that is so rich it may scare people new to DI.

Will take another look tonight.

@ilyapuchka
Copy link
Collaborator Author

Yes, I understand. The compromise can be if we make Dip container extensible and include all the features that we are not sure if they should be present in Dip in a separate repository like DipExtensions. Auto injection probably also can go there this feature, support for UI, that I will start to work on very soon, and few other new features I already have in a pocket, but I'm afraid you would not like to include them in Dip =) I think it would be cool to provide such opportunity for extensions so that we can see what people will do with it. Maybe it worths opening a separate issue to discuss that?

@AliSoftware
Copy link
Owner

I like the idea of having huge features, like UI support separated in a dedicated repo (let me know if you want me to create it and give you push access, would probably make more sense to be side by side with this repo). I think that Multiple Bindings PR is light enough to be in Dip itself though, but for stuff like AutoInjection we might wonder if that's not worth separating it.

@ilyapuchka ilyapuchka added this to the 4.1.0 milestone Nov 26, 2015
@ilyapuchka ilyapuchka changed the title Multiple bindings Multi-injection Nov 26, 2015
This was referenced Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants