Proposal: net/url: add Values.Decode

[mvdan]

Right now, if one wants to decode an URL query string into an existing non-nil Values map, the only way is something like:

values2, err := url.ParseQuery(query)
if err != nil {
    ...
}
for key, value := range values2 {
    values[key] = append(values[key], value...)
}

This means an extra allocation, and the piece of boilerplate seems like it should be unnecessary.

I think this would be much nicer, to mirror the Encode() string method we already have:

if err := values.Decode(query); err != nil {
    ...
}

The implementation would be trivial. We already have a parseQuery(m Values, query string) (err error) function, so it could be moved to the method, and ParseQuery could use the method directly.

[rsc]

Seems reasonable overall. At least two questions that need to be decided though:

• Does Decode clear the map first?
• If not, does it replace existing entries or does it append to them?
• Any others?

[mvdan]

I'd make this work similar to other encoders like encoding/json. That is, if any data is already present, the decoding inserts/appends, instead of replacing entire values.

[rsc]

/cc @bradfitz for thoughts

[bradfitz]

The behavior on duplicate values seems kinda arbitrary. Are we sure that's what others would usually want?

And is this more for performance or for convenience?

If performance, what about the case when you only want a certain field and don't care to do the allocations for any others? The most general API might be callback-based where a user func can choose which to decode (perhaps even lazily decoding keys and/or values) and when to stop iteration.

But if it's just convenience, sure. But I might choose a more verbose name to make it clear what the overwrite-vs-append behavior is.

[mvdan]

This isn't for performance; I didn't even benchmark or measure anything. It's convenience; I found this somewhat simple use case needed an unexpected amount of boilerplate.

Are we sure that's what others would usually want?
I might choose a more verbose name to make it clear what the overwrite-vs-append behavior is.

Hmm, if we're not sure about this, then it's probably best to reject this proposal. I'd prefer the user having to write an extra five lines, over us having a new bit of API that needs careful documentation so that users know what it does. The user's five extra lines would already obviously show what the code is meant to do.

[rsc]

The previous comment by @mvdan (the original author) suggests declining this proposal.
Likely decline; leaving open for a week for final comments.

[rsc]

No comments, so proposal declined.