Skip to content
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

DefaultQuery of type int, float or bool #1898

Open
paulvollmer opened this issue May 13, 2019 · 12 comments
Open

DefaultQuery of type int, float or bool #1898

paulvollmer opened this issue May 13, 2019 · 12 comments

Comments

@paulvollmer
Copy link

Could be helpful to have func's like DefaultQueryInt for example.

then we could write something like

limit, err := c.DefaultQueryInt("limit", 50)
if err != nil {
  // query is not an integer, we can response a statuscode 400 
}

Or what's the best way to use c.DefaultQuery for types like int, float, bool or time.
is there a best practise to implement?

@guonaihong
Copy link
Contributor

guonaihong commented Jun 12, 2019

I am very interested in your thoughts.
I am planning to design

 func DefaulQueryVar(key string, var, defValue interface{}) error

Var can be *int, *float, *bool, etc.
usage

var ss []string
c.DefaultQueryVar(key, &ss, []string{"1", "2", "3"})
var i int
c.DefaultQueryVar(key, &i, 3)
var f float64
c.DefaultQueryVar(key, &f, 3.14)

guonaihong added a commit to guonaihong/gin that referenced this issue Jun 12, 2019
```go
package main

import (
        "fmt"
        "github.com/gin-gonic/gin"
)

func main() {
        r := gin.Default()
        r.GET("/", func(c *gin.Context) {
                var i int
                var err error

                err = c.DefaultQueryVar("int", &i, -1)
                fmt.Printf("%v, %v\n", i, err)

                var ss []string
                err = c.DefaultQueryVar("slice", &ss, []string{"5", "5", "5"})
                fmt.Printf("%v, %v\n", ss, err)

                var b bool
                err = c.DefaultQueryVar("bool", &b, false)
                fmt.Printf("%v, %v\n", b, err)

                var f float64
                err = c.DefaultQueryVar("f", &f, 3.14)
                fmt.Printf("%v, %v\n", f, err)

        })

        r.Run()
        return
}

```
@paulvollmer
Copy link
Author

That sounds good. Why do you want to do reflection and not creating DefaulQueryInt, DefaulQueryString functions instead of DefaulQueryVar? just a question to understand the design...

@guonaihong
Copy link
Contributor

  • For the gin web framework, the fewer functions provided, the lower the learning cost.
    DefaultQueryVar , supports many types, such as int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, bool, float32, float64, etc.
  • Go 2.0 will provide generic functions in the near future. To provide a function such as DefaultQueryInt, DefaultQueryFloat32., the function prototype may be DefaultQuery(key string) (<T>, error)

@dmarkham
Copy link
Contributor

This is a interesting idea,
I for one would rather have this as a stand alone package for gin vs being built in directly.

I'm not holding my breath for generics, and when or if it happens, so many other things will want to get upgraded this will seem very small.

If it turns out this is going to be included with gin, I really dislike the idea if using reflection and a empty interface. As painful as it is to have to make the extra methods, that is what Go is currently about it's everywhere mainly because most people don't want to pay the cost of refection or losing type safety with the empty interface.
Most Libraries do not make them all. int, int64, bool, float64 seems to be kinda normal and make people will convert as needed. time you normally have to roll your own mainly because everyone has a different idea how to format them, any package you use to parse them has to make some assumptions.

@guonaihong
Copy link
Contributor

The ShadowdBind, ShouldBindJSON, ShouldBindXML, ShouldBindQuery, ShouldBindYAML functions in gin are based on the reflect package. Although many people don't like reflect, the ShouldBind APIs are super easy to use.

The idea of making DefaultQueryVar is the same as that of ShouldBindJSON, making an API that everyone likes.

@thinkerou
Copy link
Member

If need, I like the follow:

type Q struct {
  A string `query: "a" binding:"required"`
  B int `query:"b" binding:"required"`
}

@dmarkham
Copy link
Contributor

The ShadowdBind, ShouldBindJSON, ShouldBindXML, ShouldBindQuery, ShouldBindYAML functions in gin are based on the reflect package. Although many people don't like reflect, the ShouldBind APIs are super easy to use.

The idea of making DefaultQueryVar is the same as that of ShouldBindJSON, making an API that everyone likes.

Those Methods build on top of Lower level Internal and external Libraries, None of them add reflection code into gin and all of them have to handle many different data types in a single incoming payload.

DefaultQueryVar is simply a []string converter to a single type like []int64.

It's not the same as ShouldBindJSON, ShouldBindXML, ShouldBindQuery, ShouldBindYAML.

Don't get me wrong I like this idea, we should do it for path c.Param also.
I'm just a little skeptical I guess :)

@guonaihong
Copy link
Contributor

@dmarkham The idea is very good, so I extend the C. Param method.

@guonaihong
Copy link
Contributor

Now it’s done.

@thinkerou
Copy link
Member

thinkerou commented Jun 15, 2019

Why not use ShouldBindUri and ShouldBindQuery?

@guonaihong
Copy link
Contributor

xxxVar function is only an enhanced version of c.Param and c.DefaultQuery
. c.ShouldBindUri, c.ShouldBindQuery is the best choice when dealing with >=2 parameters. If only one parameter is processed, c.DefaultQueryVar and c.ParamVar are also a good choice. You don't need to define a structure first. Handle different types of variables.

@zsr228
Copy link

zsr228 commented Nov 22, 2019

@guonaihong hi, guonaihong
through you design "func DefaulQueryVar(key string, var, defValue interface{}) error",
I found, if defValue is nil, the var type how to determine.
such as

var flag bool
DefaulQueryVar("flag", &flag, nil)

maybe the design exists error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants