Fizz having an argument with Buzz

Question

For my current project I need to validate responses. The requests will be send from multiple different shell scripts and should be controlled from another script. I'd never targeted a .sh with a .sh before, so let's try some FizzBuzz first.

Fizz.sh

#!/bin/bash

if [ $# -eq 0 ]
then
    echo "This FizzBuzz is interactive. Please provide the upper limit."
    echo "Usage : $0 limit"
    exit 1
fi

for i in `seq $1`
do
   echo `source ./buzz.sh $i`
done

Buzz.sh

#!/bin/bash

([ $(($1%15)) -eq 0 ] && echo 'FizzBuzz') ||
([ $(($1%5)) -eq 0 ] && echo 'Buzz') ||
([ $(($1%3)) -eq 0 ] && echo 'Fizz') ||
echo $1;

• Is it idiomatic?
• Is this the best way to hand all output from Buzz to Fizz?

The goal is to learn decent BASH. The FizzBuzz has no need for optimization, it's mostly about style, how data should be passed from script to script and whether I followed BASH-practices or not.

Answer 1

If you're using source, you shouldn't have the shebang in the second script. If you're using the shebang, you shouldn't use source.

But rather than use source directly, I would source the script once at startup, and in it define a function that was called at the later times.

You should use {} instead of () to avoid forcing a separate subshell to spawn.

I would provide a default instead of exiting if no argument is passed. Something like limit=${1:-100}. Also, I always use named variables as early as possible.

Note that if you were using a function (with or without source), you should use local to avoid leakage.

Instead of `seq $1`, use for ((i=1; i<=limit; ++i)) to avoid precalculating and storing the entire sequence in memory (and also spawning an unnecessary subprocess).

Instead of [ $(($1%15)) -eq 0 ], use n=$1 to name the argument then use ((n % 15 == 0)). Note that let is an alias for (()), but it's idiomatic to use let for assignments and (()) for expressions.

Beware of the possibility of echo failing! Using an if-then-else chain as @glennjackman suggested will avoid this pitfall. It would also be possible to replace echo 'FizzBuzz' with {echo 'FizzBuzz'; true} but that's ugly IMO.

Answer 2

Two points:

1. echo $(something that already prints) is just redundant. This is simpler

   source ./buzz.sh $i
   

2. There does not appear to be any need for two separate files. Put buzz in a function. Also, you are using a lot of subshells that are unnecessary (a). I'd stick with the more verbose but clearer if

   buzz() {
       if (( $1 % 15 == 0 )); then
           echo 'FizzBuzz'
       elif (( $1 % 5 == 0 )); then
           echo 'Buzz'
       elif (( $1 % 3 == 0 )); then
           echo 'Fizz'
       fi
       echo $1
   }
   

   then

   for i in $(seq $1); do buzz $i; done
   

a) if you want a grouping construct without the overhead of spawning a new shell, use braces:

{ echo foo; echo bar; echo baz; } | tac