docs: tweaks to error printing and concurrency notes #25
Conversation
This patch updates the error printing examples from calling `err.(*errors.Error).String()` to simply user `err.Error()` because although the former may work in these examples, it's a bad habit to obtain and may be copy-pasted; plus, the safe code is actually shorter anyway. It also makes a minor language tweak of using the phrase "safe for concurrent use" to mimic the Go standard library documentation.
| ```go | ||
| server, err := goyave.New(goyave.Options{}) | ||
| if err != nil { | ||
| fmt.Fprintln(os.Stderr, err.(*errors.Error).String()) |
There was a problem hiding this comment.
The reason why I used a type assert and String() here is that String() and Error() don't have the same output: Error() doesn't print the stackframes, which can be useful for debugging.
This is not a problem once you have access to the Logger since it handles that for you, but since the Logger couldn't be created yet at that point in the example, I preferred to use the type assert.
There was a problem hiding this comment.
Now that you brought this to my attention, maybe this would be worth adding a small callout explaining why it's done that way below the example. 🤔
There was a problem hiding this comment.
Ahhhhh now I see what you mean! Useful, and really with the new errors package include a way to do this naturally -- they seemed to be inspired by Dave Cheney's github.com/pkg/errors by they missed stack trace!
Maybe you could make a top level helper like errors.Details(err error) string that does something like
package errors
...
func Details(err errors) string {
switch err := err.(type) {
case *errors.Error:
return errors.String()
default:
return errors.Error()
}
}Anyway, I don't know enough about your codebase at this point to know if that's a good idea so I'll move on… thanks for taking the time to explain!
| ## State | ||
|
|
||
| The server state is an atomic number. The atomic nature of this state makes it concurrently safe, so any goroutine can read it easily. It allows the start and stop controls to be called from any goroutine. | ||
| The server state is an atomic number. The atomic nature of this state makes it safe for concurrent use, so multiple goroutine can read it safely. It allows the start and stop controls to be called from any goroutine. |
There was a problem hiding this comment.
| The server state is an atomic number. The atomic nature of this state makes it safe for concurrent use, so multiple goroutine can read it safely. It allows the start and stop controls to be called from any goroutine. | |
| The server state is an atomic number. The atomic nature of this state makes it safe for concurrent use, so multiple goroutines can read it safely. It allows the start and stop controls to be called from any goroutine. |
|
Thanks for the "concurrency" change, that reads better 👍🏻 |
This patch updates the error printing examples from calling
err.(*errors.Error).String()to simply usererr.Error()because although the former may work in these examples, it's a bad habit to obtain and may be copy-pasted; plus, the safe code is actually shorter anyway.It also makes a minor language tweak of using the phrase "safe for concurrent use" to mimic the Go standard library documentation.