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

Support dynamic select #7

Merged
merged 4 commits into from
Jul 7, 2024
Merged

Support dynamic select #7

merged 4 commits into from
Jul 7, 2024

Conversation

Kuniwak
Copy link
Contributor

@Kuniwak Kuniwak commented Jul 7, 2024

This PR allows select to accept an array of channels. Previously, there was no way to select an array of channels. This PR only modifies functions related to resultBuilders, so the behavior of the channels will remain unchanged.

@Kuniwak Kuniwak marked this pull request as draft July 7, 2024 20:58
@Kuniwak
Copy link
Contributor Author

Kuniwak commented Jul 7, 2024

I found a mistake, so I am moving it back to draft.

@Kuniwak Kuniwak marked this pull request as ready for review July 7, 2024 21:03
@Kuniwak
Copy link
Contributor Author

Kuniwak commented Jul 7, 2024

I fixed the mistake.

@gh123man
Copy link
Owner

gh123man commented Jul 7, 2024

Hey! Thanks for the contribution. This is a super interesting change and seems like a good addition to the library.

I have a couple of suggestions:

  1. What do you think about calling forEach any instead?

forEach implies an operation occurs for each element - however since select will choose only one operation per call, perhaps any would make more sense since select will chose "any one" channel based on it's availability.

ex:

 await select {
      any([a, b]) {
          receive($0) { await result <- $0! }
      }
  }
  1. I think an overload to accept variadic parameters would compliment this change. ex:
public func any<T>(_ elements: T..., @SelectCollector cases: (T) -> ([SelectHandler])) -> [SelectHandler] {
    return elements.flatMap { cases($0) }
}

would allow us to also perform an operation like this

 await select {
    any(a, b) {
        receive($0) { await result <- $0! }
    }
}

let me know your thoughts.

@Kuniwak
Copy link
Contributor Author

Kuniwak commented Jul 7, 2024

What do you think about calling forEach any instead?

Sounds good. any is better, as you suggested. I renamed it at 2f00850.

I think an overload to accept variadic parameters would complement this change

Also, I agreed. I added it at 9217527.

@gh123man
Copy link
Owner

gh123man commented Jul 7, 2024

Looks great. I'm going to merge this, do some additional testing, and publish this in the next release. Thanks again!

@gh123man gh123man merged commit 394b971 into gh123man:main Jul 7, 2024
2 checks passed
@Kuniwak Kuniwak deleted the dynamic-select branch July 8, 2024 00:13
@Kuniwak Kuniwak mentioned this pull request Jul 8, 2024
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.

2 participants