HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Fizz having an argument with Buzz

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
buzzargumentwithhavingfizz

Problem

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.

Solution

Two points:

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

source ./buzz.sh $i


-
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

Code Snippets

source ./buzz.sh $i
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
}
for i in $(seq $1); do buzz $i; done
{ echo foo; echo bar; echo baz; } | tac

Context

StackExchange Code Review Q#96929, answer score: 8

Revisions (0)

No revisions yet.