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
proposal: strings: provide ContainsOnly function #46385
Comments
This proposal lacks detail. What is a ContainsOnly? Can you share links to those "many packages" that implement their own? How does it compare to ContainsAny? |
It looks like i lied. I only saw two packages implement it and both of which are for semantic versioning and they use it to check if I am sorry.
It may look like this. // ContainsAny reports whether s only contains Unicode codepoints which are in chars
func ContainsOnly(s, chars string) bool {
if len(s) == 0 || len(chars) == 0 {
return false
}
outer:
for _, r := range s {
for _, c := range chars {
if r == c {
continue outer
}
}
return false
}
return true
}
// or
func ContainsOnly(s, chars string) bool {
if len(s) == 0 || len(chars) == 0 {
return false
}
for _, r := range s {
if strings.ContainsRune(chars, r) == false {
return false
}
}
return true
} |
I think this could be written as Does this really come up often enough to deserve its own function? Can you point to some existing code that would be simpler if this function were available? Thanks. |
This proposal has been added to the active column of the proposals project |
This comes up, e.g., to test that all characters in an email address are valid, but I think that doing an O(n m) loop is wrong when you can use a lookup map instead. |
@carlmjohnson for small |
The downside of using ContainsOnly for testing a character set is that the character set is very large. Passing "abcdefghijklmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ0123456789" and making the function figure out how to do that seems much worse (and less efficient) than just writing It's even worse if Unicode is involved. It doesn't seem like we have a compelling use case here. |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
strings package has ContainsAny. I have seen many package implement thier ContainsOnly. Although it takes just a few line of code, it would be nice to have it in strings package.
The text was updated successfully, but these errors were encountered: