-
Notifications
You must be signed in to change notification settings - Fork 2
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
Create WeakResolver #97
Conversation
Sources/KnitLib/WeakResolver.swift
Outdated
public var isAvailable: Bool { return container != nil } | ||
|
||
public var strongResolver: WeakAwareResolver { unwrapped } | ||
public var optionalResolver: WeakAwareResolver? { container } |
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.
What are these properties used for?
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.
I think this is to allow for specific use cases to access the underlying container if they are willing to take responsibility of deallocating correctly. Example would be some short-lived service on log out. I don't think it will likely be needed but it is good to have in case.
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.
Adding some documentation to both properties to make explicit the responsibility of the consumer to correctly manage life cycle would be good
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.
isAvailable
: Can I do the thing?
optionalResolver
: Do the thing if possible
strongResolver
: The thing has to happen, I'm aware of the risks (now removed)
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.
Looks great!
I think we should consider removing the strongResolver
property however
d6244ee
to
864db8a
Compare
Background
It is expected that parts of the dependency graph will directly hold onto a
Resolver
as they need to resolve services after initialisation. (e.g. pushing a new view controller). This creates the potential that if the object holding the Resolver leaks then so does the Resolver. While this can be detected, it is difficult to pinpoint the cause of the issue without the aid of debug tooling.Solution
This PR adds the concept of a
WeakResolver
which holds a weak reference to the underlyingContainer
which works as a Resolver until the Container has been deallocated at which point any attempt to resolve a service will cause a crash.