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

Make Auto Injection Optional #212

Closed
mohamede1945 opened this issue Oct 18, 2018 · 3 comments
Closed

Make Auto Injection Optional #212

mohamede1945 opened this issue Oct 18, 2018 · 3 comments

Comments

@mohamede1945
Copy link

mohamede1945 commented Oct 18, 2018

Because of the current implementation of Auto-injection I think we should provide a way to turn it off. It is implemented by going through all properties of each object being created including super class ones because of that big projects with large number of dependencies and objects with many properties tend to take time to resolve such object graphs because of auto-injection overhead.

I would suggest you can have a way to completely turn it off like:

let container = DependencyContainer(autoInjectionEnabledByDefault: false)

Users can still override the default behavior for individual registrations.

container.register(.unique, autoInjectionEnabled: .enabled) { Client() }

Edit:
The initializer would default to true

class DependencyContainer {
    init(autoInjectionEnabledByDefault: Bool = true) { }
}

And the register would use an enum, defaulting to useDefault:

enum AutoInjectionEnabled {
  case useDefault
  case enabled
  case disabled
}

func register<T>(_ scope: ComponentScope = .shared, type: T.Type = T.self, tag: DependencyTagConvertible? = nil, autoInjectionEnabled: AutoInjectionEnabled = .useDefault, factory: @escaping (()) throws -> T) -> Definition<T, ()> {
    let definition = DefinitionBuilder<T, ()>
@ilyapuchka
Copy link
Collaborator

@mohamede1945 that's a good point, but this will be a breaking change for current behavior, so I'd say the default should be true.

@ilyapuchka
Copy link
Collaborator

@mohamede1945 There is now a parameter in container configuration and a method to override it on a single definition, i.e.:

let container = DependencyContainer(autoInjectProperties: false)
container.register { ServerImp() as Server }
container.register { ClientImp() as Client }
  .autoInjectingProperties(true)

@mohamede1945
Copy link
Author

That's perfect. Thanks for implementing it.

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