r.FormValue method in http package should be improved with a little change

[Jinpu Hu]

I'm a golang newbie, I think r.FormValue method in http package should
be improved with a little change.

some relative source code:

http/request.go
if r.Form == nil {
r.Form = make(url.Values)
}
// Copy values into r.Form. TODO: make this smoother.
for k, vs := range newValues {
for _, value := range vs {
r.Form.Add(k, value)
}
}

Type of r.Form is url.Value

func (r *Request) FormValue(key string) string {
if r.Form == nil {
r.ParseMultipartForm(defaultMaxMemory)
}
if vs := r.Form[key]; len(vs) > 0 {
return vs[0]
}
return ""
}


url/url.go
Type of url.Values is map[string][]string

func (v Values) Get(key string) string {
if v == nil {
return ""
}
vs, ok := v[key]
if !ok || len(vs) == 0 {
return ""
}
return vs[0]
}

func (v Values) Add(key, value string) {
v[key] = append(v[key], value)
}

First, r.FormValue can use r.Form.Get(key) instead of more code block;
Second, like multiply checkbox with same name, xxx for example,
r.Form[xxx] will be []string{xxx_value1, xxx_value2}, i think
r.FormValue should return all values rather than fist.

maybe:

add a new methods

func (r *Request) FormValues(key string) []string {
if r.Form == nil {
r.ParseMultipartForm(defaultMaxMemory)
}
if vs := r.Form[key]; len(vs) > 0 {
return vs
}
return nil
}

or change FormValue method , use strings.Join with some special
separate.

func (r *Request) FormValue(key string) string {
if r.Form == nil {
r.ParseMultipartForm(defaultMaxMemory)
}
if vs := r.Form[key]; len(vs) > 0 {
if len(vs) == 1 {
return vs[0]
} else {
strings.Join(vs, ",") // use , sep for example
}
}
return ""
}

[Brad Fitzpatrick]

Please start the discussion with words and not code.  (or at least code preceded by words)

What's the problem?

[Jinpu Hu]

First, r.FormValue can use r.Form.Get(key) instead of more code block;

Second, like multiple checkboxs with same name, xxx for example,

r.Form[xxx] will be []string{xxx_value1, xxx_value2}, i think

r.FormValue method should return all values rather than fist.

maybe:

add a new method FormValues

func (r *Request) FormValues(key string) []string {
    if r.Form == nil {
         r.ParseMultipartForm(defaultMaxMemory)
    }
    if vs := r.Form[key]; len(vs) > 0 {
         return vs
    }
    return nil
}

or change FormValue method , use strings.Join with some special
separate.

func (r *Request) FormValue(key string) string {
    if r.Form == nil {
         r.ParseMultipartForm(defaultMaxMemory)
    }
    if vs := r.Form[key]; len(vs) > 0 {
         if len(vs) == 1 {
              return vs[0]
         } else {
              strings.Join(vs, ",")  // use , sep for example
         }
    }
    return ""
}

--
website: Blog

github: Project

[Brad Fitzpatrick]

On Tue, Nov 8, 2011 at 7:27 PM, Gus Soden <augus...@rocketmail.com> wrote:

The proposed FormValues method does not add a lot of value because the application can get multiple values by writing

 vs := r.Form[key]

after ensuring that the multipart form is read. 

Exactly.

We're trying to make the http package smaller & simpler recently, not adding too much stuff, unless it's clear & very painful or impossible to do otherwise.

[André Moraes]

I have been using the http package very intensily and never used FormValue.
Every time I need a value I just call: req.ParseForm and then simply use req.Form.

From what I can see, the FormValue function could be removed from http package,
maybe it is still there for some backward reason or another reason that I really can't see.

--
André Moraes
http://andredevchannel.blogspot.com/

[André Moraes]

2011/11/9 André Moraes <and...@gmail.com>

I have been using the http package very intensily and never used FormValue.
Every time I need a value I just call: req.ParseForm and then simply use req.Form.

Also,

If the user want's 0 or 1 values, he can just call
req.Form.Get(<key>)
Which is another reason to remove FormValue function.

[Jinpu Hu]

Thanks, Brad Fitzpatrick and Gus Soden.

>
> We're trying to make the http package smaller & simpler recently, not
> adding too much stuff, unless it's clear & very painful or impossible to do
> otherwise.

So, a small change will make r.FormValue method better.

func (r *Request) FormValue(key string) string {
     if r.Form == nil {
          r.ParseMultipartForm(defaultMaxMemory)
     }

/*

     if vs := r.Form[key]; len(vs) > 0 {
          return vs[0]
     }
     return ""

*/
return r.Form.Get(key)
}

[Jinpu Hu]

Thanks, Brad Fitzpatrick and Gus Soden.


>

> We're trying to make the http package smaller & simpler recently, not
> adding too much stuff, unless it's clear & very painful or impossible to do
> otherwise.

So, a small change will make r.FormValue method better.

func (r *Request) FormValue(key string) string {
if r.Form == nil {
r.ParseMultipartForm(defaultMaxMemory)
}

/*

if vs := r.Form[key]; len(vs) > 0 {
return vs[0]
}
return ""

*/

return r.Form.Get(key)
}