Error handling and repetition
Michael Schaller
We all write this way too often:
if err != nil {
return err
}
I hate to write this or variations of it as it is boilerplate that hurts readability. I love it because it makes error handling explicit and I really have to think about error handling at every level.
How about this to reduce the boilerplate but to stay explicit?
func A() (err error) {
on assign err {
if err != nil {
return
}
}
err = ...
err = ...
...
}
What do you think?
Best,
Michael Schaller
Notes on the example:
* To make the example shorter I've used a named return variable.
* There could also be a short form like 'if assign err; err != nil {'.
* It reeks a bit like Visual Basic's 'ON ERROR' but this could be used outside of error handling as well. It could for an instance make the usage/cast of variables of type interface{} easier. If you think about it there are many many use cases.
* It feels a bit like operator overloading but it also feels a bit like defer just that something gets scheduled for the time a specific variable (also in a specific scope) gets assigned.
* If the wrong variable is "watched" via 'on assign' and an variable isn't used otherwise the compiler would still fail with the typical unused variable error.
* I'm unusure if this produces hard to read compiler error messages. It behaves a bit like preprocessor macro (search for specific assign and enhance with if block) and hence it could be hard to point the user at the right location of an error. I leave that to the compiler folks to decide how easy or hard this is.
Jan Mercl
> on assign err {
-j
Michael Schaller
There is no restriction with regards to new keywords in the go1compat doc [1]. In any case this can also be an idea for Go 2. ;-)
[1] https://golang.org/doc/go1compat
---j
--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/68J-mLCC1JI/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Jan Mercl
On Sun, Jun 7, 2015 at 2:21 PM Michael Schaller <mi...@google.com> wrote:
> There is no restriction with regards to new keywords in the go1compat doc [1].
>
""""
It is intended that programs written to the Go 1 specification will continue to compile and run correctly, unchanged, over the lifetime of that specification.
""""
Any existing program using the identifier newly becoming a keyword would not compile anymore.
-j
Michael Schaller
On Jun 7, 2015 2:33 PM, "Jan Mercl" <0xj...@gmail.com> wrote:
>
>
> On Sun, Jun 7, 2015 at 2:21 PM Michael Schaller <mi...@google.com> wrote:
>
> > There is no restriction with regards to new keywords in the go1compat doc [1].
> >
> > [1] https://golang.org/doc/go1compat
>
> """"
> It is intended that programs written to the Go 1 specification will continue to compile and run correctly, unchanged, over the lifetime of that specification.
> """"
>
Counter quote:
"""
These same considerations apply to successive point releases. For instance, code that runs under Go 1.2 should be compatible with Go 1.2.1, Go 1.3, Go 1.4, etc., although not necessarily with Go 1.1 since it may use features added only in Go 1.2
"""
> Any existing program using the identifier newly becoming a keyword would not compile anymore.
Any existing program using an unknown or not in this version available keyword must not compile.
>
> > In any case this can also be an idea for Go 2.
>
> The universe-defined interface error is special only in this. Otherwise it's like any other interface type. But it would get special treatment. IOW this would adds another exception to the existing rules. That can hardly make anything simpler for anyone learning the language.
>
The idea is for any variable of any type and is hence not limited to error handling. Please see the notes on the example I've included.
> Also, hiding a piece of control flow code in an assignment, when a certain type, which is not normally visible in the expression at all, is used, IMHO makes readability worse, not better.
>
Absolutely valid point but one can already write poorly readable code in Go today. IMHO there should be always code reviews and if code is poorly readable/understandable then it should be flagged and reworked.
I know that reality is different though and that this proposal would allow and would definitely be used for incredibly convoluted code. The same is though also true for defer for an instance. You could for an instance transform a named return value with the help of defer before the value gets returned to the caller. Is that bad code? IMHO it depends but someone would need to convince me really hard during a code review to allow it. ;-)
Henry Adi Sumarto
I'm not sure if this is the best practice in Go though.
Egon
On Sunday, 7 June 2015 14:31:33 UTC+3, Michael Schaller wrote:
Hi everyone,
We all write this way too often:
if err != nil {
return err
}
I hate to write this or variations of it as it is boilerplate that hurts readability. I love it because it makes error handling explicit and I really have to think about error handling at every level.
How about this to reduce the boilerplate but to stay explicit?
func A() (err error) {
on assign err {
if err != nil {
return
}
}
err = ...
err = ...
...
}
What do you think?
Michael Schaller
On Jun 7, 2015 5:27 PM, "Egon" <egon...@gmail.com> wrote:
>
> First what is the actual code you are problems with?
This is a proposal for running code on variable assignment. This proposal could avoid error handling repetition besides being usable for a lot of other things. If you want actual code look at any Go code that does proper error handling and count the lines that are needed for error handling alone. Most of this error handling code is repetitive and IMHO boilerplate. That's what this is about.
> Second are you providing informative error messages or not?
>
This proposal is independent of variable values and hence your question only lets me assume that you didn't understand the proposal. Please let me know what is unclear and I'm more than happy to clarify.
Egon
On Sunday, 7 June 2015 18:37:29 UTC+3, Michael Schaller wrote:
On Jun 7, 2015 5:27 PM, "Egon" <egon...@gmail.com> wrote:
>
> First what is the actual code you are problems with?This is a proposal for running code on variable assignment. This proposal could avoid error handling repetition besides being usable for a lot of other things.
If you want actual code look at any Go code that does proper error handling and count the lines that are needed for error handling alone. Most of this error handling code is repetitive and IMHO boilerplate. That's what this is about.
> Second are you providing informative error messages or not?
>
This proposal is independent of variable values and hence your question only lets me assume that you didn't understand the proposal. Please let me know what is unclear and I'm more than happy to clarify.
Giulio Iotti
I understand it... it's been proposed several times in different forms before. Usually the case is that people are not providing proper error messages. Or there is some better way of handling of errors (e.g. http://blog.golang.org/error-handling-and-go and http://blog.golang.org/errors-are-values). Or there are some cases where panics are more appropriate. Or it might be that in some cases that the handling of errors is indeed verbose. Or that by restructuring your code you can rid of them etc... but it's impossible to do that analysis without the actual real-world problems at hand.(Note, when I say real-world; I really mean it... not facilitated, not analogues, not anything else... actual code that is having those problems.)
Michael Schaller
On Jun 7, 2015 5:53 PM, "Egon" <egon...@gmail.com> wrote:
> I understand it... it's been proposed several times in different forms before. Usually the case is that people are not providing proper error messages. Or there is some better way of handling of errors (e.g. http://blog.golang.org/error-handling-and-go and http://blog.golang.org/errors-are-values). Or there are some cases where panics are more appropriate. Or it might be that in some cases that the handling of errors is indeed verbose. Or that by restructuring your code you can rid of them etc... but it's impossible to do that analysis without the actual real-world problems at hand.
Gotcha. That makes totally sense. ;-)
Let's take this function from the standard library as an example then:
http://golang.org/src/archive/zip/reader.go?s=817:866#L36
I see 6 lines of actual code and 11 lines of error handling code. 7 of the error handling code lines are copies. I don't see how this could be written any better with the current Go spec unless one uses goto. Then you might reduce the error handling code by two lines but the readability also suffers.
My proposal is to have the error handling code block once and to run it every time something gets assigned to the err variable. This would effectively get rid of the duplicated code.
Egon
On Sunday, 7 June 2015 19:24:34 UTC+3, Michael Schaller wrote:
On Jun 7, 2015 5:53 PM, "Egon" <egon...@gmail.com> wrote:
> I understand it... it's been proposed several times in different forms before. Usually the case is that people are not providing proper error messages. Or there is some better way of handling of errors (e.g. http://blog.golang.org/error-handling-and-go and http://blog.golang.org/errors-are-values). Or there are some cases where panics are more appropriate. Or it might be that in some cases that the handling of errors is indeed verbose. Or that by restructuring your code you can rid of them etc... but it's impossible to do that analysis without the actual real-world problems at hand.
Gotcha. That makes totally sense. ;-)
Let's take this function from the standard library as an example then:
http://golang.org/src/archive/zip/reader.go?s=817:866#L36
I see 6 lines of actual code and 11 lines of error handling code. 7 of the error handling code lines are copies. I don't see how this could be written any better with the current Go spec unless one uses goto. Then you might reduce the error handling code by two lines but the readability also suffers.
My proposal is to have the error handling code block once and to run it every time something gets assigned to the err variable. This would effectively get rid of the duplicated code.
Michael Schaller
On Jun 7, 2015 6:23 PM, "Giulio Iotti" <dullg...@gmail.com> wrote:
>
> This proposal looks very similar to the helper example in "Errors are Values", just with (not very helpful) additional syntactic sugar.
Let me please first clarify that I wholeheartedly agree with the "Errors are Values" blog post.
My "syntactic sugar" proposal is in two points very different from the helper approach:
1) You register error handling once per error variable. The specified error handling code will automatically run on assignment to that variable. The programmer can't forget to use the helper and there is no code duplication that could be an extra source of issues.
2) My proposal isn't a function but rather behaves like if. This has the advantage that you can directly use return with the error value.
Bakul Shah
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
Michael Schaller
On Jun 7, 2015 6:40 PM, "Egon" <egon...@gmail.com> wrote:
>
>
>
> On Sunday, 7 June 2015 19:24:34 UTC+3, Michael Schaller wrote:
>>
>> On Jun 7, 2015 5:53 PM, "Egon" <egon...@gmail.com> wrote:
>>
>> > I understand it... it's been proposed several times in different forms before. Usually the case is that people are not providing proper error messages. Or there is some better way of handling of errors (e.g. http://blog.golang.org/error-handling-and-go and http://blog.golang.org/errors-are-values). Or there are some cases where panics are more appropriate. Or it might be that in some cases that the handling of errors is indeed verbose. Or that by restructuring your code you can rid of them etc... but it's impossible to do that analysis without the actual real-world problems at hand.
>>
>> Gotcha. That makes totally sense. ;-)
>> Let's take this function from the standard library as an example then:
>> http://golang.org/src/archive/zip/reader.go?s=817:866#L36
>
>
> I rather write it as:
> f, err := os.Open(name)
> if err != nil {
> return nil, fmt.Errorf("failed to open file: %v", err)
> }
> fi, err := f.Stat()
> if err != nil {
> f.Close()
> return nil, fmt.Errorf("failed to get file stat: %v", err)
> }
> r := new(ReadCloser)
> if err := r.init(f, fi.Size()); err != nil {
> f.Close()
> return nil, fmt.Errorf("failed to initialize zip read closer: %v", err]
> }
> r.f = f
> return r, nil
>
> Although it's quite low-level, so I'm not sure whether the more information would benefit from it... usually you are not writing that low-level so, those more informative/clearer messages are more appropriate.
First of all I like your code and in case of more verbose error messages I would write it the same way you did. In case of low level code I would also just hand through the error values and in this case the code duplication is something I very much dislike.
>
>> I see 6 lines of actual code and 11 lines of error handling code. 7 of the error handling code lines are copies. I don't see how this could be written any better with the current Go spec unless one uses goto. Then you might reduce the error handling code by two lines but the readability also suffers.
>>
>> My proposal is to have the error handling code block once and to run it every time something gets assigned to the err variable. This would effectively get rid of the duplicated code.
>
>
> Remember that every proposal has a down-side, if you don't see it, you haven't probably analyzed it well enough. With every proposal you should provide the down-sides it as well. Engineering is about trade-offs... in this case the main problem I can see would be that it would strive people towards code that doesn't use good error messages. Of course current approach doesn't prohibit it as well.
That is a very valid point but one that can't be technically solved. That's a topic for code reviews IMHO. Please also note that my proposal is not limited to error variables only. It can be used on all local variables...
>
> + Egon
>
> --
> You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
> To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/68J-mLCC1JI/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
Michael Schaller
On Jun 7, 2015 6:48 PM, "Bakul Shah" <ba...@bitblocks.com> wrote:
>
> Often some cleanup action is needed after an error but before the actual return. Or you may want to modify an error to return a more appropriate error. Since "on assign" won't have the proper context, such cleanup will get quite a bit messier. Or you may want to do something else after an error return - for example on io.EOF. All in all "on assign" will create its own readability issues! And it will make debugging harder. Which error caused the return? Currently you can just add a printf before every error return. Your proposed feature also doesn't fit the Go style of making things more explicit and less magic under the hood. If you are still convinced this is worth adding, I suggest manually editing at least a couple thousand lines of code and then comparing the two versions for readability.
I share these same concerns and that's why I've asked for thoughts, especially if someone would like to see something like that and for what, besides error handling, they could imagine to use it. So far no one seems to see much value in it though...
Egon
Michael Schaller
On Jun 7, 2015 7:13 PM, "Egon" <egon...@gmail.com> wrote:
>
> I completely understand it...
>
Thanks. :-D
> Just a clarification, my litmus tests for proposals are:
...
>
Egon, this is an awesome writeup and I especially like point 4 as it includes biases.
>
> Without those diving deep into the proposal is hard work... especially if you don't have those problems yourself.
I actually didn't write "proposal" in my initial mail once. Primarily because I'm writing alm this on a tablet. ;-)
This was more intended like here is an idea, how would you use it if this existed? That kinda derailed... I guess because too many complained about repetition in error handling without thinking a bit harder about it and hence I've choose a bad example. Sorry.
>
> Finally regarding the actual proposal:
> From technical side: how would it behave with variable shadowing?
>
Only assignments to the specified variable are watched. If you watch a variable that shadows another one then the shadowed variable isn't watched but you shouldn't be able to assign to that variable easily anyway. If you watch a variable that gets shadowed by another variable then most likely the compiler will complain about an assigned but unused variable.
> From my point of view, it doesn't look like a big problem... It's not completely over-verbose at the moment... and most of the cases either have poor error messages or there is an alternative solution. Adding the proposal would complicate the flow of the code, making the code harder to understand. But, I know the frustration in some cases. Then again it usually makes me think more about the error cases and whether I can make the flow clearer...
>
Egon, that was the kind of feedback I was looking for. ;-)
Jan Mercl
> """
> These same considerations apply to successive point releases. For
-j
Ian Lance Taylor
>
> We all write this way too often:
> if err != nil {
> return err
> }
>
> I hate to write this or variations of it as it is boilerplate that hurts readability. I love it because it makes error handling explicit and I really have to think about error handling at every level.
>
> How about this to reduce the boilerplate but to stay explicit?
> func A() (err error) {
> on assign err {
> if err != nil {
> return
> }
> }
>
> err = ...
> err = ...
> ...
> }
>
> What do you think?
You need to think through the proposal in much more detail. Go
generally aims to be as orthogonal as possible in all language
concepts. That means that if we permit "on assign" (which we won't)
we will need to support code like
func F() int {
var x, y int
on assign x {
return 1
}
on assign y {
return 2
}
x, y = 0, 0
}
What happens?
You're adding a statement that is executed when a variable is
assigned, but there are many ways that a variable can be assigned.
func F() int {
var x int
on assign x {
return 1
}
fmt.Sscanf("2", "%d", &x)
return x
}
What happens?
To be clear, I think it is extremely unlikely that this kind of change
will be made to the language. It adds spooky action at a distance.
We currently only have that in one place--panic--and we discourage use
of panic.
Ian
Jason Gade
Michael Schaller
Ian, thank you for the great example. That was the kind of feedback I was looking for.
Ian's example makes it plain obvious that it would be way too easy to write hard to debug code as it mingles too much with the control flow. It's clear that this example would return 1 (assignments are left to right according to the Go spec) but this is already no longer simple and straight-forward for everyone - especially not for beginners. Not to mention that arguing about or debugging real world code can quickly get nasty... on assign would probably be quickly discouraged by code reviewers. So yeah, this wouldn't be worth the effort and dangerous. Thanks to everyone for the discussion. :-D
defer on the other side is better defined. It runs on return, however that return was triggered, which gives nastynesses like Ian's example no chance. defer also differs in that it only runs once. defer is still a bit magic but IMHO a lot less magic like for an instance goroutines.
Jonathan
On Sunday, June 7, 2015 at 7:31:33 AM UTC-4, Michael Schaller wrote:
Hi everyone,
We all write this way too often:
if err != nil {
return err
}
I hate to write this or variations of it as it is boilerplate that hurts readability. I love it because it makes error handling explicit and I really have to think about error handling at every level.
How about this to reduce the boilerplate but to stay explicit?
func A() (err error) {
on assign err {
if err != nil {
return
}
}
err = ...
err = ...
...
}
Ian Lance Taylor
> Doesn't defer also add "spooky action at a distance"?
since it becomes part of the call sequence. But, yes, it means that
"return" does something written elsewhere.
Ian
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
Wojciech S. Czarnecki
Jason Gade <jay...@gmail.com> napisał(a):
> Doesn't defer also add "spooky action at a distance"?
distance between a cleanup call and a reason for that call.
So neither "spooky" nor "at a distance" - "Needed" and "right in place".
--
Wojciech S. Czarnecki
^oo^ OHIR-RIPE