Best practice for if/else when one outcome results in exit [Bash]

alphaniner · 2012-06-19 01:51:07

I have a bash script with a lot of if/else constructs in the form of

if <condition>
then
	<do stuff>
else
	<do other stuff>
	exit
fi

This could also be structured as

if ! <condition>
then
	<do other stuff>
	exit
fi
<do stuff>

The first one seems more structured, because it explicitly associates <do stuff> with the condition. But the second one seems more logical because it avoids explicitly making a choice (then/else) that doesn't really need to be made.

Is one of the two more in line with "best practice" from pure bash or general programming perspectives?

Trilby · 2012-06-19 02:02:33

I'm not sure if there are 'formal' best practices, but I tend to use the latter form when (and only when) it is some sort of error checking.

Essentially, this would be when <do stuff> was more of the main purpose of the script, or at least that neighborhood of the script, while <do other stuff> was mostly cleaning up before exiting.

I suppose more generally, it could relate to the size of the code blocks. You wouldn't want a long involved <do stuff> section after which a reader would see an "else" and think 'WTF, else what?'. So, perhaps if there is a substantial disparity in the lengths of the two conditional blocks, put the short one first.

But I'm just making this all up from my own preferences and intuition.

When nested this becomes more obvious, and/or a bigger issue. Consider two scripts:

if [[ test1 ]]
then
    if [[ test2 ]]
    then
        echo "All tests passed, continuing..."
    else
        echo "failed test 2"
        exit
    fi
else
    echo "failed test 1"
fi

if [[ ! test1 ]]
then
    echo "failed test 1"
    exit
fi
if [[ ! test2 ]]
then
    echo "failed test 2"
    exit
fi
echo "passed all tests, continuing..."

This just gets far worse with deeper levels of nesting. The second seems much cleaner. In reality though I'd go even further to

[[ ! test1 ]] && echo "failed test 1" && exit
[[ ! test2 ]] && echo "failed test 2" && exit
echo "passed all tests, continuing..."

edit: added test1/test2 examples.

Last edited by Trilby (2012-06-19 02:27:48)

Yurlungur · 2012-06-19 02:05:05

I tend to agree with Trilby, but that's my own personal opinion as well.

alphaniner · 2012-06-19 03:33:48

In most cases the conditions are the script. What I mean is, I could technically do

<command1> >/dev/null || exit $?
<command2> >/dev/null || exit $?
...

That's largely what I did during the "prototyping" stage. But that leaves a lot to be desired, particularly when things go wrong). Once I had the concept down I started moving to

echo -n "Activating $lv ..."
echo "Activating $lv" >>"$log"
if ! lvchange -vay "$lv" &>"$tmp"
then
	echo " Failed"
	cat "$tmp" >>"$log"
	exit <code>
fi
echo " OK"

echo -n "Mounting $lv..."
echo "Mounting $lv" >>"$log"
if ! mount "$lv" "$dir" &>"$tmp"
then
	echo " Failed"
	cat "$tmp" >>"$log"
	exit <code>
fi
echo " OK"

Though I now use a function for the code in the numerous "then" blocks with that structure.

In this kind of situation, using if/then/else just seems more appropriate.

Last edited by alphaniner (2012-06-19 03:34:42)

Trilby · 2012-06-19 03:48:10

The situation dictates.

Another alternative would be something lilke

function fail() {
  case $1 in
    lvchange)
      <error message, save tmp, etc>
      ;;
    mount)
      <other error message + cleanup>
      ;;
  esac
  exit $err_code
}

echo -n "activating lv ..."
echo "activating lv" >> $log
lvchange -val "$lv" &> "$tmp" || fail lvchange
...
mount "$lv" "$dir" &>"$tmp" || fail mount

edit: D'Oh, upon rereading, I'm guessing this is about what you meant by using a function instead of the then blocks. If not this may be another alternative, if so, then great minds think alike.

Last edited by Trilby (2012-06-19 03:49:15)

fukawi2 · 2012-06-19 05:40:14

My preference is usually for the second form.

alphaniner wrote:

In most cases the conditions are the script. What I mean is, I could technically do
<command1> >/dev/null || exit $?
<command2> >/dev/null || exit $?
...

I like to do it like this:

<command1> >/dev/null || { echo "Error message blah blah blah"; exit $?; }
<command2> >/dev/null || { echo "Error message blah blah blah"; exit $?; }
...

alphaniner · 2012-06-19 13:17:02

fukawi2 wrote:

<command1> >/dev/null || { echo "Error message blah blah blah"; exit $?; }
...

Wouldn't the $? be the status of the echo command?

Trilby wrote:

If not this may be another alternative, if so, then great minds think alike.

Unfortunately I can't accept that compliment. What I'm doing is actually

function fail {
	cat "$tmp" >>"$log"
	rm "$tmp"
	echo -e "$1"
	[[ $2 ]] && exit "$2"
}

echo -n "Activating $lv ..."
echo "Activating $lv" >>"$log"
if ! lvchange -vay "$lv" &>"$tmp"
then
	fail "message" "exit status"
fi
echo " OK"

As you can see the function is designed for "non-fatal" errors as well, but I always use if/then/else in that case.

Last edited by alphaniner (2012-06-19 13:17:47)

fukawi2 · 2012-06-19 22:56:15

alphaniner wrote:

fukawi2 wrote:
<command1> >/dev/null || { echo "Error message blah blah blah"; exit $?; }
...
Wouldn't the $? be the status of the echo command?

Whoops, yes... I actually normally have a fixed exit code, and maintain a list of the codes at the top of the script. I just edited your example without paying too much attention to show you the structure I meant

Last edited by fukawi2 (2012-06-19 22:56:34)

Awebb · 2012-06-20 05:38:08

Well, do it like you'd do it in spoken language and say what you mean:

"If n is not 3, call func, if not do nothing"

If you go like this...

if n=3
then
    exit 0
else
    func
fi

... then the main use for this function is doing nothing. This reads like you pretty much expect n to be 3 and then want that thing to do nothing.

if ! n=3
then
    func
else
    exit 0
fi

...is the precise translation of the original thought. If it's NOT, then do something. You know, unlike telling people NOT to call the ambulance, when they're NOT being shot…

brebs · 2012-06-20 09:18:37

Trilby wrote:

[[ ! test1 ]] && echo "failed test 1" && exit

I would say that's bad practice. Because if the echo fails, then the exit *won't* get run - a logic error. You want to exit *regardless* of whether the echo is successful, not be dependent on the echo being successful.

Don't think to yourself, "echo would never fail" - that's not the point. The point is not to introduce such unnecessary logic dependencies.

Sadly, BASH encourages the writing of crap code.

Awebb · 2012-06-20 10:48:38

That's why my code always looks like as if ten monkeys attepted to date-rape a pack of hungry aligators. Messy, messy, messy...

Trilby · 2012-06-20 12:47:17

function monkeys_fight() {
    case "$1" in
        over_victim)
            while still_fighting; do alligator_attempt_escape; done ;;
        for_fun)
            throw_poop ;;
        *)
            echo "Why are they fighting?"
    esac
}

if [[ ${#Alligators} -lt 10 ]]; then
   monkeys_fight over_victim
elif [[ ${#Alligators} -gt 10 ]]; then
   let n_safe_alligators=${#Alligators} - 10
else
  let n_safe_alligators=0
fi

Last edited by Trilby (2012-06-20 12:48:43)

alphaniner · 2012-06-20 13:25:07

Awebb wrote:

Well, do it like you'd do it in spoken language and say what you mean:
...

Well, I could either say "If the command succeeds, then do stuff, otherwise do other stuff and exit." or "If the command fails, then do stuff and exit, otherwise do other stuff." Since the commands are the script, and the "[other] stuff" is just reactions to the outcome of commands (logging and user notification) neither seems particularly superior.

Furthermore, the real issue is due to the fact that one outcome results in exiting the script. In this case the following two constructs are functionally identical

if <command>
then do stuff
else { do other stuff; exit; }
fi

if ! <command>
then { do other stuff; exit; }
fi
do stuff

The question is, is the second construct bad form?

progandy · 2012-06-20 13:43:49

The question is, is the second construct bad form?

I don't consider it bad style when it is used to catch errors. The normal flow of the script doesn't enter the conditional blocks, which only exist to react to problems. If both choices result in a valid state, then if - else is better.

Arch Linux

#1 2012-06-19 01:51:07

Best practice for if/else when one outcome results in exit [Bash]

#2 2012-06-19 02:02:33

Re: Best practice for if/else when one outcome results in exit [Bash]

#3 2012-06-19 02:05:05

Re: Best practice for if/else when one outcome results in exit [Bash]

#4 2012-06-19 03:33:48

Re: Best practice for if/else when one outcome results in exit [Bash]

#5 2012-06-19 03:48:10

Re: Best practice for if/else when one outcome results in exit [Bash]

#6 2012-06-19 05:40:14

Re: Best practice for if/else when one outcome results in exit [Bash]

#7 2012-06-19 13:17:02

Re: Best practice for if/else when one outcome results in exit [Bash]

#8 2012-06-19 22:56:15

Re: Best practice for if/else when one outcome results in exit [Bash]

#9 2012-06-20 05:38:08

Re: Best practice for if/else when one outcome results in exit [Bash]

#10 2012-06-20 09:18:37

Re: Best practice for if/else when one outcome results in exit [Bash]

#11 2012-06-20 10:48:38

Re: Best practice for if/else when one outcome results in exit [Bash]

#12 2012-06-20 12:47:17

Re: Best practice for if/else when one outcome results in exit [Bash]

#13 2012-06-20 13:25:07

Re: Best practice for if/else when one outcome results in exit [Bash]

#14 2012-06-20 13:43:49

Re: Best practice for if/else when one outcome results in exit [Bash]

Board footer