I am trying to write simple RESTful app in Golang using gorilla mux. I wrote few handlers that look as follows:
func getUser(w http.ResponseWriter, r *http.Request) {
if r.Header.Get("Content-type") == "application/json" {
w.Header().Set("Content-Type", "application/json")
u, err := _getUser(r)
if err != nil {
http.NotFound(w, r)
return
}
json.NewEncoder(w).Encode(u) //asked for json, return json
} else {
w.Header().Set("Content-Type", "text/html")
u, err := _getUser(r)
if err != nil {
http.NotFound(w, r)
return
}
renderTemplate(w, "view", u) // asked for html, return html
}
}
func _getUser(r *http.Request) (*User, error) {
params := mux.Vars(r)
for _, u := range users {
if u.ID == params["id"] {
return &u, nil
}
}
return nil, errors.New("")
}
func main() {
router := mux.NewRouter()
router.HandleFunc("/v1/users/{id}", getUser).Methods("GET")
}
The problem I got here is that I have got a lot of duplication. Every CRUD method would have to check for content type and return either json or html.
I thought about writing a closure
func htmlOrJSON(fn func(http.ResponseWriter, *http.Request) (interface {}, error), templateName string) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
if r.Header.Get("Content-type") == "application/json" {
w.Header().Set("Content-Type", "application/json")
result, err := fn(w, r)
if err != nil {
http.NotFound(w, r)
return
}
json.NewEncoder(w).Encode(result)
} else {
w.Header().Set("Content-Type", "text/html")
result, err := fn(w, r)
if err != nil {
http.NotFound(w, r)
return
}
renderTemplate(w, templateName, result)
}
}
}
// and use as:
router.HandleFunc("/v1/users/{id}", htmlOrJSON(getUser, "view")).Methods("GET")
To remove duplication but it also does not look well. Could anyone help me make this code cleaner?
Although this is a code review question and should be in the CodeReview community, I’ll try to answer it.
Write a generic function that handles HTML and JSON rendering. The error handling IMO should happen on each handler even if you duplicate some code. It makes more sense there and makes the code more readable and explicit. You will soon see that there will be other errors that require special handling.
On the logic, most APIs accept query parameters http://api.com/user/1?fomtat=json
. This makes more sense because when a client accept more than content types you will be stuck.
const JSON = "application/json"
func getUser(w http.ResponseWriter, r *http.Request) {
u, err := _getUser(r)
if err != nil {
http.NotFound(w, r)
return
}
responseBody(u, r.Header.Get("Content-type"), &w)
}
func responseBody(u User, contentType string, w io.writer) {
switch contentType {
case JSON:
w.Header().Set("Content-Type", JSON)
json.NewEncoder(w).Encode(u) //asked for json, return json
default:
w.Header().Set("Content-Type", "text/html")
renderTemplate(w, "view", u) // asked for html, return html
}
}