Assignmnents and Ifs

Should assignment statements be inlined into if statements by default?

November 30, 2019

TL;DR

The essence of "good design" is whether the design choice makes the software easier to change ("The Pragmatic Programmer", ETC, p. 28). An assignment statement and an if statement have different reasons for changing and should therefore remain decoupled except where limiting scope matters enough to justify the inline operation. Inlining assignment statements into if statements is a feature of last resort and not a default behavior.

Introduction

Consider the following example:

Example A

err := someOperationThatMightFail()
if err != nil {
    log.Panic(err)
}

Go supports inlining assignment statements into if/else statements:

Example B

if err := someOperationThatMightFail(); err != nil {
    log.Panic(err)
}

Implications

After transforming "Example A" into "Example B" the code is more concise in that it takes up fewer lines and the err variable is scoped to the if/else statement ONLY. Both of those results could be judged as 'good' or 'bad' depending on the context.

In most cases having concise code isn't anywhere near a top priority, nor is limiting the scope of an error. Most of our functions should be very small, which already serves to limit the scope of a variable. If we have to resort to scoping tricks like this regularly it probably means our functions are doing too much with too many things. There's a design problem and it's time to refactor.

Another unhelpful side effect of inlining assignments into if statements is that the actual source code becomes harder to read and work with because 1) it's harder (or impossible) for the editor to perform extractions and other helpful operations and 2) you are now combining two orthogonal concepts (which change for different reasons) into a compound statement.

An assignment and an if statement have different reasons for changing and should remain decoupled except where limiting scope matters enough to justify the inline operation.

Comparision

We should prefer shorter lines and more whitespace between logical operations to allow for some breathing room. Consider the following examples:

Bad:

if value1, err1 := blahBlahBlah(); err1 != nil {
    log.Panic(err1)
} else if value2, err2 := yackitySmackity(value1); err2 != nil {
    log.Panic(err2)
} else {
    fizzBuzz(value1, value2)
}

Good:

value1, err1 := blahBlahBlah()
if err1 != nil {
    log.Panic(err1)
}

value2, err2 := yackitySmackity(value1)
if err2 != nil {
    log.Panic(err2)
}

fizzBuzz(value1, value2)

It's much easier to change/rework the 'good' example. It is therefore, a better design choice in most situations.

Conclusion

Having said all of that I propose that our default code layout NOT include inlining assignments into if statements.

-Michael Whatcott