Why avoid using "&&" in bash script?

Question

I am writing a bash script which contains a simple if section with two conditions:

  if [[ -n $VAR_A ]] && [[ -n $VAR_B ]]; then
    echo >&2 "error: cannot use MODE B in MODE A" && exit 1
  fi

A senior engineer reviewed my code and commented:

please avoid using && when you could simply execute the two commands in subsequent lines instead.

He didn't further explain. But out of curiosity, I wonder if this is true, and what is the reason for avoiding using &&.

Answer 1

The review comment probably refers to the second usage of the && operator. You don't want to not exit if the echo fails, I guess, so writing the commands on separate lines makes more sense:

if [[ -n $VAR_A ]] && [[ -n $VAR_B ]]; then
    echo >&2 "error: cannot use MODE B in MODE A"
    exit 1
fi

BTW, in bash you can include && inside the [[ ... ]] conditions:

if [[ -n $VAR_A && -n $VAR_B ]]; then

Answer 2

This is not a general comment against &&. I suspect the engineer you spoke to was considering the (very unlikely, but still theoretically possible) case where the echo itself failed. If you have this:

if error; then
    echo foo && exit
fi

Then, if for whatever reason the echo command fails, the exit will not run, so you will not actually catch the error. If you have them separate, the exit will run even if the echo fails:

if error; then
    echo foo
    exit 
fi

So the engineer is indeed right that it is safer to separate these two specific statements. However, don't interpret that as a general directive against using &&. You should simply make sure to use && only in the cases where you really do want to make the execution of one command contingent upon the successful execution of another.

For example, this is perfectly fine:

command && echo 'command worked!' 

Answer 3

Folks have made a good case for echo failing. In addition to this point about correctness, you could also make the case for readability. You wrote

echo >&2 "error: cannot use MODE B in MODE A" && exit 1

When I translate that to English in my head, I get something like:

print the following to standard error: "error: cannot use MODE B in MODE A"
if the print operation completed successfully, then exit with code 1

Someone reading this is likely to ask, "why did you bother specifying if the print operation completed successfully"? In this case, the two lines are simpler than the && because there is less logic in the code. An English version of the recommended two-line alternative is:

print the following to standard error: "error: cannot use MODE B in MODE A"
then exit with code 1

That's less complex so it takes less brain power to read it. We're not left wondering what the && is for.

Answer 4

Put simply, you should not assume that code executed in an exit path will not cause an error itself. This is not specific to shell script, but just good programming advice in general.

By using echo ‘foo’ && exit, you will only exit if the echo 'foo' succeeds. The likelihood of that failing is indeed exceedingly low, but you should not count on that, especially when the cost of doing it right is so small. This becomes significantly more important when you have a more complicated exit path that may invoke other functions in your code and it’s not obvious that your exit path will only invoke ‘mostly safe’ stuff like echo, but it’s generally good form to use ‘correct’ code in all cases so you get in the habit of doing it properly (and so your colleagues know what to expect).

If, for some reason, you need that to be one line (I would argue this should never be the case though), the correct syntax is echo 'foo' ; exit, which causes the parser to treat the two commands as if they were on separate lines.

---

As an aside, in your conditional, you should almost always prefer either:

if [[ -n $VAR_A && -n $VAR_B ]]; then

which is bash-specific but more efficient than what you have currently, or:

if [ -n "$VAR_A" ] && [ -n "$VAR_B" ]; then

which will work in a semantically equivalent manner in any POSIX-compliant shell.

Answer 5

There's no need to write two commands on one line. If you had 100 commands in your if statement you wouldn't && them together because it would be horrible to read. Whilst other responses concerning the possibility of echo failing are technically correct, I suspect the senior devs intent was about readability.