First draft #1
First draft #1
Conversation
| func boot() { | ||
| var err error | ||
|
|
||
| logger.Info(fmt.Sprintf("booting! ttl: %dms / refresh interval: %dms / save interval: %dms\n", requestTtl, RefreshInterval, SaveInterval)) |
There was a problem hiding this comment.
in all these you can just use Infof all loggers generally always have a f variant:
https://godoc.org/github.com/op/go-logging#Logger.Infof
| reqCounter.Refresh() // purge expired events before starting | ||
| } | ||
|
|
||
| logger.Info("initializing signal traps\n") |
There was a problem hiding this comment.
Haven't used that logger but rarely \n is necessary.
| } | ||
|
|
||
| func (this *RequestCounter) Refresh() { | ||
| // is this thread safe? or do we need a mutex? |
There was a problem hiding this comment.
maps are not thread safe, you need a mutex
| } | ||
| } | ||
|
|
||
| func (this *Request) IsExpired(ttl time.Duration) bool { |
There was a problem hiding this comment.
Don't use generic names such as "me", "this" or "self"
https://github.com/golang/go/wiki/CodeReviewComments#receiver-names
|
Looking good, how about adding some interfaces for the main components and pulling the server out into an struct so that |
| saveInterval string, | ||
| sleepPerRequest string, | ||
| ) (*Config, error) { | ||
| _requestTtl, err := time.ParseDuration(requestTtl) |
There was a problem hiding this comment.
never seen the underscore naming convention used in Go
| http.ListenAndServe(r.config.BindAddr, nil) | ||
| } | ||
|
|
||
| func (r *RequestCounter) process(response http.ResponseWriter, request *http.Request) { |
There was a problem hiding this comment.
If we start to go by the book now, this should implement the http.Handler interface
https://golang.org/pkg/net/http/#Handler
Then the semaphore should become a reusable http middleware.
http://www.alexedwards.net/blog/making-and-using-middleware
Then create http.Server in the main function and .ListenAndServe() from there.
No description provided.