-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Ability to resolve multiple implementations of same type #154
base: master
Are you sure you want to change the base?
Conversation
So the first question is why? I'm not sure of the use case or what problem this will solve. Secondarily, if you're nesting containers for mocking or modularization, then the current code only gets the first set of items it finds. It doesn't get "all" of them. |
Valid question! To your first one: being able to resolve multiple instances of the same registered type is something I am used to when working with .Net. Microsofts Dependency Injection framework and Autofac provide options for that. Good catch on the nested containers, will look into that. |
…tions of some kind
@hmlongco I addressed your feedback about the nested containers. Can you please review? |
Went through this again and found that it still suffers from a bug when handling multiple containers. Functionally this mechanism gathers up all of the named containers and stuffs them into an array so you can call them as needed. Right? So what happens if container B is a child of container A... and container A has a named registration of "Fred" that was supposed to override the Fred in container B? In the current implementation both Freds will be gathered, returned and executed... and I don't think that's the desired result. |
…orrectly respecting the container hierarchy.
Didn't consider that. Made some additions and adjustments. @hmlongco |
Terribly sorry for the delay. Been working on getting Factory, my latest DI system, out the door. I'll put this in if you still want it, but one more change is needed; Throwing a fatal error if nothing is found doesn't make sense. Just return an empty array. |
Sources/Resolver/Resolver.swift
Outdated
if let registrations = root.lookupAll(type) { | ||
return registrations.compactMap { reg in return reg.resolve(resolver: root, args: args) } | ||
} | ||
fatalError("RESOLVER: '\(Service.self)' types not resolved.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return []
Sources/Resolver/Resolver.swift
Outdated
if let registrations = lookupAll(type) { | ||
return registrations.compactMap { reg in return reg.scope.resolve(registration: reg, resolver: self, args: args) } | ||
} | ||
fatalError("RESOLVER: '\(Service.self)' types not resolved.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return []
…e found for looking up multiple implementations and fixed registration key
Addressed your PR review comments @hmlongco Sorry for my delay this time (was on PTO). |
Hello, we need that too! @hmlongco did you have time to review the PR? |
这是来自QQ邮箱的自动回复邮件。
邮件已收到
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No actually
This PR adds the ability to resolve all registered implementations for a given type, where the implementations are differentiated by names.
Please let me know what I can optimize.