Any alternative error handling / reporting?

[lamvak]

Hi!

My head starts to spin when I read my own code, for the amount of

if err != nil {

    return nil, err

}

Is there no neat alternative? Program should panic only when there is a reason to panic - when programmer doesn't know how to continue computation (or at least that I got from the documentation).

Well, that idiom is what I know I can do for the most of the time: clean non garbage collected memory (mmaps) and return an error indication. But can't it be anything different that this?

[Dave Cheney]

I find if my code is littered with error handling it is a sign that I
need to compose my functions into higher order units.

Can you post a sample that is particularly troubling ?

Dave

[Patrick Mylund Nielsen]

func Foo(data []byte) (t Thing, err error) {
if err = Bar(); err != nil {
return
}
err = json.Unmarshal(data, &t)
return

[lamvak]

I was thinking of things like this one function:

https://gist.github.com/2656724

This is an actual (and working!) piece of library i'm currently working on.

Now, I know I may split the function - but I don't really need it at this particular moment, other than if it might really help with this ifing and scratching.

But I fail to see how it may: split or joined - at each of those steps I must check if the was no previous errors.

If we had a polymorphic types it could be solved nice and with a one liner - with something like Haskell's Maybe type. But as it is now, I'm at a loss - how do you handle this... error handling? ;)

lamvak

[Kyle Lemons]

I would write that as

func Foo(data []byte) (Thing, error) {

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

    return nil, err

  }

  var t Thing

  if err := json.Unmarshal(data, &t); err != nil {

    return nil, err

  }

  return t, nil

}

except, probably, with some more context around the errors.  To me this looks fine... I can see that Bar runs, then Unmarshal runs, and if they fail their errors are returned.

[Kyle Lemons]

On Thu, May 10, 2012 at 5:21 PM, lamvak <lam...@gmail.com> wrote:

I was thinking of things like this one function:

https://gist.github.com/2656724

This is an actual (and working!) piece of library i'm currently working on.

Now, I know I may split the function - but I don't really need it at this particular moment, other than if it might really help with this ifing and scratching.

But I fail to see how it may: split or joined - at each of those steps I must check if the was no previous errors.

If we had a polymorphic types it could be solved nice and with a one liner - with something like Haskell's Maybe type. But as it is now, I'm at a loss - how do you handle this... error handling? ;)

lamvak

I wouldn't use named return variables for that and I wouldn't constantly reuse the same error value.  Even if you do, though, you can move the "err =" lines into the initializer of their subsequent if statement and shave like 10 lines worth of vertical space.

[lamvak]

Well, ten lines are not to shabby. But that's not the solution I'm trying to find. It's only a squeezing ;).

l.

[Patrick Mylund Nielsen]

The point was to demonstrate how you could write something without
writing "return nil, err" everywhere. (If you wanted to check
something that only returned an error, you could just do if Bar() !=
nil { ... }, anyway.) Personally, I almost never use named return
values or the if err = Foo(); err != nil shorthand.

[Kyle Lemons]

It may look verbose now, but it's a common enough idiom that you'll get used to reading and writing it, and I think at some point you'll wake up and realize that you like it better than any cleaner alternative because of how clear and obvious and consistent it is.

[Patrick Mylund Nielsen]

Coming from Python, I've often thought "wow, I'm writing so much error
handling code", but soon I realize that I'm handling a lot of
different scenarios, from the start and by default, that I would only
handle in Python after the fact (read: crash.) I find that this
results in much more robust code (and I'm sure that that was one of
the motivating factors behind this.)

[Patrick Mylund Nielsen]

Actually, a good analogy is this:

Would you say that this code is "good"?

func Foo(data []byte) Thing {
var t Thing
conv, _ := convertData(data)
_ = json.Unmarshal(conv, &t)
t, _ = sanitize(t)
return t
}

No? That's what you write in Python. Go just makes the wrong thing
look bad and unnatural.

On Thu, May 10, 2012 at 7:50 PM, Patrick Mylund Nielsen

[Patrick Mylund Nielsen]

Thinking about it, I don't think I've ever started off with a try,
except SomeException wrapper around any code in Python. It's always
been retroactive. That Go "forces" you to consider the errors, and the
compile-time checks, are by far my two most favorite things in this
language.

On Thu, May 10, 2012 at 7:55 PM, Patrick Mylund Nielsen

[lamvak]

To this (and your later) message: I don't claim that it's not wise to early check for errors - but what I would like to see is less repetitive and verbose way of doing that. Like the said Haskell's Maybe type: it does not scrap error handling - instead, it makes it a little more convenient and shorthand.

[Patrick Mylund Nielsen]

Maybe it would be interesting if you could specify a closure to run
when a given variable, e.g. err, becomes non-nil, and just assign to
that variable throughout your function without the

if err != nil {
return foo, err
}

e.g.

func Foo(data []byte) (Thing, error) {

var err error
ReturnOn(err, func {
log.Println("There was an error unserializing the data:", err)
return
})
err = Bar()
       err = json.Unmarshal(data, &t)
err = sanitize(t)
       return t, nil
}

But this is perhaps not as easily digestible.

[Patrick Mylund Nielsen]

Make that

func Foo(data []byte) (Thing, error) {

var (
t Thing
err error
)

ReturnOn(err, func {
log.Println("There was an error unserializing the data:", err)

return t, err

})
err = Bar()
err = json.Unmarshal(data, &t)
err = sanitize(t)
return t, nil
}

(of course this example begs the use of named return values, but you
get the gist!)

On Thu, May 10, 2012 at 8:10 PM, Patrick Mylund Nielsen

[Kyle Lemons]

All of these suggestions make the same assumption that is made with exceptions: that any error in a particular block of code should be handled the same way.  In general, that's not the case, and it's much harder to go back once you've made that assumption.  I really like that I can go add more logging, stick a breakpoint, store a "known good" value, or add more error context to error conditions and that I know and can see exactly where they are at all times.

[Patrick Mylund Nielsen]

I agree. I realized after hitting send that this was eerily similar to
exceptions.

[Robert Johnstone]

Except that in the concrete case being discussed, there is a lot of repetitive "return nil, err".  So, while I agree that any proposed solution must allow you to break out and additional ffunctionality, none of the comments in this thread so far have fairly addressed the large degree of boilerplate that is present.

[Robert Johnstone]

This issue has been raised on the mailing lists a few times in the past.  So far, I haven't seen a better solution.  What you are writing appears to be close to idiomatic Go.  In fact, as you probably gathered from other posters, most authors would probably have more lines devoted to error handling (me included).  The alternative would be to rely more on exceptions (or a similar mechanism), which have their own disadvantages.

Sorry that I cannot provide a better answer.

[lamvak]

W dniu piątek, 11 maja 2012 16:17:56 UTC+2 użytkownik Robert Johnstone napisał:

This issue has been raised on the mailing lists a few times in the past.  So far, I haven't seen a better solution.  What you are writing appears to be close to idiomatic Go.  In fact, as you probably gathered from other posters, 

In fact, that's the case. But I really hoped that someone has figured out some better solution, more flexible.

 

most authors would probably have more lines devoted to error handling (me included).  The alternative would be to rely more on exceptions (or a similar mechanism), which have their own disadvantages.

IMO the optimal solution would add no more than 3 additional lines for a whole such ladder as I presented in the gist at the beginning of coding; and then if one would like to add more functionality to handle errors in some particular fragment, one would just inscribe.

I don't like exceptions too.

Sorry that I cannot provide a better answer.

Let's just think some more :).

[Nathan Keronkow]

I can't really overstate how much I sympathize with this. Those lines
crop up everywhere. I see it in my own code and it seems like it's all
over the standard library as well. I think the last place I noticed it
sticking out was the documentation for os.Exec.StdoutPipe:

cmd := exec.Command("echo", "-n", `{"Name": "Bob", "Age": 32}`)
stdout, err := cmd.StdoutPipe()
if err != nil {
log.Fatal(err)
}
if err := cmd.Start(); err != nil {
log.Fatal(err)
}
var person struct {
Name string
Age int
}
if err := json.NewDecoder(stdout).Decode(&person); err != nil {
log.Fatal(err)
}
if err := cmd.Wait(); err != nil {
log.Fatal(err)
}
fmt.Printf("%s is %d years old\n", person.Name, person.Age)

I've heard the philosophical arguments that it's best to place error-
handling code front and center but I think it loses its gravity when
you're necessarily doing the same thing over and over again. In Perl
(everyone's favorite model of thoughtful and disciplined programming
language design), I got pretty fond of either using the postfix syntax
for checking errors, or using 'or'; the canonical example being:

open(my $fh, ">", "output.txt") or die $!;

In Go I tried doing something similar:

func chk(e error) {
if e != nil {
log.Panic(e)
}
}

uSock, err := net.ResolveUnixAddr("unix", sockPath) ;chk(err)

Not very pretty, but it seemed to promise to become less distracting
the more I saw it. Nonetheless, go fmt insisted on putting it on the
next line, and there wasn't really an elegant way to make it available
without copy-pasting everywhere. More generally it could be written:
func chk(e error, f func(e error)) . It'd be nice if for the specific
case of error handling I could use something like that in postfix to
trim down redundant code. Whatever anyone says, checking for an
unrecoverable error and then either panicking or bailing out of a
function while logging and returning the exact same error seems like
the general case for error-checking. If Go swims against the current
with regards to exceptions it'd be nice to have better shorthand for
what otherwise is going to become a very tedious idiom.

And I think you all give people too much credit; pushing explicit
error-checking in a way that makes it stylistically obtrusive or
repetitive to write will probably just induce people to ignore errors
until they bite during runtime.

>On May 10, 8:24 pm, Kyle Lemons <kev...@google.com> wrote:
>Even if you do, though, you can move the "err=" lines into the initializer of their subsequent if statement and shave like 10 lines worth of vertical space.

I think I got out of the habit of doing this a while ago myself
because it locks any other variables the expression assigns to into
the scope of the if block.

[lamvak]

I could not agree more.

Only, I thought of using Panic kind of that way, but I consciously dismissed that. After all, Panic is not an exception. Panicking should be used only when program bums into an error of a potentially very harmful kind; one that slips out of the assumption base.

If one's doing IO, one ought to consider hardware faults. Using panic on IO errors would be just sloppiness. Same here: I'd like to have some more concise idiom to cope with a flow of control of several instructions where I do predict that errors may occur, but I dismiss option of using Panic.

That's in general. You Panic and Recover here, so that's not exactly simply panicking, but IMO it's a misuse nevertheless - the bare presence of application of Panic should indicate that programmer bumps into something odd "out of the blue".

l.

[Robert Johnstone]

I'm aware of two alternate strategies that can be used, but they are not very generic.

1) I've duplicated some functions to panic instead of returning an error.  So, for example, I have some classes with members String and StringOrDie.  It may be bad form, but adding four lines, plus extra variable declarations, was crossing a line when I really just wanted to write fmt.Println( myvar.String() ).  In addition to the code duplication, you arrive at a situation where you're ignoring potential errors, so I would not recommend this as a general approach, but it could be used to alleviate particular pain points.

2) The package html/template has the function Must.  You can also write similar functions to take a generic handler to parameterize the behaviour on error (log.Fatal, panic, possible others).  See http://play.golang.org/p/YFmtdPs-l2.  Unfortunately, without generics, it is impossible have to a single function to handle all cases.  Perhaps in the future the language can provide a built-in (similar to append), to provide this idiom.  Also, this idiom won't handle the most common case, "return nil, err".  Still, you could provide a similar function in your packages in some instances.

The above two techniques may help in specific instances.  Unfortunately, as I said earlier, I think in most instances you are left with the existing solution.

[First Last]

Any new features or keywords should be taken into careful
consideration before adding them to the language. That being said I
would enjoy if you could use some keyword (or special character) to
symbolize that you want to handle the error simple by returning the
latest error value and the "zero" value of all other return
parameters.

The keyword (or special character) should be carefully chosen so that
it won't collide with something else. But for this example I will
simply use "@" as a placeholder.

Concider the following example:

func Foo() (Thing, error) {
f, err := os.Open("test.json")

if err != nil {
return nil, err
}

defer f.Close()
data, err := ioutil.ReadAll(f)

if err != nil {
return nil, err
}

var t Thing
if err := json.Unmarshal(data, &t); err != nil {
return nil, err
}
return t, nil
}

And the same example using "@":

func Foo() (Thing, error) {
f, @ := os.Open("test.json")
// the following will be inserted by the compiler, directly after
the above
// function call:
//
// if @ != nil {
// return nil, @
// }
defer f.Close()
data, @ := ioutil.ReadAll(f)
var t Thing
if @ = json.Unmarshal(data, &t)
return t, nil
}

"@" would simply represent a variable that can store the error value,
compare it to nil after the function call has been made, and return
the error value if it ain't equal to nil. All return parameters except
the error parameter should be returned as their "zero" representation.

[Canopée]

This looks like the proposition I made some time ago :
http://canop.org/blog/?p=195

[si guy]

You could also allow meta-functions(right term? i don't actually know) to the left of an assignment couldn't you?

Probably a bad idea, readability goes out the window..... Wouldn't that be the case with most shortcuts for this though?

func someFunc()(someType,someErrorType){

/*Do some stuff and return a someErrorType if there's an error

*/

}

func someOtherFunc() (someValueType,someErrorType){

   v,(if @ != nil;{return nil,@;}) := someFunc()

   //Some processing on v

   return something,nil

[Ugorji Nwoke]

I suggested something like this before, an idea which could be generalized even beyond errors.

Every value has a known zero value, which can be checked by equality. If your code typically does a simple return on error, then you can use it.

func Foo() (t *Thing, err error) { 

   f, err := os.Open("test.json") 

   if err != nil { 

      return

   } 

   defer f.Close() 

   data, err := ioutil.ReadAll(f) 

   if err != nil { 

      return

   } 

   t = new(Thing)

   err = json.Unmarshal(data, &t)             

   if err != nil {

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

      return

   } 

   return

} 

reduces to: 

func Foo() (t *Thing, err error) { 

   f, ^err := os.Open("test.json")                // compiler/lexer adds a "; if err != nil { return }; "

   defer f.Close() 

   data, ^err := ioutil.ReadAll(f)                 // compiler/lexer adds a "; if err != nil { return }; "

   t = new(Thing)

   err = json.Unmarshal(data, &t)             // compiler/lexer doesn't add anything

   if err != nil {

      ^err = fmt.Errorf("Foo: %v", err)        // compiler/lexer adds a "; if err != nil { return }; "

   }

   return

} 

The rules:

 - ^ can only be used when assigning to a named return variables of boolean, numeric or string type or whose zero value is nil.

- By using this on the call site, you can decide whether you want the automatic "return if non-zero" to kick in at that point.

This doesn't detract from the go idiom, but gives a concise way to handle what has become the common case (check for error and return from the function at that point), while removing "boilerplate" and making the code clearer. You can still decide to add more context to an error, but for the common case of return an error up the stack, this is invaluable. 

I have functions that will reduce LOC by up to 50% if this is implemented, without changing the logic or semantics of the function at all.

There are other techniques for adding more context to an error if desired (that's a common pushback). For example, my code sometimes does:

func a() (err error) {

  defer handleErr("a", &err)

}

where handleErr looks like:

func handleErr(tag string, err *err) {

  if *err == nil { 

     return

  }

  *err = fmt.Errorf("%s: %v", tag, *err)

  return

[Aaron Bohannon]

On Tuesday, May 15, 2012 6:27:31 AM UTC-4, lamvak wrote:

Only, I thought of using Panic kind of that way, but I consciously dismissed that. After all, Panic is not an exception.

FYI: in a technical programming-languages sense, panics are simply exceptions.  The main difference between Go's exceptions and the ones in other languages is that, if you want to catch one kind, then you have to catch _any_ kind and re-throw if necessary.

 

Panicking should be used only when program bums into an error of a potentially very harmful kind; one that slips out of the assumption base.  If one's doing IO, one ought to consider hardware faults. Using panic on IO errors would be just sloppiness. 

Where one _ought_ use exceptions (i.e., panics) is an important software engineering question.  Panicking on IO errors is not sloppiness if one's program recovers at the place where it is appropriate to do so.  (See my example below.)

I found myself doing exactly the same things.  In fact, I'm tempted to use this function and just deal with the necessary cast:

func Must(x interface{}, e error) interface{} {

  if e != nil {

    panic(e)

  }

  return x

}

I am really quite surprised that there is no built-in generic function to do this in a way that doesn't require casting.  However, without a built-in function or generics in the language, the only way to do it without a cast is to write a different function for each type.  Of course, sometimes, the generic "Must" isn't even what you want.  For instance, when you do a Read(), you do not want to panic on EOF.  That _would_ be an abuse of the panic mechanism.  Thus, I found myself writing code that looked a lot like this:

http://play.golang.org/p/RvUiXS9guW

If I correctly understood the example in the language spec (http://golang.org/ref/spec#Handling_panics), that is the manner in which the panic mechanism was intended to be used, and it is fairly clean and succinct.  When I rewrote a simple library of mine in that style, instead of writing "if err != nil {...}" everywhere, I cut the number of lines of code in half.

 - Aaron

[Kyle Lemons]

On Fri, Jun 1, 2012 at 11:14 AM, Aaron Bohannon <aaro...@gmail.com> wrote:

On Tuesday, May 15, 2012 6:27:31 AM UTC-4, lamvak wrote:

Only, I thought of using Panic kind of that way, but I consciously dismissed that. After all, Panic is not an exception.

FYI: in a technical programming-languages sense, panics are simply exceptions.  The main difference between Go's exceptions and the ones in other languages is that, if you want to catch one kind, then you have to catch _any_ kind and re-throw if necessary.

 

Panicking should be used only when program bums into an error of a potentially very harmful kind; one that slips out of the assumption base.  If one's doing IO, one ought to consider hardware faults. Using panic on IO errors would be just sloppiness. 

Where one _ought_ use exceptions (i.e., panics) is an important software engineering question.  Panicking on IO errors is not sloppiness if one's program recovers at the place where it is appropriate to do so.  (See my example below.)

On Tuesday, May 15, 2012 10:16:37 AM UTC-4, Robert Johnstone wrote:

I'm aware of two alternate strategies that can be used, but they are not very generic.

1) I've duplicated some functions to panic instead of returning an error.  So, for example, I have some classes with members String and StringOrDie.  It may be bad form, but adding four lines, plus extra variable declarations, was crossing a line when I really just wanted to write fmt.Println( myvar.String() ).  In addition to the code duplication, you arrive at a situation where you're ignoring potential errors, so I would not recommend this as a general approach, but it could be used to alleviate particular pain points.

2) The package html/template has the function Must.  You can also write similar functions to take a generic handler to parameterize the behaviour on error (log.Fatal, panic, possible others).  See http://play.golang.org/p/YFmtdPs-l2.  Unfortunately, without generics, it is impossible have to a single function to handle all cases.  Perhaps in the future the language can provide a built-in (similar to append), to provide this idiom.  Also, this idiom won't handle the most common case, "return nil, err".  Still, you could provide a similar function in your packages in some instances.

The above two techniques may help in specific instances.  Unfortunately, as I said earlier, I think in most instances you are left with the existing solution.

I found myself doing exactly the same things.  In fact, I'm tempted to use this function and just deal with the necessary cast:

func Must(x interface{}, e error) interface{} {

  if e != nil {

    panic(e)

  }

  return x

}

I think everyone has been tempted to write that at one point, but please don't.  For the same reason, don't write the only-slightly-more-complicated version that converts panics into error returns from the calling function.  It might seem like you always want to panic on error, and it might seem like you always return on error, but that's simply not the case and when you go back and read the code or have to add debug prints or recovery a year from now, you will thank yourself for doing the natural thing and having an if statement.

[Robert Johnstone]

I understand what you are trying to protect against, but does YAGNI not apply?  The two code styles provide the same functionality, so YAGNI is not really applicable, but the underlying principle still holds.  You are suggesting it is better to write 2x to 3x the lines of code just in case.

If the few particular instances where it is necessary to insert debug prints or recovery, the call to Must can be replaced with the more verbose if statement.  Hell, you could probably write a gofix statement to replace all of the Must calls with if statements.  Unless the use of Must is going to block you architecturally from switching, I just don't see the problem.

(That said, I don't use this technique very much myself.  See my previous post)

[Kyle Lemons]

No, I insist that the 3 lines in question are always better than the 1 line in question, because it's more readable, clearer, more maintainable, etc.  You will feel the pain later.  YAGNI, which is dubious outside of XP anyway, does not apply.

[Robert Johnstone]

I'm pretty sure that avoiding paying for things you won't use always applies (XP or not), but won't argue with you regarding readability, since we don't have a good objective measure to settle the question one way or the other.

[Cary Hull]

On Thu, May 10, 2012 at 5:11 PM, Patrick Mylund Nielsen <pat...@patrickmylund.com> wrote:

func Foo(data []byte) (t Thing, err error) {
       if err = Bar(); err != nil {
              return
       }
       err = json.Unmarshal(data, &t)
       return

}

I've used this approach when the nesting is just 1-2 levels. With tabs it's pretty readable.

func Foo(data []byte) (t Thing, err error) {

    if err = Bar(); err == nil {

        err = json.Unmarshal(data, &t)

    } 

    return

}

On Thu, May 10, 2012 at 6:58 PM, lamvak <lam...@gmail.com> wrote:
> Hi!
> My head starts to spin when I read my own code, for the amount of

> if err != nil {
>     return nil, err
> }

[Aaron Bohannon]

On Fri, Jun 1, 2012 at 5:42 PM, Kyle Lemons <kev...@google.com> wrote:
> No, I insist that the 3 lines in question are always better than the 1 line
> in question, because it's more readable, clearer, more maintainable, etc.
>  You will feel the pain later.  YAGNI, which is dubious outside of XP
> anyway, does not apply.

What about DRY (don't repeat yourself)? Does that apply? One can
debate ad nauseum about _where_ a particular error should be handled,
but the fact remains that there is only _one_ place a particular error
should be handled in a particular program. If programmers must
explicitly test for errors on every call, then they will be repeating
the same logic at 90% of their call sites. I totally see the value of
returning error values at an API boundary, and I am not arguing with
that. I just think that explicitly handling error values on every
single function call is silly.

Consider the two programs below (runnable code here:
http://play.golang.org/p/6YxbRyMxzc). Does anyone actually believe
that the first is more readable and maintainable than the second one?
The latter program is still benefiting from Go's API design because
all calls that may generate an error must be wrapped in a Must/MustIO,
so it is visible where potential errors are being propagated upward.

- Aaron

(Disclaimer: I know the programs below repeat themselves -- but only
for the purpose of keeping the example as simple as possible while
illustrating what happens in the case of nested function calls.)

////////////////////////////////////////////////////////////////////////////////
// Program A: 64 lines
////////////////////////////////////////////////////////////////////////////////

func reportA(n int) error {
_, err := fmt.Printf("read %d bytes\n", n)
if err != nil {
return err
}
return nil
}

func fA(r io.Reader) (string, error) {
buf := make([]byte, 4)
n, err := r.Read(buf)
if err != nil {
return "", err
}
err = reportA(n)
if err != nil {
return "", err
}
s, err := gA(r)
if err != nil {
return "", err
}
return rot1(buf) + s, nil
}

func gA(r io.Reader) (string, error) {
buf := make([]byte, 4)
n, err := r.Read(buf)
if err != nil {
return "", err
}
err = reportA(n)
if err != nil {
return "", err
}
s, err := hA(r)
if err != nil {
return "", err
}
return rot1(buf) + s, nil
}

func hA(r io.Reader) (string, error) {
buf := make([]byte, 4)
n, err := r.Read(buf)
if err != nil {
return "", err
}
err = reportA(n)
if err != nil {
return "", err
}
return rot1(buf), nil
}

func mainA() {
reader := strings.NewReader("Hello, world")
s, err := fA(reader)
if err != nil {
fmt.Println("there was a problem:", err)
} else {
fmt.Println("result:", s)
}
}

////////////////////////////////////////////////////////////////////////////////
// Program B: 35 lines
////////////////////////////////////////////////////////////////////////////////

func reportB(n int) {
MustIO(fmt.Printf("read %d bytes\n", n))
}

func fB(r io.Reader) string {
buf := make([]byte, 4)
reportB(MustIO(r.Read(buf)))
return rot1(buf) + gB(r)
}

func gB(r io.Reader) string {
buf := make([]byte, 4)
reportB(MustIO(r.Read(buf)))
return rot1(buf) + hB(r)
}

func hB(r io.Reader) string {
buf := make([]byte, 4)
reportB(MustIO(r.Read(buf)))
return rot1(buf)
}

func mainB() {
reader := strings.NewReader("Hello, world")
s, err :=
func () (s string, err error) {
defer SetError(&err)
return fB(reader), nil
}()
if err != nil {
fmt.Println("there was a problem:", err)
} else {
fmt.Println("result:", s)
}
}

[Peter S]

For Program A, just a quick glance at the source reveals that it doesn't handle EOF correctly. For Program B, I had to go to the other source file to find out that it doesn't handle EOF correctly either.

As for DRY, in this example, the cause of the repetition of error handling is that IO is interspersed with logic, thus increasing the number of locations where error handling is necessary.

When starting with Program A, rather than trying to refactor to use common error handling at each location (which is attempted in Program B), I'd suggest to refactor to separate IO and logic; this would reduce the number of locations where error handling is needed in the first place.

With more complex programs, there is usually a wider variety of types of errors that need to be handled, and it is generally not correct to always handle the same kind of error the same way regardless of where it occurred. Having the error check "inlined" makes it much easier to spot incorrect error handling.

Although there may be cases where factoring out common error handling to helper functions can be beneficial, it should not be thought of as a universal solution, but considered case-by-case for specific types of errors, and always verified that it is actually suitable for all places where it is called from (or where a call may be added in the future). I think in most cases there are more efficient ways to improve the program.

Peter

[Kyle Lemons]

On Fri, Jun 1, 2012 at 6:07 PM, Aaron Bohannon <aaro...@gmail.com> wrote:

On Fri, Jun 1, 2012 at 5:42 PM, Kyle Lemons <kev...@google.com> wrote:
> No, I insist that the 3 lines in question are always better than the 1 line
> in question, because it's more readable, clearer, more maintainable, etc.
>  You will feel the pain later.  YAGNI, which is dubious outside of XP
> anyway, does not apply.

What about DRY (don't repeat yourself)?  Does that apply?  One can
debate ad nauseum about _where_ a particular error should be handled,
but the fact remains that there is only _one_ place a particular error
should be handled in a particular program.  If programmers must
explicitly test for errors on every call, then they will be repeating
the same logic at 90% of their call sites.  I totally see the value of
returning error values at an API boundary, and I am not arguing with
that.  I just think that explicitly handling error values on every
single function call is silly.

In addition to what Peter said, I will point out that you're still "explicitly handling" the error value, you just happen to be doing it in a single line.  When your pattern is (ab)used, the function that does the handling is rarely within eyesight, and thus a maintainer reading it has no idea what it's doing without tracking it down.  I also hazard a guess that there would probably be more than one such wrapper (probably for different return types or counts), which means that it suddenly becomes more difficult to spot differences in error handling and visually validate that all errors of a particular variety are handled.  When the handling is inline, because our brains are incredibly good at visual pattern matching, these tasks become almost trivial.