-
Notifications
You must be signed in to change notification settings - Fork 157
Remove unnecessary struct extension #26
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
Conversation
150ea69
to
939c634
Compare
Hi @oronbz, the docs may just be a little confusing here, but technically that passage is intended. It's trying to demonstrate the differences between the struct-style of dependencies and the protocol-style. In the struct-style you construct values to represent instances of the dependency, whereas in the protocol-style you create types that conform. that when one uses structs to model dependencies, in order to construct "instances" of the dependency you construct values of the struct rather than create types that conform to a protocol. I'll think of ways to make that clearer. |
Yes, but it seems both of these sections: extension AudioPlayerClient {
static let live = Self(/* ... */)
static let mock = Self(/* ... */)
static let unimplemented = Self(/* ... */)
} extension AudioPlayerClient: DependencyKey {
static let liveValue = Self(/* ... */)
static let previewValue = Self(/* ... */)
static let testValue = Self(/* ... */)
} Are showing the exact same point that you mentioned, while the second one will satisfy the default dependencies which is probably the best (better?) practice. I was confused as do why would I even want to implement the first example and found no use of it, since the second one already accomplish both the struct way of defining a dependency and implementing In addition, I believe this page https://pointfreeco.github.io/swift-dependencies/main/documentation/dependencies/designingdependencies, would also benefit from adding the |
Let me take a quick pass at fleshing out what I had in mind for that passage. I do want to be a little more explicit. |
Hi @oronbz, I just pushed an edit to your branch. Can you let me know if that clarifies things? I also took your suggestion to add the |
@mbrandonw I was about to respond that it was even worse now IMO, but after the latest commit, it seems a bit clearer now, with more examples. |
Also, thinking out loud here, but I wonder how to incorporate the It was the first thing that I've tried to look for without any luck, and pretty common use case in any DI library/solution. |
We have plans to write up more documentation about that pattern. We left it out because we wanted to understand the ins and outs, but we now have good test coverage on it and we use that style out in the wild. I will merge this PR as-is and we will get to that topic sometime soon. |
This may be a leftover or an oversight, but correct me if I'm wrong, this docs part is unrelated, and the
DependencyKey
extension
should be enough.