# Measuring typing speed in terminal without a mistake

I tend to dislike my code. It's always a mess. I can't critique my own code though, because I just turn it into another mess. I've been programming slightly better every year, but this year it has hardly changed.

#!/bin/bash

RED='\e[101m'
NC='\033[0m' # No Color

lesson[1]="#!/bin/bash"
lesson[2]="#!/bin/bash"

lessons=${#lesson[@]} current_lesson=1 speed=70 function start() { SECONDS=0 #restart timer reset_user_input string_to_type=${lesson[$1]}; length_of_string=${#string_to_type}
echo $string_to_type count=0 while [ "$count" -ne "$length_of_string" ]; do checkCharacter done time_it_took=$SECONDS
sec=60
wpm=$((sec / time_it_took)) wpm=$((wpm * length_of_string / 5))
echo " "
echo "speed: "$wpm } function checkCharacter() { IFS= read -r -s -n 1 user_input_char add_user_input$user_input_char

if [ "$user_input_char" == " " ]; then add_user_input " " if [ "$user_input_char" == "${string_to_type:$count:1}" ]; then
printf " "
else
printf ${RED}" "${NC}
fi
else
if [ "$user_input_char" == "${string_to_type:$count:1}" ]; then printf$user_input_char
else
printf ${RED}$user_input_char${NC} fi fi let count+=1 } function check_correct() { if [ "$user_input" == "$string_to_type" ]; then return; else echo "too many mistakes, try again!" false; fi } function check_speed() { if [ "$wpm" -gt speed ]; then
return;
else
echo "too slow, try again!"
false
fi
}

function start_lesson() {
start $1 if check_speed && check_correct; then echo "you succeeded!" echo " " # padding for every lesson return; fi echo " " # padding for every lesson false } function reset_user_input() { user_input="" } function add_user_input() { user_input=${user_input}$1 } while [ "$current_lesson" -le "$lessons" ]; do echo "lesson:"$current_lesson
until start_lesson $current_lesson; do : ; done; let current_lesson+=1 done  ## 2 Answers I think this is very nice, clean code. Let's make it better! ### Consistency Consistency helps a lot, making the code easier to understand. • Some functions are named camelCase, others snake_case. Stick to one pattern. • Some variables are named SHOUT_CASE, others snake_case. Stick to one pattern. The latter is recommended, because upper case variable names are commonly used for system variables like PATH and others. • The indentation is mostly 8 spaces, with few exceptions. Use the same everywhere. ### Variable declaration It's good to declare global constants at the top of the file where they are easy to see. The same is not true for state variables that are implementation details, such as current_lesson. That variable would have been better to initialize right before you need it. ### Iterating over arrays Instead of a counting loop over the lesson array, there is a more idiomatic approach: for value in "${lesson[@]}"; do ...; done


I would also rename the array to plural lessons.

### The return value of functions

There are some subtle points I don't like in this code:

function check_correct() {
if [ "$user_input" == "$string_to_type" ]; then
return;
else
echo "too many mistakes, try again!"
false;
fi
}


A return command without parameter will use the exit code of the last statement. In this case it's success. I would find it clearer if you used an explicit return 0 there.

The else branch ends with a false statement. If this is the last statement in the function, then the exit code of the function will be 1, as intended. But this is not explicit enough. The function doesn't return immediately on the false statement. An explicit return 1 would be much better.

Like this:

function check_correct() {
if [ "$user_input" == "$string_to_type" ]; then
return 0
else
echo "too many mistakes, try again!"
return 1
fi
}


With my other suggestions further down, I recommend to write this function like this:

check_correct() {
if ! [ "$user_input" = "$string_to_type" ]; then
echo "too many mistakes, try again!" >&2
return 1
fi
}


### Undocumented features

The == operator is not documented in man [. For that reason I recommend using = instead of == in [ ... ].

### Style

The recommended function declaration style is without the function keyword:

start() {


There are a bunch of redundant semicolons at end of lines that can be safely removed.

wpm=$((sec / time_it_took))  You can write slightly simpler like this: ((wpm = sec / time_it_took))  This works too: ((wpm *= length_of_string / 5))  I don't remember the explanation, but instead of let count+=1, ((count+=1)) is recommended. An even better way to write is ((count++)). ### Sharpening the saw I've been programming slightly better every year, but this year it has hardly changed. Just keep doing it! I type so fast your script had to stop with a division by zero error :D line 64: [: speed: integer expression expected This code segment count=0 while [ "$count" -ne "$length_of_string" ]; do checkCharacter done  is not transparent with how count is incremented. I would place (( count++ )) inside the loop instead of at the end of the checkCharacter function. I see you're trying to attach meaningful names to these numbers, but you left out the magic number 5. I know it's the average number of English letters per word, but the number sticks out like a sore thumb among the other variable names. time_it_took=$SECONDS
wpm=$((sec / time_it_took)) wpm=$((wpm * length_of_string / 5))

I also dislike how the same name wpm is assigned different values in consecutive order. You could have just written wpm=\$(( sec / time_it_took * length_of_string / 5 )) or given the first assignment a different name.