If statement best practice

Question

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.

Actually, the question you already linked ([Best practice on if/return](https://softwareengineering.stackexchange.com/q/157407)) 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
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
@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

score 3 · Accepted Answer · answered Aug 13 '18 at 19:39

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

If statement best practice

1 Answers1