Shell – Why is using && 75 times faster than if…fi and how to make code clearer

shellshell-script

I have the following working code:

largest_prime=1
for number_under_test in {1..100}
do
  is_prime=true
  factors=''
  for ((divider = 2; divider < number_under_test-1; divider++));
  do
    remainder=$(($number_under_test % $divider))
    [ $remainder == 0 ] && [ is_prime ] && is_prime=false && factors+=$divider' '
  done
  [ $is_prime == true ] && echo "${number_under_test} is prime!" || echo "${number_under_test} is NOT prime (factors= $factors)"  [ $is_prime == true ] && largest_prime=$number_under_test
done
printf "\nLargest Prime= $largest_prime\n"

This code runs quickly is 0.194 seconds. However I found the && is_prime= false a bit hard to read and it could look (to the untrained eye) as if it was being tested rather than being set which is what it does.
So I tried changed the && into an if...then and this works – but is 75 times slower at 14.48 seconds. It's most noticeable on the higher numbers.

largest_prime=1
for number_under_test in {1..100}
do
  is_prime=true
  factors=''
  for ((divider = 2; divider < number_under_test-1; divider++));
  do  
    remainder=$(($number_under_test % $divider))
    if ([ $remainder == 0 ] && [ $is_prime == true ]); then
      is_prime=false
      factors+=$divider' '
    fi  
  done
  [ $is_prime == true ] && echo "${number_under_test} is prime!" || echo "${number_under_test} is NOT prime (factors= $factors)"  [ $is_prime == true ] && largest_prime=$number_under_test
done  
printf "\nLargest Prime= $largest_prime\n"

Is there any was to have the clarity of the block without the slowness?

Update (1/4/2015 10:40am EST)

Great feedback! I am now using the following. Any other feedback ?

largest_prime=1
separator=' '
for number_under_test in {1..100}; {
  is_prime=true
  factors=''
  for ((divider = 2; divider < (number_under_test/2)+1; divider++)) {
    remainder=$(($number_under_test % $divider))
    if [ $remainder == 0 ]; then
      is_prime=false
      factors+=$divider' '
    fi
  } 
  if $is_prime; then
    printf "\n${number_under_test} IS prime\n\n"
    largest_prime=$number_under_test
  else
    printf "${number_under_test} is NOT prime, factors are: "
    printf "$factors\n"
  fi
}
printf "\nLargest Prime= $largest_prime\n"

Best Answer

That's because you're spawning a sub-shell every time:

if ([ $remainder == 0 ] && [ $is_prime == true ]); then

Just remove the parentheses

if [ $remainder == 0 ] && [ $is_prime == true ]; then

If you want to group commands, there's syntax to do that in the current shell:

if { [ $remainder == 0 ] && [ $is_prime == true ]; }; then

(the trailing semicolon is required, see the manual)

Note that [ is_prime ] is not the same as [ $is_prime == true ]: you could write that as simply $is_prime (with no brackets) which would invoke the bash built-in true or false command.
[ is_prime ] is a test with one argument, the string "is_prime" -- when [ is given a single argument, the result is success if the argument is non-empty, and that literal string is always non-empty, hence always "true".

For readability, I would change the very long line

[ $is_prime == true ] && echo "${number_under_test} is prime!" || echo "${number_under_test} is NOT prime (factors= $factors)"  [ $is_prime == true ] && largest_prime=$number_under_test

to

if [ $is_prime == true ]; then
  echo "${number_under_test} is prime!"
else 
  echo "${number_under_test} is NOT prime (factors= $factors)"
  # removed extraneous [ $is_prime == true ] test that you probably
  # didn't notice off the edge of the screen
  largest_prime=$number_under_test
fi

Don't underestimate whitespace to improve clarity.

Related Question