In order to reduce basic error-checking bloat in Go software and improve readability, I propose shifting eligible error-checking to the right by the addition of control-flow that operates on the error type and a related gofmt rule.
Go has an error-checking issue. Whenever a function is called that returns a value and an error. At least 3 additional lines of code are required to check it.
This isn't an issue when the code is actually handling the error (as the error handling becomes part of the logical-flow of the code) but when the error is simply passed to the caller, the error-checking obstructs the logical-flow of the code and makes it more difficult to understand what the code is trying to do.
In a perfect world, there wouldn't be any errors and IO functions could be written as such.
The behaviour of this function is easy to reason about.
func OpenImage(name string) image.Image {
return image.Decode(os.Open(name))
}
Now consider the OpenImage functions below, _OpenImage2_ checks errors and _OpenImage1_ does not. The logical-flow of _OpenImage1_ is easier to read than _OpenImage2_.
func OpenImage1(name string) image.Image {
f, _ := os.Open(name)
img, _, _ := image.Decode(f)
return img
}
func OpenImage2(name string) (image.Image, error) {
f, err := os.Open(name)
if err != nil {
return nil, fmt.Errorf("OpenImage: %w", err)
}
img, _, err := image.Decode(f)
if err != nil {
return nil, fmt.Errorf("OpenImage: %w", err)
}
return img, nil
}
Most of the time, errors do need to be checked though and this causes the logical-flow of the code to be obstructed. Here is another example from here.
func (ds *GitDataSource) Fetch(from, to string) ([]string, error) {
fmt.Printf("Fetching data from %s into %s...\n", from, to)
if err := createFolderIfNotExist(to); err != nil {
return nil, err
}
if err := clearFolder(to); err != nil {
return nil, err
}
if err := cloneRepo(to, from); err != nil {
return nil, err
}
dirs, err := getContentFolders(to)
if err != nil {
return nil, err
}
fmt.Print("Fetching complete.\n")
return dirs, nil
}
Trying to read functions like this is jarring as the reader is interrupted by error checking at almost every step.
x.return [ ExpressionList ] .
For a value _x_ of type _error_, _x.return_ inside of function _F_ terminates the execution of _F_
if _x != nil_, and optionally provides one or more resulting error values. Any functions deferred by F are executed before F returns to its caller.
There are three ways to return values from a function with a result type:
func OpenImage(name string) (image.Image, error) {
f, err := os.Open(name); err.return fmt.Errorf("failed to open image: %w", err)
img, _, err := image.Decode(f); err.return fmt.Errorf("failed to decode image: %w", err)
return img, nil
}
func OpenImageSpecificErrors(name string) (img image.Image, openErr error, decodeErr error) {
f, err := os.Open(name); err.return fmt.Errorf("failed to open image: %w", err), nil
img, _, err := image.Decode(f); err.return nil, fmt.Errorf("failed to decode image: %w", err)
return img, nil, nil
}
func OpenImage(name string) (image.Image, error) {
f, err := os.Open(name); err.return
img, _, err := image.Decode(f); err.return
return img, nil
}
func OpenImage(name string) (image.Image, error) {
wrap := func(in error) error {
return fmt.Errorf("OpenImage: %w", in)
}
f, err := os.Open(name); err.return wrap(err)
img, _, err := image.Decode(f); err.return wrap(err)
return img, nil
}
func (ds *GitDataSource) Fetch(from, to string) ([]string, error) {
fmt.Printf("Fetching data from %s into %s...\n", from, to)
err := createFolderIfNotExist(to); err.return
err = clearFolder(to); err.return
err = cloneRepo(to, from); err.return
dirs, err := getContentFolders(to); err.return
fmt.Print("Fetching complete.\n")
return dirs, nil
}
Pros
Cons
Sorry for the misoperation
Has similarities to #33177, #32946, #38151, #39148, and likely others.
I agree that the error checking and returning in the simple case if verbose in go. On the other hand it forces the developer to think about the actual handling, i.e. to not only return the error but also implement additional cleanup tasks (closing files etc).
In the proposal here I'm unclear how return values for the non-error results would be passed back?
@andig
In regards to cleanup tasks, that is what _defer_ and wrapper functions are for.
I want to make the distinction between handling an error (where appropriate action is taken and the error is not returned) and not handling an error (the error is transformed in some way, clean-up tasks are run and the error is returned so that somewhere up the stack it can get handled).
In Go, errors are explicit and a decision to handle needs to be made every time an error value is recieved. This is one of Go's strengths.
Handling errors in Go is great. Returning errors are... not so great. It looks like the error is being handled and this obstructs the readabillity of the code.
In my experience, Go doesn't force you to handle errors, it can actually discourage you from doing so. "My function is incredibly long and unreadable from all this error checking, why would I want to make it worse, I'll just return the error".
An argument can be made that having a shorter way to return errors will encourage less error handling. However I would speculate that the inverse can be true, having shorter functions psychologically gives the developer more room for error-handling. Less is more!
Keep in mind that this proposal is not strictly about saving characters, it's about moving error-passing to the right to improve readability. Personally I'm not a fan of previous check/try/? proposals (not explicit enough, everything is hidden, etc).
To clarify the behavior of non-error results:
_error.return_ returns non-error results set to their zero value. If you need to return different values then you must use a standard return statement.
It seems like having very long lines with error handling on the right will tend to obscure the error handling. I understand that that is kind of the point of this issue. But errors do need to be handled. Hiding the handling at the end of a long line doesn't clarify the function, it obscures it.
If we don't try to put the error handling at the end of a long line, we could put it on the next line. But then we aren't saving all that much with this approach. We're saving an if, a != nil, and a couple of braces. And admittedly a couple of line breaks.
I feel like the try proposal (#32437) tried to address error handling at a deeper level. That proposal was not accepted, but it had the advantage of trying to recast error handling. This proposal seems to be tweaking error handling a bit. Is the benefit worth the cost of a language change?
Based on the comments above, and the lack of support in emoji voting, this proposal is a likely decline. Leaving open for four weeks for final comments.
No further comments.
Most helpful comment
It seems like having very long lines with error handling on the right will tend to obscure the error handling. I understand that that is kind of the point of this issue. But errors do need to be handled. Hiding the handling at the end of a long line doesn't clarify the function, it obscures it.
If we don't try to put the error handling at the end of a long line, we could put it on the next line. But then we aren't saving all that much with this approach. We're saving an
if, a!= nil, and a couple of braces. And admittedly a couple of line breaks.I feel like the
tryproposal (#32437) tried to address error handling at a deeper level. That proposal was not accepted, but it had the advantage of trying to recast error handling. This proposal seems to be tweaking error handling a bit. Is the benefit worth the cost of a language change?