Why I consider if/else idiomatic Go

August 23, 2016

Update: Why I still consider if/else idiomatic Go

Please adhere to the Gopher values. I’m not writing this to criticize anyone nor claim right and wrong. This is simply an attempt to contribute to the community to try and improve our standards.

Introduction

I love and strive for beautifully designed and styled code. But not at the cost of correctness. This post is about resolving a bug, not about a preferred coding style. Just like we have bugs in our code, I believe we have a bug in an idiom.

Contrary to Go Code Review Comments, I believe the if/else style should be idiomatic Go instead of the if/fall-through style.

I also believe the if/else style adheres more to Go’s core design principles of simplicity by being more consistent (consistency is a simplification). Additionally, it eliminates the possibility of introducing several types of bugs without itself introducing any.

Preferring the if/fall-through style is effectively preferring looks over substance, correctness, simplicity, and accuracy (e.g. during maintenance).

Idiomatic?

This is what most consider to be idiomatic Go:

if err != nil {
  // error handling
  return // or continue, etc.
}
// normal code?

I suspect that style is probably mostly influenced by Go Code Review Comments. Logically, it literally means:

if err != nil {
  // error handling
}
// depending on the error handling, possibly execute this

But that is almost certainly a fairly severe logic-bug since what is typically intended is:

if err != nil {
  // error handling
} else {
  // if and only if there isn't an error, execute this
}

In the if/fall-through idiom, normal flow depends on the result of the error handling logic. For every fall-through, it exposes all subsequent code to the results of the prior error handlers. That is a severe bug in the logic of most algorithms.

Remember the very high profile goto fail bug? Part of the reason Go requires curly braces is to avoid such bugs. But a similarly easy to code bug in Go is still possible due to the fall-through idiom (just an example):

if err != nil {
  // error handling
  if !logging {
    return err // or continue, etc.
  }
}
// normal code?
return nil

In the above, the code isn’t necessarily wrong so there is no compiler error. Even go vet and linters allow it. But it’s almost certainly a bug.

But using if/else:

if err != nil {
  // error handling
  if !logging {
    return err // or continue, etc.
  }
} else {
  // normal code
  return nil
}

At worst, that will compile but won’t execute the normal code when there’s an error. At best, that’s the end of the function body and the compiler will catch the error with missing return at end of function.

Conclusion: The correctness of the code is always more important than its style.

Initialization and habits

Again, the Go Code Review Comments has a problem. It recommends:

x, err := f()
if err != nil {
  // error handling
  return
}
// use x

There are a few problems with preferring that style.

Habits

It encourages ignoring error returns because it’s easy to. Let’s say you’re just hacking a quick solution:

x, _ := f()

or even getting the error and just not coding the test.

x, err := f()

That’s an incredibly horrible habit. It’s so horrible, I use this Dash snippet to ensure I never code that:

,_:= =

, err := @cursor; err != nil {
  return err
} else {

}

But using the if/else initialization statement makes it difficult to ignore the error. A good habit to have. Habits mean even when hacking.

if x, err := f(); err != nil {
  // error handling
} else {
  // use x
}

Conclusion: Good, consistent habits are a type of cognitive freedom.

Initialization

But maybe more important is the initialization statement’s scoping of variables. The consequence of the scoping is actually two-fold.

First, it’s just a good software development practice to only allow variables to be scoped to their actual usage needs.

Second, if/else can be less verbose while remaining stylistically consistent.

I’ve seen huge functions that can be summarized to this:

u, err := k()
if err != nil {
  return -1.0, err
}

x, err := f(u)
if err != nil {
  return -1.0, err
}

y, err := g(u)
if err != nil {
  return -1.0, err
}

if x < 0.0 && u > 12.0 {
  return -1.0, fmt.Errorf("Bad value")
}

z, err := h(y)
if err != nil {
  return -1.0, err
}
return z, nil

You might prefer to read that rather than the similar if/else style below but there are several problems inherent in the code above that are not present when using if/else.

Why does the variable x exist? The purpose of f is simply the determine if there’s an error in u but you can’t ignore the return because it doesn’t compile: no new variables on left side of :=. Alternatively, you can

_, err1 := f(u)
if err != nil {
  return -1.0, err
}

But that leaves you open to accidental copy/paste error like the one I introduced above. It should be:

_, err1 := f(u)
if err1 != nil {
  return -1.0, err1
}

Not only is that an easy error to introduce, it would be difficult to track down because

  1. testing such structures is complicated by the number of conditionals in the function.
  2. the faulty if-statement looks like all the others so is easy to presume it’s correct.

Another downside is simply that it introduces another variable for the remainder of the scope.

But the same logic using if/else will not only compile using a consistent style and variable naming, it is actually more logically correct, accurate and even more concise, albeit, yes, less pretty. Consequently, the code below is actually a correct representation of the intended logic for this algorithm:

if u, err := k(); err != nil {
  return -1.0, err

} else if _, err := f(u); err != nil {
  return -1.0, err

} else { // This indentation implies a successful branch, meaning a good value for u

  if y, err := g(u); err != nil {
    return -1.0, err

  } else if y < 0.0 && u > 12.0 {
    return -1.0, fmt.Errorf("Bad value")

  } else if z, err := h(u); err != nil {
    return -1.0, err

  } else {

    return z, nil
  }
}

Another consequence of this style is that it encourages the writing of functions. Since you are probably averse to deep nesting, more than one/two levels of nesting is an indication to write functions to perform the nested operations. With the fall-through style, it actually facilitates writing long, complex functions since you’re avoiding the nesting inherent in the algorithm. Long functions impede testability. I think habits that encourage writing smaller functions are good habits.

Conclusion: Consistency frees you from several possible types of errors while encouraging better algorithm design.

Summary

The if/else style should be idiomatic Go.

And, yes, of course the if/fall-through style is still useful since its logic is a valid use-case. I’m only saying that the if/else style is more inline with Go’s simplicity of design principle, it’s safer, and it’s almost universally, consistently applicable.

Consistency and uniformity are important simplifications for program design. Providing a clear path for thought is more important than a slight improvement to appearances. To put it differently, if/else allows you to think more about what you want to write instead of how you have to write it because of if/else’s more consistent usage pattern and its safer semantics.

Discussion, links, and tweets

comments powered by Disqus