hmlongco / Factory

A new approach to Container-Based Dependency Injection for Swift and SwiftUI.
MIT License
1.83k stars 115 forks source link

`Singleton` scope when used with a `ParameterFactory` #79

Closed lawmaestro closed 1 year ago

lawmaestro commented 1 year ago

I guess this isn't a bug per se but something which caught me ought as it was unexpected behaviour, so I wanted to call it out to get a view on it and whether it's perhaps an oversight / something which could potentially be added?

I recently started using ParameterFactory in the following way:

enum InputParam {
  case param1
  case param2
}

static let testDependency = ParameterFactory<InputParam, DependencyType>(scope: .singleton) {
  DependencyConcreteType(input: $0)
}

My expectation here was that the resolver would provide me back with the same instance of the DependencyConcreteType for each resolution when using param1 as the input and a different instance when resolving with the param2 input. However, this doesn't appear to be the case as the identifier (GUID) is created against the registration and this doesn't account for what params have been specified. So regardless of what the input param is set to, the resolver will return the same instance for any resolution. Is this intended behaviour? Thanks!

hmlongco commented 1 year ago

It's the expected behavior and actually documented as such.

While your case is relatively straightforward, if you stop to think abut it you'll realize that it would be extremely difficult to accomplish in practice since the parameter(s) could be anything. Your enumeration, an image, a novel, or whatever.

That being the case, attempting to use it as a cache key would be problematic. You might be able to do it if I required the parameter to be hashable, but that would be an unnecessary constraint on other parameter types. Attempting to add some sort of parameter conformance might work, but could also lead to unwanted cache expansion and other problems.

At any rate, I'll mull over it it.

And thanks for working with Factory.

lawmaestro commented 1 year ago

Thanks for the response. I figured it was probably the case but wanted to call it out even if just to throw an idea into the mix for potential future changes.

I haven't spent too much time thinking about what the implementation could look like but it didn't feel beyond the realms of possibility. I was thinking along the lines of hashable conformance for ParameterFactory params, as you've suggested. If this constraint feels like too much of a burden to place on people using the ParameterFactory then I guess introducing a new type such as DiffableParameterFactory could be a potential option.

Either way, introducing this kind of behaviour under the umbrella of the singleton scope would be a little odd on reflection as it would be deviating away form the definition of a singleton, perhaps more true to the definition of sharedInstance 😉

Thanks again for taking this into consideration and thanks for writing Factory 🙂

hmlongco commented 1 year ago

I'm closing this as it's currently behaving as designed, but I'll also consider looking into alternative behavior for a future release.