hmlongco / Factory

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

Parameter factory cache does not take parameters into account #183

Closed doozMen closed 8 months ago

doozMen commented 8 months ago

Conder this

For a container that uses caching

var barLD: ParameterFactory<TC.SystemColorScheme, String> {
    ParameterFactory(self) { condition in
      let current = TC.shared.colorScheme()
      defer { TC.shared.colorScheme.register { current } }

      switch condition {
      case .light:
        TC.setColorScheme(.light)
        return TC.shared.bar()
      case .dark:
        TC.setColorScheme(.dark)
        return TC.shared.bar()
      }
    }
  }

This when instantiate

    public init(_ container: ManagedContainer, key: StaticString = #function, _ factory: @escaping (P) -> T) {
        self.registration = FactoryRegistration<P,T>(key: key, container: container, factory: factory)
    }

So the key here does not take into a account he parameter names, so caching will not work as it will for different parameters not dismiss the cache but return the same value.

So the test below fails when you use caching but when it is unique it succeeds


  func testConditionalFoo() {
    XCTAssertEqual(TC.shared.barLD(.light), "light")
    XCTAssertEqual(TC.shared.barLD(.dark), "dark")
  }
hmlongco commented 8 months ago

Not seeing where you're caching here, but that's true, and mentioned in the documentation for ParameterFactory.

"Finally, if you define a scope keep in mind that the first argument passed will be used to create the dependency and that dependency will be cached. Since the cached object will be returned from now on any arguments passed in later requests will be ignored until the factory or scope is reset."

Since parameters can be anything (simple variables, structs, classes) turning them into keys could well generate a near-infinite set of values.

If you have a limited set of values you could use sub-factories that cache individually and call them from an uncached ParameterFactory.

doozMen commented 8 months ago

Ok so it is thé expected behavior and logical with your explanation. Then you can close the issue