44

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

3
  • 5
    Not being able to see the other code, I’d suspect the senior engineer saw the echo command in possibly a smaller sized terminal, then kept reading and wondered why you were still carrying out business logic after an error occurred, then spent extra time scrolling back up to find the non-explicit exit statement attached to an echo command. Can’t say for sure, but it seems you’re indenting with spaces whereas they may be indenting with tabs. Jan 4 at 3:18
  • 31
    In your code, if the echo doesn't work for whatever reason, the exit 1 will not fire. Not only will it not return the intended exit code (upon which the caller may depend), but the shell will continue to run whatever is below the if statement. I doubt that is what you intended, is it?
    – Nate T
    Jan 4 at 13:55
  • 4
    Another way to think about the review comment is: "On your echo line, why are you making the 'exit 1' conditional on the echo succeeding?" -- probably you want to always exit 1 as soon as you end up within that if-clause, so there is no point in adding a conditional saying that it should only be done if the echo succeeds (which is what && means)
    – pzkpfw
    Jan 9 at 13:55

5 Answers 5

93

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
9
  • Is there any reason why we prefer having the && inside the [[ ... ]] condition?
    – IsaIkari
    Jan 3 at 15:47
  • 10
    I find it more readable. It's also a bit faster.
    – choroba
    Jan 3 at 15:49
  • 26
    @IsaIkari, see also [ -n "$VAR_A" ] && [ -n "$VAR_B" ] which is standard sh syntax (and then you can remove the dependency to bash). Jan 3 at 17:36
  • 5
    @G.Sliepen, no -a and -o binary [ operators should never be used. They make for unreliable test expressions and are deprecated. Jan 5 at 11:29
  • You should not say that one can put the "&&" inside the test itself... it is not as much portable. It is recommended to chain them up (such as in the exemple above by Stephane Chazelas) and keep each [ something ] containing only 1 (unary or binary) condition in it. Jan 6 at 13:09
66

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!' 
4
  • 29
    Also @IsaIkari, if you wanted to execute two commands independently but really wanted them to be inside one line (for aesthetic reasons, saving screen estate or whatever), you would use ; instead of && - eg. echo foo; exit 1. That would work the same as two-line variant Jan 4 at 0:51
  • 6
    echo would fail if the output stream was closed. I've seen some users post code that does e.g. 2>&- rather than 2>/dev/null recently, and if that was done to the stream used by echo then the utility would fail and exit would not be called.
    – they
    Jan 4 at 15:42
  • 1
    @they yes, that's one of the very unlikely cases, indeed.
    – terdon
    Jan 4 at 15:44
  • 1
    Everyone, I know this is on the network's Hot Questions list, but please remember to only leave comments if you are requesting a clarification or pointing out a mistake in the answer. If you have a new solution, please post an answer. Comments should not be used for discussion, so if you want to discuss the general merits or drawbacks of &&, or speculate on what this senior engineer may have been thinking, please don't do it here.
    – terdon
    Jan 5 at 10:18
22

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.

2
  • 4
    👍🏽 that's the main point for me. Why would you decide to exit only if echo succeeds, that makes no sense. If echo fails, on the contrary, that's one more reason for wanting to abort asap. You could even write it: echo>&2 error || exit; exit 1, that is exit with echo's failure exit status if echo failed, and exit with code 1 otherwise. Jan 5 at 8:18
  • 3
    @StéphaneChazelas exit with echo's failure exit status if echo failed IMO that's not a good idea. If the exit code from this script means anything, replacing it with the exit code from echo will break that. If the exit code from the script doesn't mean anything but that meaning is added in the future, exiting with echo's failed exit status only when echo fails opens the door to one very hard-to-find, just-about-impossible-to-recreate bug. Should meaning be added to the value of the script exit status, it's very likely that the echo ... || exit; will be missed. Jan 5 at 18:52
18

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.

2
  • 2
    [[ -n $VAR_A && -n $VAR_B ]] is not bash-specific, it comes from ksh and is also supported by zsh and yash in addition to bash (but is not valid sh syntax) Jan 4 at 15:26
  • 1
    The "on one line or not" question is also relevant to coding style; as @Johnny Henly commented yesterday, it's easier to miss while skimming the code when it's off to the right. (Perhaps on a narrow terminal or in a side-by-side diff). To humans, a separate line inside the if is extra obvious that this is a separate effect. Jan 5 at 6:08
1

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.

1
  • 1
    The && between two commands means "only execute the second command if the first one completes successfully" - the extension to three or more commands is obvious. Also, strings of commands can stretch over several lines. In this case a nested if statement could have been used! Jan 12 at 1:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.