Skip to content

Conversation

oronbz
Copy link
Contributor

@oronbz oronbz commented Jan 17, 2023

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.

@mbrandonw
Copy link
Member

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.

@oronbz
Copy link
Contributor Author

oronbz commented Jan 17, 2023

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 live/preview/test values. I believe that a beginner such as I will be confused as well, and just want the straight forward way of creating a dependency.

In addition, I believe this page https://pointfreeco.github.io/swift-dependencies/main/documentation/dependencies/designingdependencies, would also benefit from adding the extension DependencyValues definition, and again, a beginner might be confused from trying to create a dependency by using this doc page and wondering why it won't work (because he/she didn't implement DependencyValues). So if you'd like, I could add this to the PR as well.

@mbrandonw
Copy link
Member

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.

@mbrandonw
Copy link
Member

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 DependencyValues computed property. Thanks!

@oronbz
Copy link
Contributor Author

oronbz commented Jan 17, 2023

@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.

@oronbz
Copy link
Contributor Author

oronbz commented Jan 17, 2023

Also, thinking out loud here, but I wonder how to incorporate the @Dependency inside a Dependency example that I gave here: https://github.com/pointfreeco/swift-dependencies/pull/27/files to one of the main topics, maybe here using AVAudioEngine as a dependency?

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.

@mbrandonw
Copy link
Member

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.

@mbrandonw mbrandonw merged commit 58339f5 into pointfreeco:main Jan 17, 2023
@oronbz oronbz deleted the docs_struct_fix branch January 17, 2023 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants