A Simple Bash Timer

Question

This is a timer I made quite some time ago and just came across it again today. I'm just wondering if there's anything wrong with it or anything I could improve.

timer() {   # Countdown timer
    let "hoursToGo = 0"
    let "minutesToGo = 0"
    let "secondsToGo = 0"

    convertHours(){
        let "hoursToGo = $1"
    }
    convertMinutes(){
        let "hoursToGo = hoursToGo + $1/60"
        let "minutesToGo = $1%60"
    }
    convertSeconds(){
        let "hoursToGo = hoursToGo + $1/3600"
        let "minutesToGo = minutesToGo + ($1%3600)/60"
        let "secondsToGo = ($1%3600)%60"
    }
    help(){
        echo "Usage:"
        echo "timer *number* hour(s)"
        echo "OR"
        echo "timer *number* minute(s)"
        echo "OR"
        echo "timer *number* second(s)"
        echo "OR"
        echo "timer *number* hour(s) *number* minute(s)"
        echo "OR"
        echo "timer *number* hour(s) *number* second(s)"
        echo "OR"
        echo "timer *number* hour(s) *number* minute(s) *number* second(s)"
        echo "OR"
        echo "timer *number* *unit(second(s), hour(s), minute(s))*"
        echo "OR"
        echo "timer *number*    (this will default to seconds)"
        echo "OR"
        echo "timer             (it will request the values)"
        return
    }
    if [ -z $* ] ; then
        read -rp "Hour(s):   " "hoursToGo"
        read -rp "Minute(s): " "minutesToGo"
        read -rp "Second(s): " "secondsToGo"
    elif [[ $1 == "h" ]] || [[ $1 == "help" ]] ; then
        help
        return
    elif [ ! -z $1 ] && [ -z $2 ] ; then
        convertSeconds $1
    elif [[ $2 == *"second" ]] || [[ $2 == *"seconds" ]]; then
        convertSeconds $1
    elif [[ $2 == *"minute" ]] || [[ $2 == *"minutes" ]]; then
        if [ -z $3 ] && [ -z $4 ] ; then
            convertMinutes $1
        elif [[ $4 == *"second" ]] || [[ $4 == *"seconds" ]]; then
            convertMinutes $1
            convertSeconds $3
        else
            help
            return
        fi
    elif [[ $2 == *"hour" ]] || [[ $2 == *"hours" ]]; then
        if [ -z $3 ] && [ -z $4 ] ; then
            convertHours $1
        elif [[ $4 == *"minute" ]] || [[ $4 == *"minutes" ]]; then
            if [ -z $5 ] && [ -z $6 ] ; then
                convertHours $1
                convertMinutes $3
            elif [[ $6 == *"second" ]] || [[ $6 == *"seconds" ]]; then
                convertHours $1
                convertMinutes $3
                convertSeconds $5
            fi
        elif [[ $4 == *"second" ]] || [[ $4 == *"seconds" ]]; then
            convertHours $1
            convertSeconds $3
        fi
    else
        help
        return
    fi

    let "time=(hoursToGo*3600)+(minutesToGo*60)+secondsToGo"
    SECONDS=0

    while (( $SECONDS < $time )) ; do
        let "hoursPassed=SECONDS/3600"
        let "minutesPassed=(SECONDS%3600)/60"
        let "secondsPassed=(SECONDS%3600)%60"
        echo -ne "$hoursPassed hour(s), $minutesPassed minute(s), $secondsPassed second(s)/$hoursToGo hour(s), $minutesToGo minute(s), $secondsToGo second(s)" \\r
        sleep 1
    done

    echo
    echo "Done!"
}

Answer 1

good

• Putting this in a function will definitely make it more reusable.
• It was nice that it keeps the time output on one line.
• Your indentation is consistent which makes reading everything quite easy.
• The usability of your timer is very friendly and the help docs are thorough. I like the flexibility of your inputs.

suggestions

• It would be nice to include some examples of usage. The help section suffices in this case, but in code this long it is a bit buried.
• I generally find the advice of https://www.shellcheck.net/ to be quite helpful. The next few points will cover things uncovered by that tool.
• The let form of telling the shell to do math will work, but it has lost favor to the (( )) form of doing math. One advantage of the (( )) for doing math on a line by itself is that it becomes natural to use the $(( )) for doing calculations in a substitution. As SC2219 notes (( )) also doesn't have the quoting hazards that let does.
• It is a good habit to pass your function arguments with substitutions as double quoted. This will prevent the shell from splitting something into two arguments because of internal spaces when you are expecting only one. See SC2086 for more on this.
• Another advantage of let or (( )) is that variables are available without using the $ in front of them. See also SC2004.
• It'd be nice if it didn't print the hours or minutes when they're zero. All of my tests were shorter than that so they were always zero.
• I really wished that it let me do a countdown, but I understand that could be a feature you have no interest in.
• Shells are often rather imprecise when it comes to sleeping how much you want. Even if the sleep 1 was for precisely one second the rest of your code is going to consume some amount of CPU time and so this will add a bit of time to each iteration. Trying to fix this in a shell script is probably not worth it, but it could be a fun challenge.
• You mix [[ ]] and [ ] for conditionals. The [[ ]] version is more modern and has fewer surprises.
• I thought it was neat to have nested functions, but it turns out "Bash has local variables, but not local functions." (from this discusison) When I sourced the file with your timer code in it and looked at set I found that all of the functions were fully visible. Since you're not actually hiding anything it'd be better to move those functions above timer so they're clearly not "trying to hide".

Really these are all pretty minor issues. They're some good things to think about if you're doing further shell scripts, but the existing code is pretty good already. The readability of the code is quite good and it isn't hard to guess that you've had good experience in some more normal procedural language or a few.

If you choose to revise this script, remember it should be posted as a new question on this site rather than editing this question.