0

I am reviewing best practice with if statements. Below in example A I include the entire code to be run and ensure it remains within the if statement. While for example B the code to be executed resides outside the if statement.

From my understanding of Best practice on if/return, example A appears to be best practice.

Example A

#!/bin/bash
if [ `whoami` != 'root' ]
then
  echo "MUST be ROOT!"
  exit
else
  ifconfig -a | sed 's/[ \t].*//;/^\(lo:\|\)$/d'
  read -p "Enter interface: " int
  mac=$(macchanger $int | grep 'Permanent MAC:' |  grep -o -E '([[:xdigit:]]{1,2}:){5}[[:xdigit:]]{1,2}')
  ifconfig $int down
  iwconfig $int mode manage
  macchanger $int $mac
  ifconfig $int up
  echo "$int in stock configuration with MAC: $mac"
  echo "Connect to a Wi-Fi network and run 'ping 1.1.1.1'"
  sleep 2 && cinnamon-settings network &
fi

Example B

#!/bin/bash
if [ `whoami` != 'root' ]
then
  echo "MUST be ROOT!"
  exit
fi
ifconfig -a | sed 's/[ \t].*//;/^\(lo:\|\)$/d'
read -p "Enter interface: " int
mac=$(macchanger $int | grep 'Permanent MAC:' |  grep -o -E '([[:xdigit:]]{1,2}:){5}[[:xdigit:]]{1,2}')
ifconfig $int down
iwconfig $int mode manage
macchanger $int $mac
ifconfig $int up
echo "$int in stock configuration with MAC: $mac"
echo "Connect to a Wi-Fi network and run 'ping 1.1.1.1'"
sleep 2 && cinnamon-settings network &

I can foresee a potential example C, which would include structure:

if [ `whoami` != 'root' ]
then
  echo "MUST be ROOT!"
  exit
if [whoami' == 'root' ]
  then
   ...
fi

However, this seems like creating an extra unnecessary condition, which uses additional resources. So, I will disregard example C.

  • 2
    Actually, the question you already linked (Best practice on if/return) looks like a much better duplicate question. What specifically is your question that is not yet answered by that question? Both your examples A and B are discussed there in depth, and I agree with you that example C only introduces unnecessary duplication. – amon Aug 13 '18 at 19:35
  • @amon - my question does not involve return and used an exit condition within the first if statement. The answer involved a summary of which example A or example B is better practice and possibly a comment regarding the exit condition being within the if statement or whether it should be at the end. – safesploit Aug 13 '18 at 20:07
  • 1
    but an exit from a shell script is effectively a return with respect to the surrounding control flow. Any advice about if/return can probably be generalized to if/exit in Bash. – amon Aug 13 '18 at 20:46
  • 1
    @amon I was unaware of this fact "return exits from the function while exit exits from the program." – safesploit Aug 13 '18 at 21:51
3

In this particular case, I don't think there is much to recommend approach A versus approach B, or vice versa.

However, I disagree that in general it's better to use

if A then 
  stuff
else 
  other stuff
endif

versus:

if A then 
  stuff;
  exit/return;
endif

other stuff;

The problem with the nesting approach, is that when applied rigorously, you can get alot of nesting, and nesting is hard for humans to understand.

IMHO, the following is often simpler and clearer:

  if A then
    check any boundary cases that dont have mcuh to do the the core logic.
    either throw, or return or whatever makes sense.
  end;

  ASSERT (!A); /// and any further assertions of stuff you checked above.

  // then freely write code ignoring those corner cases as already handled.
  • I was beginning to believe the best practice was the "best way", although you are right. Nesting is more difficult to read for humans, and most text editors do not clearly show the beginning-to-end of nesting, which makes me feel less efficient when programming when I become lost in a maze of code. – safesploit Aug 13 '18 at 20:01

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