0

I didn't like the way the code that I had looked, so I compressed all of the main 'blocks' into functions. (This would also prevent the code from running if an error was present.) I don't know if this is bad practice or not. Here's the code (about the first line: bash-4.3 is installed in /usr/local/bin/bash.):

#!/usr/local/bin/bash
cat /dev/null > out.txt
resetcolor() {
    echo -ne "\e[0m"
}
color() {
    resetcolor
    if [ $# -eq 1 ]
    then
        echo -ne "\033[38;5;${1}m"
        elif  [ $2 == "b" ]
        then
            echo -ne "\033[1;38;5;${1}m"
    fi
}

prob() {
    x=$(echo "scale=4;($1/$c)" | bc)
    y=$(echo "scale=2;$x*100" | bc) 
    if   [[ ${y%??} == *".00"* ]] ; then echo "${y%?????}%"
    elif [[ ${y#????} == "000" ]] ; then echo "${y%????}%"
        elif [[ ${y#???} == "3300" ]] ; then echo "${y%?????}.33%"
    elif [[ ${y#???} == "6600" ]] ; then echo "${y%?????}.67%"
    else echo "${y%??}%"
        fi

}
readmessage() {
if [ $# -eq 1 ]
then
    c=$1
    echo "Flipping coins..."
    else
    echo "Enter number of coins to flip..."
    read c
    echo "Flipping coins..."
fi
}
flip() {
     < /dev/urandom LC_CTYPE=C gtr -dc HT | head -c $c | while read -n 1 ccoin
    do
        echo -n "$ccoin"
        echo "$ccoin" >> out.txt
    done
    echo -e "\n"
}
stats() {
if (($c<10)) 
then
    exit 1
else
    co=$( < out.txt )
    color 035
    echo "Heads:"
        h=`grep -o "H" <<<"$co" | wc -l`
        color 047 b
        echo $h
        resetcolor
        color 027 
    echo "Tails:"
        color 045 b
        t=`grep -o "T" <<<"$co" | wc -l`
        echo $t
        color 255 b
    echo " "
    echo "Experimental probability"     
        echo "Heads:"
            prob $h 
        echo "Tails:"
            prob $t
fi
}
###
readmessage $1
flip
stats
###
3
  • 5
    I'm voting to close this question as off-topic because it belongs on Code Review...
    – jasonwryan
    Jun 25 '15 at 7:04
  • @jasonwryan Oops, sorry, didn't know I should put it there (nor did I even know it was a thing, haha!) Jun 25 '15 at 7:36
  • What's the point of this? It's a coin flip? I think your code could be condensed to flip(){ return "$(tr -c 01 '[0*127][1*]|dd bs=1)"; } </dev/urandom
    – mikeserv
    Jun 25 '15 at 13:19
1

It's definitely not a bad practice. Using functions is one of the good ways to divide code into blocks. This way it's easy to:

  • see what parts of code are enabled
  • disable a part of code
  • see what parts of code are doing what, because they are labeled and clearly parted from other parts of code with functions

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