24

I see here that testing whether $? is zero (success) or something else (failure) is an anti-pattern, but I have not been able to find this anywhere else.

Sticking to the definition of anti-pattern of the Wikipedia: "An anti-pattern (or anti-pattern) is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive." Why would this be an anti-pattern?

kvantour
  • 25,269
  • 4
  • 47
  • 72
svye
  • 343
  • 2
  • 5
  • 3
    "Usually ineffective" is a poorly-thought-through component of that definition. Something can be an antipattern while working more than 50% of the time, if there's a best-practice alternative that addresses a major pitfall or caveat in same without any negative tradeoff. Indeed, several things that are antipatterns are such because they introduce security risks; a security bug only needs to be exploited *once* to make the code with that vulnerability unacceptable, not on more than 50% of executions. – Charles Duffy Mar 30 '16 at 15:53
  • 1
    ...unless it means "usually" in that *most*, but not all, uses of antipatterns tend to be ineffective; that's fuzzier, and thus more difficult to dispute. – Charles Duffy Mar 30 '16 at 17:30
  • 1
    See [BashPitfalls #44 (cmd; (( ! $? )) || die)](https://mywiki.wooledge.org/BashPitfalls#cmd.3B_.28.28_.21_.24.3F_.29.29_.7C.7C_die). – pjh Feb 10 '20 at 18:29

2 Answers2

41

This is an antipattern because it introduces complexity that wouldn't exist if you didn't require the exit status to be recorded at all.

if your_command; then ...

has much less to go wrong than

your_command
if [ "$?" -eq 0 ]; then ...

For examples of things that can go wrong: Think about traps, or even new echo statements added for debugging, modifying $?. It's not visually obvious to a reader that a separate line running your_command can't have anything added below it without changing logical flow.

That is:

your_command
echo "Finished running your_command" >&2
if [ "$?" -eq 0 ]; then ...

...is checking the echo, not the actual command.

Thus, in cases where you really do need to deal with exit status in a manner more granular than immediately branching on whether its value is zero, you should collect it on the same line:

# whitelisting a nonzero value for an example of when "if your_command" won't do.
your_command; your_command_retval=$?
echo "Finished running your_command" >&2 ## now, adding more logging won't break the logic.
case $your_command_retval in
  0|2) echo "your_command exited in an acceptable way" >&2;;
  *)   echo "your_command exited in an unacceptable way" >&2;;
esac

Finally: If you enclose your_command inside of an if statement, this marks it as tested, such that your shell won't consider a nonzero exit status for purposes of set -e or an ERR trap.

Thus:

set -e
your_command
if [ "$?" -eq 0 ]; then ...

...will never (barring a number of corner cases and caveats which plague set -e's behavior) reach the if statement with any value of $? other than 0, as the set -e will force an exit in that case. By contrast:

set -e
if your_command; then ...

...marks the exit status of your_command as tested, and so does not consider it cause to force the script to exit per set -e.

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • Interesting. But only if it's used as a binary/boolean is it an anti pattern – KevinDTimm Mar 30 '16 at 15:30
  • 2
    @KevinDTimm, indeed. The language feature has use; using it when it's not called for is what's frowned on. – Charles Duffy Mar 30 '16 at 15:31
  • 1
    @KevinDTimm, ...and the the title explicitly calls out the use case of "see if a command succeeded or not" -- if that command follows zero/nonzero exit status conventions, then determining whether it succeeded really is a boolean decision. – Charles Duffy Mar 30 '16 at 15:32
  • It usually is used as a boolean. Taking different actions for different non-zero values of `$?` is relatively rare. – Keith Thompson Mar 30 '16 at 15:33
  • 1
    After so many years of writing software it wouldn't occur to me to use the incorrect form except maybe in the case of early development effort where I'm interested in the output for debug purposes. As has been said, do not add unnecessary complexity. – KevinDTimm Mar 30 '16 at 15:35
  • @KevinDTimm I would argue that using exit codes to communicate information is the anti-pattern. 0 for success. 1 for failure. Anything else is line noise. – William Pursell Oct 10 '21 at 12:22
  • 3
    @WilliamPursell, distinguishing between different types of failure is sometimes meaningful and useful -- I do have places in production code where I want to ignore error X but not error Y -- and while needing to inspect `$?` is not clean code, it's not as nasty as trying to inspect text written to stderr (and the fragility, localization issues, etc. that accompany same) – Charles Duffy Oct 10 '21 at 14:12
0

In my opinion this is an anti pattern if you don't need return value of the command and just need to check status.

If we need function return value and exit code there is no other way (or i don't know it, correct me if i am wrong). In example below we need to proceed only if previous command was succesfull and we are using return value in other function

Example:

test() { echo "A" ; return 1; }
l_value=$(test); l_exit_code=$?;

if (( l_exit_code == 0 )); then
   fn_other_function_to_pass_value $l_value
else
   echo "ERROR" >&2
   exit 1
fi
Arkon88
  • 180
  • 5
  • 3
    There is another way. You can do `if l_value=$(test); then ...`. The exit status of a simple command consisting solely of assignment statements is the exit status of the command that is substituted the last (i.e. the rightmost `$(...)` in all modern shells), or 0 if no command is substituted. For example the exit status of `a=b` is 0, `a=$(exit 1)` is 1, `a=$(exit 1)$(exit 2)` is 2, and `a=$(exit 1) b=$(exit 2)` is 2. There is no consensus among shells as to what the exit status of such commands should be when redirections are involved (e.g. `a=$(exit 1) >$(echo /dev/null)`) though. – oguz ismail Oct 10 '21 at 19:11