-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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
x/exp/slices: add ContainsFunc #53983
Comments
This operation is very common. It also often goes by the name It'd also be really nice to have the dual, |
This proposal has been added to the active column of the proposals project |
It sounds like the main justification would be that we have Index and Contains (which wraps Index), and we have IndexFunc but not ContainsFunc. So adding ContainsFunc would complete the set. @bradfitz reports that in the Tailscale source tree there are four implementations of ContainsFunc, only one generic. So that argues for adding ContainsFunc.
Sorry, I confused myself with #54386. We don't have it already. But probably keeping the naming pattern going does make sense. |
<tangent>
or perhaps it would be
The latter is most analogous to what Common Lisp calls 'some'. Writing out those type signatures, I notice the collision between 'Any' and 'any', which have very different meanings. That makes me think maybe the function should have a different name. Maybe Some. If anyone (someone?) wants to push further on this, feel free to file a separate proposal issue. |
I'd like to push a little bit to consider using I'd call the (T, ok) flavor of this |
Personally, I would like to see these signatures:
It's pretty convenient to use
|
I appreciate the elegance of Any and All, but we have the analogy already set up: Index is to Contains as IndexFunc is to ___. The answer is clearly ContainsFunc. We should use the name people already familiar with the other APIs will expect if we're going to do this. (In strings and bytes, we also have IndexAny and ContainsAny, which is something else entirely.) |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
x/exp/slices now has ContainsFunc (golang/go#53983) so we can delete our versions. Change-Id: I5157a403bfc1b30e243bf31c8b611da25e995078 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
x/exp/slices now has ContainsFunc (golang/go#53983) so we can delete our versions. Change-Id: I5157a403bfc1b30e243bf31c8b611da25e995078 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This work was completed by https://go.dev/cl/417374. |
x/exp/slices now has ContainsFunc (golang/go#53983) so we can delete our versions. Change-Id: I5157a403bfc1b30e243bf31c8b611da25e995078 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
During the design of the
slices
packageContainsFunc
function was dropped at the time of discussion (#47203 (comment)). Briefly, the discussion was about the signature of the function and how often it is used.I suggest finally making
ContainsFunc,
since we haveContains
, so it will be consistent to haveContainsFunc
as well. Also, it's just convenient to have such a function. Talking about the signature, for me, it's reasonable to make it the same as forIndexFunc
:ContainsFunc[E comparable](s []E, f func(E) bool) bool
Note:
I've created a PR for this change because I'm new here and didn't know that such change needs a proposal. In case you are curious – here it is: golang/exp#39
The text was updated successfully, but these errors were encountered: