-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: strings: add a Join method to Builder #49546
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
Comments
Bonus question: should bytes mirror this api? |
It would help to see some examples of real code that would use this new feature. As you say, it's pretty easy to write your own loop, and that loop could be in an imported third party package. Does this happen often enough to make it useful to add to the standard library? https://golang.org/doc/faq#x_in_std |
It seems like this proposal is primarily a workaround for a “deficiency” in the compiler—that it cannot see that the outer strings.Builder is redundant and eliminate it (probably because strings.Join can’t be inlined?). Looking at my own code, I think the pattern of creating a temporary buffer ([]byte, strings.Builder, etc.) then appending/writing it to a larger buffer is actually fairly common. Perhaps it would be better to update the compiler (somehow) instead of adding a new, very specific API. |
First, I have nothing against keeping this additional API in a separate package. I just came across this and felt it can help a bigger audience. One example would be SQL query building. I've condensed something simplified in a single function, for demonstration purpose: To build a query like:
Using the existing API:
With the proposed API:
Benchmarks:
|
A more general solution would be a strings/bytes.Join variant that took an io.Writer: package strings
func WriteJoin(w io.Writer, elems []string, sep string) (n int64, err error)
// and similarly for bytes That wouldn't have to live in the stdlib either but if it did I would certainly use it a lot. |
I was going to propose something very similar, @jimmyfrasche (Bikeshedding: Edit: Fix the bikeshed's name. |
After doing some more investigations, I have concluded that it is indeed better to create and maintain my own package with specific use cases, which sometimes is more than just Thanks everyone for taking the time to respond. Leaving this issue open for now, in case people will still want to vote to include the |
This proposal has been added to the active column of the proposals project |
Based on the discussion above, this proposal seems like a likely decline. |
I wrote https://pkg.go.dev/github.com/jimmyfrasche/jointo so there's something to look at and because I'm sure I'll want to use it if this proposal is declined. |
No change in consensus, so declined. |
In my original PR, the name I used ( |
Sometimes, I find myself wanting to join a slice of strings and append the result to a
strings.Builder
. This can be done by:Builder.WriteString(strings.Join([]string{"foo", "bar"}, ", "))
, less coding, but results in 1 extra allocation.strings.Join
function.Currently,
strings.Join()
already uses astrings.Builder
. This issue proposes to move the code from this function into a method ofstrings.Builder
. The currentstrings.Join()
function will become a wrapper which allocates a new builder and calls this new method for further processing.Example (adjusted copy of
strings.Join()
):Benchmarks code:
Benchmark run:
Items open for discussion:
Join
,AppendJoin
orWriteJoin
?).WriteString()
?error
, as returned byWriteString()
? (Is alwaysnil
)Related: PR #42850, which got put on hold for not going trough the proposal process (and duplicates code)
The text was updated successfully, but these errors were encountered: