Good introduction. A few thoughts:
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.
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:
You might like: 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:
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: Then in your handler function: 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.