Which error handling pattern do you prefer?

[Kn]

Hi, guys, I want to know which error handling pattern do you prefer. Following is a code snippet from go stdlib.

https://sourcegraph.com/github.com/golang/go/-/blob/src/net/http/h2_bundle.go?L1848

Let me simplify my questions:

Pattern1: like the code in go stdlib, in the same function, we first declare one error variable, then in the following if-statement we use one-liner pattern to declare a new error variable.

```go

func MyFunc() error {

  v, err := doSomething()

  if err != nil {

    return err

  }

  // stuff about processing `v`

  if err := doAnotherthing(); err != nil {

    return err

  }

  // stuff about processing ...

}

```

I think pretty code should not only be readable but also to conform to same code style. So I think why not use the already declared variable before. Then we have the following pattern.

Pattern2: Firstly doSomething() returns multiple return values and we need processing `v` later, so we use `v, err := doSomething()` as before. But, I think the `error` encountered is special compared to other local variables, it represents a state of current function (success or failure). So I think only one error variable is enough, it can be used for indicating any error, no matter the error is generated from `doSomething()` or `doAnotherthing()`. 

And, I didn't use the one-liner pattern which may be used for minimize the variable's scope. Some people think we should use one-line pattern here to conform to this rule.

```go

func MyFunc() error {

  v, err := doSomething()

  if err != nil {

    return err

  }

  // stuff about processing `v`

  err := doAnotherthing()

  if err != nil {

    return err

  }

  

  // stuff about processing ...

}

```

Of course I know the rule to minimize the variable's scope, but I think error is special and consistent coding style is important and beautiful. I searched the go source code, these two patterns are both used.

I want to know which pattern do you prefer and suggested.

Thanks!

[Axel Wagner]

I don't prefer either. I decide on a case-by-case basis. I generally ignore the question of scope, though. The relevant question (to me) is readability. If the statement is short, I use the one-line version, otherwise (or if I need to use either result after the conditional) I use the version with a separate statement. But that's a rule-of-thumb, as I said, I decide case by case.

--
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.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/2999eca8-1cc4-475c-8f85-0d2c8b966268n%40googlegroups.com.

[Brian Candler]

func MyFunc() error {

  v, err := doSomething()

  ...

  err := doAnotherthing()

}

That won't compile anyway; the second assignment would have to be

  err = doAnotherthing()

See: https://play.golang.org/p/pvNC7YHI88j

I do find this rather annoying, to the point where I'll stick a "var err error" at the top of a function rather than have the first assignment be different to the subsequent ones.  And yet, a single assignment is fine if you use the assign-and-test form, since this introduces a new variable which is only scoped to that block:

        // This is OK

if err := doAnotherthing(); err != nil {

return err

}

See: https://play.golang.org/p/4NKmFoX7uj2

It is also possible to use named return values and a naked return, although I get the impression this tends to be frowned upon.

[Kn]

Oh, I have a typo error, in the second pattern, `err := doAnotherthing()` should be `err = doAnotherthing()`.

[Kn]

`I do find this rather annoying, to the point where I'll stick a "var err error" at the top of a function rather than have the first assignment be different to the subsequent ones. `

I think this is a good way. I usually code in this way if there're multiple error assignments.

If each error assignment statement only return single return value (the error itself), I code it in one-liner way.

[Miguel Angel Rivera Notararigo]

I tend to use errX (X is adapted according to context) for function scoped errors, and err for block scoped errors

func MyFunc() error {

  v, errDS := doSomething()

  ...

  errDA := doAnotherthing()

}

if err := doAnotherthing(); err != nil {

return err

}

That way you don't shadow errors.

[David Finkel]

I can't +1 this enough.

I've caught so many bugs from shadowed errors (and re-used error variables). I consider it a rather bad anti-pattern to have a single err variable that's reused throughout a scope.

If you have unique error variable names and you forget to do something with an error that you've assigned a name you automatically get unused variable compile-errors. (just this is enough to be worthwhile)

With that said, constraining the scope using the initializer statement on an if (or switch) statement suffices when you don't need any other return values, at which point I may use the err variable-name (although I often make those unique for clarity anyway).

--
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.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAF9DLCmR4ZdnVs4A28BSrPcbiHsQ_ufub5cSPjCt2SDy2dA1xA%40mail.gmail.com.

[Michael Ellis]

FWIW (which may not be much) I've settled on explicitly naming my return values in the function declaration.  If the function returns include an error, I name always name it err. The general pattern is

func foo() (v someType, err error) {

  err = doSomething()

  if err != nil {

    err = fmt.Errorf("some context: %v", err)

    return

}

  err = doNextThing()

  if err != nil {

     err = fmt.Errorf("some context: %v", err)

     return

  }

// etc

return

}

Naming the error solves the initial := problem.  As for shadowing, I make it a point never to do a := assignment involving err. For example, if I need to call os.Open, I do

var f *os.File

f, err = os.Open(...)

I tend to use other names for errors only when it's something I can fix within the local scope.

At first I tried hard to avoid the verbosity.  Now I use a few helpful snippets to reduce keystrokes and an editor plugin the partially fades any block that starts with "if err != nil".  The latter works amazingly well (for me at least) to reduce visual clutter.  I like it much better than one I tried that auto-folds error blocks.  It made me waste time unfolding them to see what was inside :-)

YMMV, of course.

[jake...@gmail.com]

On Friday, November 12, 2021 at 6:29:46 PM UTC-5 michael...@gmail.com wrote:

FWIW (which may not be much) I've settled on explicitly naming my return values in the function declaration.  If the function returns include an error, I name always name it err. The general pattern is

func foo() (v someType, err error) {

  err = doSomething()

  if err != nil {

    err = fmt.Errorf("some context: %v", err)

    return

}

  err = doNextThing()

  if err != nil {

     err = fmt.Errorf("some context: %v", err)

     return

  }

// etc

return

}

 

As Brian mentioned above, naked return values are generally frowned upon. You can easily find the arguments if you like, but even the official tour of go (at https://tour.golang.org/basics/7)  says:

"Naked return statements should be used only in short functions, as with the example shown here. They can harm readability in longer functions."

That's a pretty official caution. Personally, I am very much in that camp. In my experience, naked returns harm readability in all cases, just less in short functions.

So this is probably not a good pattern to use as your default one.