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

Egg timer in Bash

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

Problem

I just started to learn programming, going through a lot of different tutorials, trying out different programming languages and I stumble in the same sort of questions all over the place.

I give you an simple egg timer in bash I use for cooking.

#!/bin/bash

### varibales
min=$1
sec=$2
message=$3

### run
timer=$(($min * 60 + $sec))
for i in $(seq 0 $timer)
do 
remain=$(( $timer - $i ))
echo -ne "  $(($remain / 60)):$(($remain % 60))  \r"
sleep 1
done

echo -ne "                                  \r"
echo "$message"
afplay ~/alarm.mp3


This egg timer is very simple and therefore usually reliable. I use it for cooking. Yet, when I imagine doing it on a grander scale, there would be so many questions, that are neglectable in such a small application. But I am not sure if they would stay neglectable.

-
Is it a good idea to keep the additional variable timer in there? It helps readability, but wouldn't that hold another variable in my RAM? Shouldn't I better forget about readability and directly calculate it in the echo command. Same goes with the variables set for min, sec and message. Those variables are basically doubled for readability alone. I could directly use them in the script.

-
Using sleep to count down time. As far as I know, things like sleep, wait and pause can be off when the CPU is under heavy usage. Something probably to consider for scripts on my old raspberry, that only has one core. Shouldn't I first calculate the time of the alarm and then use my system time to calculate the time left? Should it do that in every iteration of the loop, that has to comes at least every second or should I only do it every minute or hour?

-
If I check it with system time, should I rather put the checks in interlapping loops than add an if check? Feels like an interlapping loop would take less processing power than use if checks every time it runs.

-
While we're on the question of waiting. How do I find good timings for programs? I me

Solution

Your main questions


Is it a good idea to keep the additional variable "timer" in there? It helps readability, but wouldn't that hold another variable in my RAM? Shouldn't I better forget about readability and directly calculate it in the "echo" command. Same goes with the variables set for "min", "sec" and "message". Those variables are basically doubled for readability alone. I could directly use them in the script.

Readability is extremely important. Write programs for people, not for computers. The computer has no problem reading a program that's written ugly. But it's not the computer that might have to search for bugs or implement the next feature, that's going to be a human. Code is read far more than it's written.

As far as RAM goes, the cost of an additional variable is negligible,
you can safely ignore it.


Using "sleep" to count down time. As far as I know, things like "sleep", "wait" and "pause" can be off when the CPU is under heavy usage. Something probably to consider for scripts on my old raspberry, that only has one core. Shouldn't I first calculate the time of the alarm and then use my system time to calculate the time left? Should it do that in every iteration of the loop, that has to comes at least every second or should I only do it every minute or hour?

Yes, it will be more accurate to calculate the target end time, and in every iteration recalculate the remaining time to display. It's up to you how you pace the loop. Every second seems fine, with a sleep 1 like it is now.


If I check it with system time, should I rather put the checks in interlapping loops than add an if check? Feels like an interlapping loop would take less processing power than use if checks every time it runs.

I don't really get what an "interlapping" loop is, but in any case, you only need one loop, something like this: (see at the bottom a complete implementation)

target_time=...

while :; do
    current_time=$(date +%s)
    ((current_time >= target_time)) && break
    print_remaining_time current_time target_time
done



While we're on the question of waiting. How do I find good timings for programs? I mean, let's assume my program needs to check something regularily. Like, a file on the computer. How do I find a good timing for it? Every .5 second would feel pretty responsive. Yet, am I clogging my CPU with unnecessary checks? Is there some rule of thumb, like check 10 times more often for things on your RAM instead of things on your HD?

That's a bit too broad to answer. It can depend on your hardware and specific circumstances and requirements. And looping is not always the right thing to do. In the example you gave, waiting for a file, it's better to look for a way to listen on filesystem events, rather than a loop with a sleep.

Code review

Bash arithmetics can help you simplify a lot. For example, instead of this:

timer=$(($min * 60 + $sec))


You can write like this:

((timer = min * 60 + sec))


That is, you can drop all the $ and use comfortable spacing around operators.

You did not indent the body of your for loop.
This is not easy to read. It would be better this way:

for i in $(seq 0 $timer)
do 
    remain=$(( $timer - $i ))
    echo -ne "  $(($remain / 60)):$(($remain % 60))  \r"
    sleep 1
done


seq is not standard, and therefore not recommended.
And you can easily achieve the same thing using for:

for ((i = 0; i < timer; ++i)); do


You did not validate your input.
If the script is called without parameters, the behavior will be odd.
It would be more friendly to display a help message to tell the user that something's wrong.

Suggested implementation

Applying some of the above suggestions (also borrowing a bit from the answer of @200_success, a cleaner, safer, more readable implementation:

#!/bin/bash

if test $# -lt 3; then
    echo usage: $0 minutes seconds message
    exit 1
fi

min=$1
sec=$2
message=$3

((target_time = $(date +%s) + min * 60 + sec))
while :; do
    ((current_time = $(date +%s)))
    ((current_time >= target_time)) && break
    ((remain_sec_total = target_time - current_time))
    ((remain_min = remain_sec_total / 60))
    ((remain_sec = remain_sec_total % 60))
    printf '%4d:%02d  \r' $remain_min $remain_sec
    sleep 1
done

printf '%-7s\n' "$message"

Code Snippets

target_time=...

while :; do
    current_time=$(date +%s)
    ((current_time >= target_time)) && break
    print_remaining_time current_time target_time
done
timer=$(($min * 60 + $sec))
((timer = min * 60 + sec))
for i in $(seq 0 $timer)
do 
    remain=$(( $timer - $i ))
    echo -ne "  $(($remain / 60)):$(($remain % 60))  \r"
    sleep 1
done
for ((i = 0; i < timer; ++i)); do

Context

StackExchange Code Review Q#112968, answer score: 7

Revisions (0)

No revisions yet.