5
\$\begingroup\$

My goal is to have a timer function that operates from the bash linux command line. I am looking for review and feedback. Printing the expected number of characters (dots, dashes, or bars) is nice-to-have, not a must-have.

For instance, I type "timer 10", and the function waits for 10 seconds before saying "timer complete". While it counts down, it outputs a single period "." for each second of the countdown.

So it would look like this:

$> timer 10
.......... timer complete

The code for that is:

function timer {
 time=${1}
 end=$((SECONDS+time))
 while [ $SECONDS -lt $end ]; do
 printf '.';
 sleep 1;
 done
 echo ' timer finished.';
}
export -f timer

You could take it further to handle hours and minutes as well as seconds, and to use units (h, m, s). For hours, each dot would represent 5 min, and each hour would end in a "|". For minutes, each dot would represent 15 sec, and each minute would end in a "-".

So that would look like this:

$> timer 1 h 5 m 10 s
...........|
...-...-...-...-...-
.......... timer complete

I am having some trouble getting the expected number of dots to be printed to screen. I think it has something to do with my calculations of $((operation here)) slowing things down. Here is that code:

# 'counter' utility method:
# ie: counter 5 # wait 5 seconds
function counterSec {
 time=${1}
 end=$((SECONDS+time))
 sleep 1;
 while [ $SECONDS -lt $end ]; do
 printf '.';
 sleep 1;
 done
}
export -f counterSec
# 'counter' utility method:
# ie: counter 5 # wait 5 minutes
function counterMin {
 time=${1}
 end1=$((SECONDS+(time*60)))
 while [ $SECONDS -lt $end1 ]; do
 end2=$((SECONDS+45))
 while [ $SECONDS -lt $end2 ]; do
 sleep 15;
 printf '.';
 done
 sleep 15;
 printf '-';
 done
}
export -f counterMin
# 'counter' utility method:
# ie: counter 5 # wait 5 hours
function counterHour {
 time=${1}
 end1=$((SECONDS+(time*3600)))
 while [ $SECONDS -lt $end1 ]; do
 end2=$((SECONDS+3300)) # (55*60)
 while [ $SECONDS -lt $end2 ]; do
 sleep 300; # (5*60)
 printf '-';
 done
 sleep 300; # (5*60)
 printf '|';
 done
}
export -f counterHour
# 'counter' utility method
# more complex version of timer
function counter {
 time1=${1} # hour or min or sec
 time2=${3} # min or sec
 time3=${5} # sec
 unit1=${2} 
 unit2=${4}
 unit3=${6} 
 if [ $# -eq 2 ]; then
 # hour, min, or sec
 if [ "$unit1" == "h" ]; then
 counterHour $time1
 fi
 if [ "$unit1" == "m" ]; then
 counterMin $time1
 fi
 if [ "$unit1" == "s" ]; then
 counterSec $time1
 fi
 elif [ $# -eq 4 ]; then 
 # hour, min / hour, sec / min, sec
 if [ "$unit1" == "h" ]; then
 counterHour $time1
 printf "\n";
 if [ "$unit2" == "m" ]; then
 counterMin $time2
 elif [ "$unit2" == "s" ]; then
 counterSec $time2
 fi
 elif [ "$unit1" == "m" ]; then
 counterMin $time1
 printf "\n";
 if [ "$unit2" == "h" ]; then
 counterHour $time2
 elif [ "$unit2" == "s" ]; then
 counterSec $time2
 fi
 elif [ "$unit1" == "s" ]; then
 counterSec $time1
 printf "\n";
 if [ "$unit2" == "h" ]; then
 counterHour $time2
 elif [ "$unit2" == "m" ]; then
 counterMin $time2
 fi
 fi
 elif [ $# -eq 6 ]; then
 # hour and min and sec
 if [ "$unit1" == "h" ]; then
 counterHour $time1
 printf "\n";
 if [ "$unit2" == "m" ]; then
 counterMin $time2
 printf "\n";
 if [ "$unit3" == "s" ]; then
 counterSec $time3
 fi
 elif [ "$unit2" == "s" ]; then
 counterSec $time2
 printf "\n";
 if [ "$unit3" == "m" ]; then
 counterMin $time3
 fi
 fi
 elif [ "$unit1" == "m" ]; then
 counterMin $time1
 printf "\n";
 if [ "$unit2" == "h" ]; then
 counterHour $time2
 printf "\n";
 if [ "$unit3" == "s" ]; then
 counterSec $time3
 fi
 elif [ "$unit2" == "s" ]; then
 counterSec $time2
 printf "\n";
 if [ "$unit3" == "h" ]; then
 counterHour $time3
 fi
 fi
 elif [ "$unit1" == "s" ]; then
 counterSec $time1
 printf "\n";
 if [ "$unit2" == "h" ]; then
 counterHour $time2
 printf "\n";
 if [ "$unit3" == "m" ]; then
 counterMin $time3
 fi
 elif [ "$unit2" == "m" ]; then
 counterMin $time2
 printf "\n";
 if [ "$unit3" == "h" ]; then
 counterHour $time3
 fi
 fi
 fi
 fi
 echo ' counter finished.';
}
asked Aug 24, 2022 at 22:01
\$\endgroup\$
5
  • 2
    \$\begingroup\$ According to the rules of the site (see the help center), the code must be working correctly before we can review it. \$\endgroup\$ Commented Aug 25, 2022 at 1:25
  • \$\begingroup\$ Hello, I would like to say that the code is working fine, except that periodically it seems that it's not printing out all of the characters that I expected it to. I believe that this is not a logic issue, but rather something to do with the sleep command, or the tracking of $SECONDS by the kernel. Since this is likely a kernel issue, I don't know if changing the logic will make a difference. But I was hoping by posting this code, someone with more knowledge of the OS might have an insight to improve the code. Otherwise I am fine with the code as-is and will continue to use it. \$\endgroup\$ Commented Aug 25, 2022 at 14:28
  • 1
    \$\begingroup\$ You should be aware that sleep delays execution by at least the number of seconds that you specify. There is no guarantee of exactness. \$\endgroup\$ Commented Aug 25, 2022 at 21:11
  • 3
    \$\begingroup\$ Is your main question to make the script display the expected number of dots, or is your main question to get feedback on the code you have so far? If you are mainly looking for a review and feedback, and you update the description to make that clear, then I think the question can be reopened. You could mention that displaying the expected number of dots is a nice-to-have, not a must-have. \$\endgroup\$ Commented Aug 26, 2022 at 8:32
  • 1
    \$\begingroup\$ Yea, I see the dots more as a progress bar of sorts, an indicator the script hasn't hung up and a general idea of how far along things are. Those things are rarely ever exact. \$\endgroup\$ Commented Aug 26, 2022 at 15:58

1 Answer 1

6
\$\begingroup\$

Validate inputs

The script quietly ignores invalid inputs. It would be good to add some validation logic and fail when used incorrectly.

Since the counter* functions are exported, they should validate their inputs. On the other hand, if you export only the counter function, then you could validate inputs only in that one, and then you could let the other helper functions assume that they can trust their caller.

Example validator for time units:

is_valid_time_unit() {
 [[ $# = 1 ]] && [[ 1ドル == [hms] ]]
}

Example validator for positive integers:

is_positive_int() {
 [[ $# = 1 ]] && [[ 1ドル =~ ^[0-9]+$ ]]
}

Don't repeat yourself

The counter function implements a complex if-else chain for all the possible permutations of h m s parameters. This is not easy to read and error prone. Since the time and unit pairs are processed the same way regardless of the order, you can use a loop, for example:

counter() {
 local unit count
 while true; do
 count=1ドル; shift
 unit=1ドル; shift
 case $unit in
 h) counterHour "$count" ;;
 m) counterMin "$count" ;;
 s) counterSec "$count" ;;
 esac
 if (($# > 0)); then
 echo
 else
 break
 fi
 done
 echo ' counter finished.';
}

Use the modern style to declare functions

Instead of:

function counterSec {

The modern recommended form is:

counterSec() {

Drop unnecessary ; at line ends

For example here, the ; are unnecessary:

printf '.';
sleep 1;

Print blank lines with echo

Instead of printf "\n" it's simpler to just write echo.

Use more spaces in arithmetic contexts

Instead of:

end1=$((SECONDS+(time*60)))

It's easier to read when there are spaces around operators:

end1=$((SECONDS + (time * 60)))

Even easier to put the whole assignment into an arithmetic evaluation ((...)):

((end1 = SECONDS + (time * 60)))

Printing progress markers precisely

As you have observed, the script sometimes doesn't print all the characters that you would expect by simple math. On the other hand I would remark that the script will terminate after approximately the expected time.

In an operating system where mutliple tasks compete for finite CPU, it's not realistic to expect that a process could print interval markers precisely. Delays can happen due to competition with other processes, and also because the marker printing process itself has non-zero CPU cost.

You could make sure it prints the mathematically expected number of markers, with at least the designated number of sleep per interval, but then the script might terminate much later, or theoretically never, on a heavily loaded system.

I think the current UX of the script is fine, and it would be futile to try to achieve better, especially using a blunt tool such as Bash.

answered Aug 26, 2022 at 16:06
\$\endgroup\$
2
  • \$\begingroup\$ Thank you sir. Just to add, regarding echo vs printf: stackoverflow.com/questions/8467424/… \$\endgroup\$ Commented Aug 27, 2022 at 20:32
  • 1
    \$\begingroup\$ That thread is about a more complex, and more general use case. In the specific, narrow use case here, I see no reason to write more than a simple echo without any parameters. \$\endgroup\$ Commented Aug 27, 2022 at 21:27

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.