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.';
}
\$\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\$ 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\$
    – Steve T
    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\$ 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\$
    – janos
    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\$
    – Mast
    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.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you sir. Just to add, regarding echo vs printf: stackoverflow.com/questions/8467424/… \$\endgroup\$
    – Steve T
    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\$
    – janos
    Aug 27, 2022 at 21:27

Your Answer

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

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