My today's goal was to further practice POSIX shell scripting.
So, I decided to write the following POSIX shell script for generating pseudo-random numbers.
Any and all reviews would be appreciated.
#!/bin/sh
bold=$(tput bold)
red=$(tput setaf 1)
nocolor=$(tput sgr0)
bold_red="${bold}${red}"
print_error_and_exit()
{
# check if exactly two arguments have been passed
if [ "$#" -ne 2 ]
then
print_error_and_exit "print_error_and_exit(): There have not been passed two arguments!" 2
fi
# check if the second argument is a number
if ! is_this_a_number "2ドル"
then
print_error_and_exit "print_error_and_exit(): The argument #2 is not a number!" 5
fi
echo "${bold_red}1ドル Exit code = 2ドル.${nocolor}" 1>&2
exit "2ドル"
}
is_this_a_number()
{
# check if exactly one argument has been passed
if [ "$#" -ne 1 ]
then
print_error_and_exit "is_this_a_number(): There has not been passed exactly one argument!" 2
fi
# check if the argument is an integer number
if [ "1ドル" -eq "1ドル" ] 2> /dev/null
then
return 0
else
return 1
fi
}
random_number()
{
# check if exactly two arguments have been passed
if [ "$#" -ne 2 ]
then
print_error_and_exit "random_number(): There have not been passed exactly two arguments!" 3
fi
# check if the first argument is a number
if ! is_this_a_number "1ドル"
then
print_error_and_exit "random_number(): The argument #1 is not a number!" 4
fi
# check if the second argument is a number
if ! is_this_a_number "2ドル"
then
print_error_and_exit "random_number(): The argument #2 is not a number!" 5
fi
# min is inclusive
range_min="1ドル"
# max is inclusive too
range_max="2ドル"
# RANDOM is not POSIX-available,
# so we can use shuf for instance
shuf -i "$range_min"-"$range_max" -n 1
}
# temporary testing loop
i=1
while [ "$i" -lt 10 ]
do
random_number 1 9
i=$((i + 1))
done
1 Answer 1
Firstly, it's good that you use tput
to portably obtain terminal codes if they exist. Too many shell script authors fail to do that! We might choose to only evaluate those if we know we'll use them (i.e. inside print_error_and_exit
), and there seems little point in setting $bold_red
to use only once.
I wonder if the error checking within print_error_and_exit
might be overkill. If you're using it only within this script, you might omit the argument checking (if you made a mistake in the recursive call, you might end up in a worse position than not checking at all!).
It might be worth accepting the arguments to print_error_and_exit
in the opposite order (exit code first), to avoid the important part being overlooked at the end of the line.
Some of the if
tests are very verbose compared to simply chaining commands with &&
or ||
. That might be a style decision, but I would certainly reconsider
if command
then
return 0
else
return 1
fi
When it's the last line of the function, that simplifies to just
command
(unless you really care which non-zero value indicates failure).
Although we know that $#
expands to an integer, and is therefore safe to use without quoting, there's nothing wrong with quoting it as you have (and here on Stack Exchange it helps the prettifier distinguish it from a comment).
I was concerned that shuf
might be very inefficient if a large range is selected, but reading the source for the GNU coreutils implementation shows that it manages to avoid building the entire list of numbers to choose from. Whether other implementations are similarly optimised is left as an exercise.
Modified program
#!/bin/sh
print_error_and_exit()
{
# check if exactly two arguments have been passed
test "$#" = 2 || print_error_and_exit 2 "print_error_and_exit(): There have not been passed two arguments!"
# check that the first argument is a number
is_this_a_number "1ドル" || print_error_and_exit 5 "print_error_and_exit(): The argument #1 is not a number!"
bold=$(tput bold)
red=$(tput setaf 1)
nocolor=$(tput sgr0)
echo "$bold$red2ドル Exit code = 1ドル.$nocolor" >&2
exit 1ドル
}
is_this_a_number()
{
# check if exactly one argument has been passed
test "$#" = 1 || print_error_and_exit 2 "is_this_a_number(): There has not been passed exactly one argument!"
# check if the argument is an integer
test "1ドル" -eq "1ドル" 2>/dev/null
}
# print a number in range 1ドル to 2,ドル inclusive
random_number()
{
# check if exactly two arguments have been passed
test "$#" = 2 || print_error_and_exit 3 "random_number(): There have not been passed exactly two arguments!"
# check if the arguments are both numbers
is_this_a_number "1ドル" || print_error_and_exit 4 "random_number(): The argument #1 is not a number!"
is_this_a_number "2ドル" || print_error_and_exit 5 "random_number(): The argument #2 is not a number!"
shuf -i "1ドル-2ドル" -n 1
}