Re: [go-nuts] error handling code is too verbose

[Jan Mercl]

On Sat, Nov 3, 2012 at 12:20 PM, <dsk...@gmail.com> wrote:
> Hi,
>
> I often write code like the the function below but this style can make the
> simplest functions hard to read because the actual logic is drowned in error
> handling.

I would say that the _actual_ logic _includes_ the error handling and
moreover at exactly where the error can occur. I, for one, consider
this to be one of the advantages Go brings to us.

This topic is repeating, you can find some older threads about it in
this list. Warning: Some of them may be too long or a bit heated ;-)

-j

[Jesse McNelis]

On Sat, Nov 3, 2012 at 10:20 PM, <dsk...@gmail.com> wrote:

Hi,

I often write code like the the function below but this style can make the simplest functions hard to read because the actual logic is drowned in error handling. Is there a better pattern that separates the error handling from the logic so that the logic is more readable?

The error handling is about the same as other languages. But Go requires explicit checking which makes code a bit longer but more readable because it doesn't try to hide the error handling part of the logic.

Lots of previous threads on this list you can search for.

Also some reading material:

http://blog.golang.org/2011/07/error-handling-and-go.html

https://plus.google.com/116810148281701144465/posts/iqAiKAwP6Ce  

--
=====================
http://jessta.id.au

[David Anderson]

On Sat, Nov 3, 2012 at 3:58 PM, <dsk...@gmail.com> wrote:

I wasn't trying to say that Go is bad. I just had the feeling that I was doing it wrong.

It's a common feeling with folks getting started with Go. Reverting to explicit error handling takes a while to adjust to.

 

Would you say that if I use my suggested alternative (that I find more readable) that I'm doing it wrong?

Your approach feels wrong to me, yes. Now that I'm used to it, I prefer reading the error handling inline, as it makes clear what is going on.

I've never seen your approach used in Go. It feels a little like the Error monad in Haskell, only without the monadic goodness. It definitely works, but personally, I'd avoid it. It forces you to look at three functions to figure out how errors are handled, rather than just the one.

- Dave

 

--
 
 

[David de Kloet]

> For a case like your example, definitively not. I thought the problem was
> that error handling was too verbose, but you are making it not only more
> verbose, but also quite obfuscated.

The problem of the verbosity is that if you try to quickly see what is
going on, all you see if a series of error handling blocks. What I'm
looking for is a way that makes it easier to see what is actually
going on (apart from error handling). If that's even more verbose then
that is not necessarily a problem as long as the verbosity is pushed
to a place where it's less in the way.

I was hoping I was missing some nice pattern that every body else is
using but I hadn't figured out yet. Apparently that's not the case. So
consider my question answered, unless you have another interesting
pattern for handling errors to share.

[Jens Alfke]

On Saturday, November 3, 2012 4:20:42 AM UTC-7, dsk...@gmail.com wrote:

Hi,

I often write code like the the function below but this style can make the simplest functions hard to read because the actual logic is drowned in error handling. Is there a better pattern that separates the error handling from the logic so that the logic is more readable?

I know what you mean, and I'm also unhappy with the Go application I've written, in that so much of the code is obscured by repetitive boilerplate for the form "if (err != nil) { return err }" (only spread over three lines due to gofmt policy.)

I'm considering rewriting my code to use exceptions instead, i.e. panic and recover. This should make it a lot more compact and readable. As far as I can tell, this style isn't intrinsically discouraged/deprecated in Go, as long as the exceptions aren't thrown across a package boundary; that is, public package functions should return errors as results rather than panic. I can live with that.

Jan Merci wrote:

>I would say that the _actual_ logic _includes_ the error handling and 
>moreover at exactly where the error can occur. I, for one, consider 
>this to be one of the advantages Go brings to us. 

While I agree that error handling is part of the logic, it's also true that 99% of the time that part consists of checking for an error returned by a function call and immediately returning the same error from the function. That's _exactly_ the sort of boilerplate that I prefer not to have to type and read, especially when it occurs after every two or three lines of a function!

I also don't see how explicit error handling is an "advantage Go brings to us", since you can do that style of error handling in any language if you want, whether or not it supports exceptions.

--Jens

[egon]

There are several ways how you can make it more readable, but not necessarily better:

First, helper functions:

func didfail(err error) bool {

  if err != nil {

    context.Errorf("%v", err)

    return true

  }

  return false

}

func check(err error) { _ = failed(err) }

func doStuff(context Context) (err error) {

  x, err := computeX()

  if didfail(err) {

    return

  }

  err = doSomethingWithX(x)

  check(err)

  return

}

Panic/Recover - for handling exceptional cases (see Russ Cox-s blog post)

func doStuff(context Context) (err error) {

  defer func(){

    if r := recover(); r != nil {

      return r

    }

  }()

  x := computeX()

  doSomethingWithX(x)

}

This can mean that you have to clean up the mess computeX or doSomethingWithX did... it may be good idea to make a proper handling of such cases:

func handleErrors(fn func(Context)) func(Context) error {

  return func(ctx Context) error {

    defer func(){

      if r := recover(); r != nil { return r }

    }()

    tmp := MakeTemporary(ctx)

    fn(tmp)

    err := tmp.CommitTo(ctx)

    return err

  }

}

func doStuff(context Context) {

  x := computeX()

  doSomethingWithX(x)

}

var DoStuff = handleErrors(doStuff)

there are probably more... and this still may have lot of panic calls, but it can be encapsulated inside some check function...

I'm not saying that they are good solutions... but they are options that you can consider...

+egon

On Saturday, November 3, 2012 1:20:42 PM UTC+2, dsk...@gmail.com wrote:

Hi,

I often write code like the the function below but this style can make the simplest functions hard to read because the actual logic is drowned in error handling. Is there a better pattern that separates the error handling from the logic so that the logic is more readable?

func doStuff(context Context) (err error) {

  x, err := computeX()

  if err != nil {

    context.Errorf("Error computing x: %v", err)

    return

  }

  err = doSomethingWithX(x)

  if err != nil {

    context.Errorf("Error doing something: %v", err)

  }

  return

}

If I wouldn't have to do error handling, the code could look like this which is actually readable:

func doStuff() (err error) {

  x := computeX()

  doSomethingWithX(x)

}

This gives me the impression I must be doing something wrong, but what?

Should I do something like this?

func computeX(context Context, err *error) (x X) {

  x, *err = reallyComputeX()

  if *err != nil {

    context.Errorf("Error computing x: %v", err)

  }

  return

}

func doSomethingWithX(x X, context Context, err *error) {

  if *err != nil { return }

  err = reallyDoSomething(x)

  if *err != nil {

    context.Errorf("Error doing something: %v", err)

  }

}

func doStuff(context Context) (err error) {

  x := computeX(context, &err)

  doSomethingWithX(x, context, &err)

}

This is more readable (to me) but it feels very silly to call doSomethingWithX even if computeX failed, only to return immediately because err was already != nil.

Any other suggestions?

Thanks,

David

[Thomas Bushnell, BSG]

On Sat, Nov 3, 2012 at 4:20 AM, <dsk...@gmail.com> wrote:

I often write code like the the function below but this style can make the simplest functions hard to read because the actual logic is drowned in error handling. Is there a better pattern that separates the error handling from the logic so that the logic is more readable?

func doStuff(context Context) (err error) {

  x, err := computeX()

  if err != nil {

    context.Errorf("Error computing x: %v", err)

    return

  }

  err = doSomethingWithX(x)

  if err != nil {

    context.Errorf("Error doing something: %v", err)

  }

  return

}

If I wouldn't have to do error handling, the code could look like this which is actually readable:

func doStuff() (err error) {

  x := computeX()

  doSomethingWithX(x)

}

If you wish to ignore the error, write

func doStuff() {

  x, _ := computeX()

  doSomethingWithX(x)

}

This is just like Python. You can ignore the error with this:

def doStuff():

  x = computeX()

  doSomethingWithX(x)

Or you can handle the error with this:

def doStuff():

  try:

    x = computeX()

  except e:

    context.Errorf("Error computing x")

    raise e

  try:

    doSomethingWithX

  except e:

    context.Errorf("Error doing something")

    raise e

I think the Go version is vastly more readable. The only difference is that Python programmers are used to ignoring errors.

Thomas

[Anssi Porttikivi]

An idea: Could we recognize error blocks and print them in small font?

[andrey mirtchovski]

> An idea: Could we recognize error blocks and print them in small font?

scheduled for go2 addition to gofmt right after ansi colouring makes it in.

[Jens Alfke]

On Nov 3, 2012, at 6:17 PM, "Thomas Bushnell, BSG" <tbus...@google.com> wrote:

If you wish to ignore the error, write...

The goal isn't to ignore the error (which would almost always be a bad idea), rather to propagate it back up to the caller without introducing three additional lines of boilerplate. That would seem to fit in with Go's goals of reducing clutter.

I think throw/catch exceptions are a fine way to do that; Go's designers seem to disagree (and I know it's a religious topic.) It would be nice if there were some sort of compromise, like a syntactic shorthand for "if(err != nil) {return err}".

--Jens

[Michael Jones]

Such a construct has been proposed and discussed recently in this mailing list.

On Sat, Nov 3, 2012 at 7:26 PM, Jens Alfke <je...@mooseyard.com> wrote:
> like a syntactic shorthand for "if(err != nil) {return err}"

--
Michael T. Jones | Chief Technology Advocate | m...@google.com | +1
650-335-5765

[Peter Kleiweg]

On 3 nov, 12:20, dskl...@gmail.com wrote:

> I often write code like the the function below but this style can make the
> simplest functions hard to read because the actual logic is drowned in
> error handling. Is there a better pattern that separates the error handling
> from the logic so that the logic is more readable?

There are special cases you have to handle in a specific way. But
often, there are general cases like, "error not allowed, panic", or
"can't go on because of error, return". For these, I have two helper
functions:

func CheckErr(err error) {
if err != nil {
log.SetFlags(0)
_, filename, lineno, ok := runtime.Caller(1)
if ok {
log.Fatalf("%v:%v: %v\n", filename, lineno, err)
} else {
log.Fatalln(err)
}
}
}

func WarnErr(err error) error {
if err != nil {
f := log.Flags()
log.SetFlags(0)
_, filename, lineno, ok := runtime.Caller(1)
if ok {
log.Printf("%v:%v: %v\n", filename, lineno, err)
} else {
log.Println(err)
}
log.SetFlags(f)
}
return err
}

I use this in code like this:

x, err := doSomething()
CheckErr(err)

doSomethingWith(x)

Or:

x, err := doSomethingElse()
if WarnErr(err) {
return
}

doSomethingWith(x)

Combined specific and general:

for {
line, _, err := rd.ReadLine()
if err == io.EOF {
break
}
CheckErr(err)

doSomethingWith(line)
}

[Peter Kleiweg]

On 4 nov, 11:10, Peter Kleiweg <pklei...@xs4all.nl> wrote:

> Or:
>
>     x, err := doSomethingElse()
>     if WarnErr(err) {
>         return
>     }
>
>     doSomethingWith(x)

That should be:

    x, err := doSomethingElse()
    if WarnErr(err) != nil {
        return
    }

    doSomethingWith(x)

[Florian Weimer]

* Jan Mercl:

> On Sat, Nov 3, 2012 at 12:20 PM, <dsk...@gmail.com> wrote:
>> I often write code like the the function below but this style can make the
>> simplest functions hard to read because the actual logic is drowned in error
>> handling.
>
> I would say that the _actual_ logic _includes_ the error handling
> and moreover at exactly where the error can occur.

Merely threading error values isn't error handling. If you leave out
the context.Errorf() call in the example provided (which is the common
case), it's still verbose, and it's boilerplate code without any
actual functionality.

Personally, the verbosity doesn't bug me that much. I'm more
concerned that the error handling for functions which are called just
for their side effects is omitted accidentally. It's difficult to
check this automatically. For instance, sometimes, omitting local
error checking is safe because the code actually works on bufio.Reader
objects and relies on the fact that errors are sticky.

[Larry Clapp]

I missed it / don't remember it / can't find it.  :(

I, for one, would probably be happy if return was an expression:

result, err := someFunction();

err == nil || return 0, err

Or possibly goto:

result, err := someFunction();

err == nil || goto ERROR

or if there were some way to turn statements into expressions:

result, err := someFunction();

err == nil || do { return 0, err }

I suppose I should just admit that other languages (or rather, other code) just ignore(s) errors too much, and the fact that idiomatic Go sometimes looks cluttered is because other languages (other code) are(is) inappropriately tidy.  :)

-- Larry

[Norbert Roos]

Hi,

> This gives me the impression I must be doing something wrong, but what?

You could simply add comments which describe what the program is doing.
Then you can concentrate on reading only the comments to get an idea of
the program flow. Any other reader or even you after one year will be
thankful:

func doStuff(context Context) (err error) {

// compute X

x, err := computeX()
if err != nil {
context.Errorf("Error computing x: %v", err)
return
}

// do something with X

err = doSomethingWithX(x)
if err != nil {
context.Errorf("Error doing something: %v", err)
}
return
}

(Of course the comments may also be more verbose...)

Norbert