code review for justforfunc episode #2
Conversation
|
Wow thanks for you PR, i am very excited for view your video the next Monday. I really thank, regards ! |
| w.Header().Set("Content-Type", "application/json") | ||
| err = json.NewEncoder(w).Encode(response{Data: data, Success: err == nil}) | ||
| if err != nil { | ||
| log.Printf("could not encode response to output: %v", err) |
There was a problem hiding this comment.
shouldn't status become 500 if it fails to encode?
|
|
||
| c, err := h.storage.Save(url) | ||
| if err != nil { | ||
| return nil, http.StatusBadRequest, fmt.Errorf("Could not store in database: %v", err) |
There was a problem hiding this comment.
If database fails, it's not users fault.
Shouldn't this be sending 500?
func responseHandler(h func(io.Writer, *http.Request) (interface{}, int, error)) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
data, status, err := h(w, r)
if err != nil {
data = err.Error()
}
w.WriteHeader(status)
w.Header().Set("Content-Type", "application/json")
err = json.NewEncoder(w).Encode(response{Data: data, Success: err == nil})
if err != nil {
log.Printf("could not encode response to output: %v", err)
}
}
}
Otherwise the content-type never gets populated. |
| return nil, err | ||
| } | ||
|
|
||
| _, err = p.db.Exec("update shortener set visited=$1, count=$2 where uid=$3", true, item.Count+1, id) |
There was a problem hiding this comment.
After the update, the visited status nor the count are getting reflected into the item pointer. Could be better use a UPDATE RETURNING in a single queryRow and Scan.
thanks for allowing me to review your code!
I'll be publishing a video explaining all of this on Monday on justforfunc.
In the meanwhile happy to answer any questions 😄