I have the following handling of consecutive errors:
nodes, err := model.AllNodes()
if err != nil { // This error handling
pr := progressRes{ // is Just a copy of
Type: progressResponse, // error handling
Message: err.Error(), // for the next error,
MessageType: errorMessage, // what is the
Progress: 100, // best practice
} // to avoid
go response(ws, pr.json()) // repeating myself here
return // without making the code
} // complicated
links, err := model.AllLinks()
if err != nil {
pr := progressRes{
Type: progressResponse,
Message: err.Error(),
MessageType: errorMessage,
Progress: 100,
}
go response(ws, pr.json())
return
}
What is the best practice to avoid repeating myself in the above code without making the code complex? I can think of adding a new func
, however I just thought there might be a better idea which I'm not aware of.
If you are repeating a number of steps in multiple places the correct approach would be to abstract away those steps into a procedure, which is what programming is about. This applies to error handling just as well as to any other parts of your program.
One option:
func allNodesAndLinks() ([]*Node, []*Link, error) {
nodes, err := model.AllNodes()
if err != nil {
return nil, nil, err
}
links, err := model.AllLinks()
if err != nil {
return nil, nil, err
}
return nodes, links, nil
}
// ...
nodes, links, err := allNodesAndLinks()
if err != nil {
pr := progressRes{
Type: progressResponse,
Message: err.Error(),
MessageType: errorMessage,
Progress: 100,
}
go response(ws, pr.json())
return
}
Another option:
func respondWithError(ws io.Writer, err error) {
pr := progressRes{
Type: progressResponse,
Message: err.Error(),
MessageType: errorMessage,
Progress: 100,
}
response(ws, pr.json())
}
// ...
nodes, err := model.AllNodes()
if err != nil {
go respondWithError(ws, err)
return
}
links, err := model.AllLinks()
if err != nil {
go respondWithError(ws, err)
return
}
There are probably two different approaches you can use; either defer
or, as you suggested, using another function.
Defer would work like this:
// All of these must be defined before we call defer.
var err error
var progressResponse string
var errorMessage string
defer func() {
// Since the deferred function will be called even if
// the function has completed successfully, we need to
// check that there actually has been an error before we
// create the error response.
if err == nil {
return
}
pr := progressRes{
Type: progressResponse,
Message: err.Error(),
MessageType: errorMessage,
Progress: 100,
}
go response(ws, pr.json())
}()
nodes, err := model.AllNodes()
if err != nil {
// The deferred function will automatically be called here.
return
}
links, err := model.AllLinks()
if err != nil {
// And in every other place where the function returns.
return
}
The problem here is that there can be some pitfalls related to variable shadowing. Take this example:
var err error
defer func() {
if err != nil {
handleError(err)
}
}()
if a, err := doSomething(); err != nil {
return
}
Here's a proof of concept on the playground.
The problem here is that the err
inside of the if
clause is not the same as the one in the upper scope; if you declare a
in the upper scope as well, and use a single assignment =
instead of a declaration :=
, then it works as expected. Variable shadowing is a common pitfall, especially for beginners; further reading.
Thus, the approach I generally use and recommend, is that of having another function. The caller generally only needs one actual return argument anyway, so it usually doesn't become very complex.
func a() {
links, err := b()
if err != nil {
pr := progressRes{
Type: progressResponse,
Message: err.Error(),
MessageType: errorMessage,
Progress: 100,
}
go response(ws, pr.json())
return
}
}
func b() ([]Link, error) {
nodes, err := model.AllNodes()
if err != nil {
return nil, err
}
links, err := model.AllLinks()
if err != nil {
return nil, err
}
// you probably then do something with nodes and links?
return resolveLinks(nodes, links)
}
My preferred approach, because it makes unit testing much easier than the other suggestions, would be:
func doStuff() {
if err := doStuffWithError(); err != nil {
pr := progressRes{
Type: progressResponse,
Message: err.Error(),
MessageType: errorMessage,
Progress: 100,
}
go response(ws, pr.json())
return
}
}
func doStuffWithError() error {
nodes, err := model.AllNodes()
if err != nil {
return err
}
links, err := model.AllLinks()
if err != nil {
return err
}
// Do something with nodes and links
return nil
}