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

passing values as a struct #3

Closed
willnorris opened this issue Aug 1, 2013 · 3 comments
Closed

passing values as a struct #3

willnorris opened this issue Aug 1, 2013 · 3 comments

Comments

@willnorris
Copy link

I've been toying with the idea of using uritemplates for go-github for a little while now. In particular, I'm interested in trying to clean up some of the code I have for appending query parameters, that currently looks something like:

type ListOptions struct {
        Page int
}

func (s *OrganizationsService) List(user string, opt *ListOptions) ([]Organization, error) {
        u := fmt.Sprintf("users/%v/orgs", user)
        if opt != nil {
                params := url.Values{
                        "page": []string{strconv.Itoa(opt.Page)},
                }
                u += "?" + params.Encode()
        }
        ...
}

For small option types this isn't too bad, but gets worse with larger ones and just seems like it could be cleaner. What I've been thinking about is mimicking how encoding/json and encoding/xml use field tags. So something like:

type ListOptions struct {
        Page int `url:"page"`
}

func (s *OrganizationsService) List(user string, opt *ListOptions) ([]Organization, error) {
        u := fmt.Sprintf("users/%v/orgs", user)
        u += uritemplates.Parse("{?page}").Expand(ListOptions)
        ...
}

Of course, I would ideally just have a single line with a template of users/{user}/orgs{?page}, but I haven't decided on the best way to pass the user into Expand() yet. I don't want to put it in ListOptions since it's a required parameter to the List() function and I want that to be explicit.

There are of course ways to do this by converting the struct into a map first, and then passing that to Expand(), but I figured I'd float this idea to you and see what you thought.

@jtacoma
Copy link
Owner

jtacoma commented Oct 24, 2013

I like this idea, it seems pretty doable and a good fit with Go conventions.

@willnorris
Copy link
Author

cool... I'll take a look at this. For what I was doing, I ended up writing a small library that converts structs directly to a url.Values. Even with this support in the uritemplates library, you have to define all of your fields twice, once as tags on your struct fields, and again as part of the URI template. That completely makes sense, but for my purposes it was easier to go straight from struct to url.Values, since all variables were always part of the query string, never part of the path.

Anyway, I mention only to say that there was a fair amount of logic to handle edge cases as well as things like omitempty, embedded structs, etc that you might consider. Feel free to take a look.. much of it is based on the encoding/json package: https://github.com/google/go-querystring

@jtacoma
Copy link
Owner

jtacoma commented Oct 24, 2013

Nice: the reflectValue function in your query/encode.go really is thorough. I've had to write code that carried these responsibilities more than once myself, maybe I'll put together a gist.

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

2 participants