1) Be careful with locks in the form "x.Lock(); x.DoSomething(); x.Unlock()". If DoSomething panics, you will still be holding the lock, and that's pretty much the end of your program. ("x.Lock(); defer x.Unlock(); x.DoSomething()" avoids this problem, but obviously in the non-panic case, the lock is released at a different time than in this implementation. Additional tweaking is required.)
Generally I don't like locks in the request critical path because waiting for a lock is uncancelable, but in this very simple case it doesn't matter. For more complicated concurrency requirements, consider the difference between x.Lock()/x.Do()/x.Unlock vs. select { case x := <-ch: doSomethingWithX(x); case <-request.Context().Done(): error(request.Context().Err()) }. The channel wait can be cancelled when the user disconnects, or hits the stop button in the error, or the request timeout is reached.
2) Long if/else statements are harder to read than a switch statement. Instead of:
if(foo == "bar") {
// Bar
} else if (foo == "baz") {
// Baz
} else {
// Error
}
You might like: switch(foo) {
case "bar":
// Bar
case "baz":
// Baz
default:
// Error
}
These are exactly semantically equivalent, and neither protect you at compile-time from forgetting a case, but there is slightly less visual noise. Worth considering.3) I have always found that error handling in http.HandlerFunc-tions cumbersome. The author runs into this, with code like:
foo, err := Foo()
if err != nil {
http.Error(w, ...)
return
}
bar, err := Bar()
if err != nil {
http.Error(w, ...)
return
}
Basically, you end up writing the error handling code a number of times, and you have to do two things in the "err != nil" block, which is annoying. I prefer: func DoTheActualThing() ([]byte, error) {
if everythingIsFine {
return []byte(`{"result":"it worked and you are cool"}`), nil
}
return nil, errors.New("not everything is okay, feels sad")
}
Then in your handler function: func ServeHTTP(w http.ResponseWriter, req *http.Request) {
result, err := DoTheActualThing()
if err != nil {
http.Error(w, ...)
return
}
w.Header().Set("content-type", "application/json")
w.WriteHeader(http.StatusOK)
w.Write(result)
}
In this simple example, it doesn't matter, but when you do more than one thing that can cause an error, you'll like it better.