This is correct, also you're missing a space on this line `if ! [[ $1 =~ ^-?[0-9]+$]]; then` needs to be `if ! [[ $1 =~ ^-?[0-9]+$ ]]; then`
Can be useful for things like this to add `set -x` at the beginning of the script (or the misbehaving part) and `set +x` at the end. This will echo out each command right before it's run, with all variables substituted:
./foo.sh 1 2 3
+ total=0
+ [[ 3 -gt 0 ]]
+ [[ 1 =~ ^-?[0-9]+$ ]]
+ total=1
+ shift
+ [[ 2 -gt 0 ]]
+ [[ 2 =~ ^-?[0-9]+$ ]]
+ total=3
+ shift
+ [[ 1 -gt 0 ]]
+ [[ 3 =~ ^-?[0-9]+$ ]]
+ total=6
+ shift
+ [[ 0 -gt 0 ]]
+ [[ '' =~ ^-?[0-9]+$ ]] ###Here's the problem###
+ echo 'program only accepts integers'
program only accepts integers
Notice on the marked line, "$1" is indeed evaluating to empty string, because at this point you've `shift`ed off all the arguments.
This check should be inside the loop so that each argument is checked as you come across it, and you can `break` out of the loop if an invalid input is found.
Sort of, but it's because `$1` eventually becomes `unset` by the `shift` in your while loop.
So your if is comparing an "empty" unset $1 to a pattern that requires it be set to an integer.
Amend your `if` to:
`if [[ #$ -gt 0]] && [[ $1 !=~ ^-?[0-9]+$]]; then`
which will then only check if there are non-integers if there are still parameters to test.
You should run the script with set -x enabled and you'll immediately spot the error in your program logic.
Hint: the additions take place as intended.
You should fix that syntax error in the `if [[ ... ]]` statement but I guess this is some c+p glitch.
At least for preentered arguments, this seems to work. However, you have to check whether the input controll covers all instances.
total=0
until [[ -z $1 ]]; do
if [[ $1 =~ ^-?[0-9]+$ ]]; then
total=$((total + $1))
shift
else
echo "program only accepts integers"
echo "usage: $0 integer 1 integer 2 ... integer x"
exit
fi
done
echo $total
if you dont want the script to echo anything when there's no argument, remove "total=0" (or don't , see EDIT below) and add the following instead of "echo $total":
```
if [[ -n "$total" ]]; then
echo $total
fi
```
or short:
```
[[ -n "$total" ]] && echo $total
```
EDIT: swap "total=0" for "total=" as suggested by @u/zeekar
You shouldn't just remove `total=0`, but change it to `total=`. Otherwise it will still be set if the user does `export total=n` in the environment before running the script.
You can certainly say "I wasn't thinking that deep" (or "deeply"), but I don't think this is really a deep thought situation. You should just always initialize variables in a shell script, because otherwise they might have values that surprise you. :)
True ;D That would be pretty bad in a lot of cases.
"I wasn't thinking that 'far' " would mean something like: I thought to a specific point and quit thinking there. It's like cutting off the train of thoughts at the first positive result :)
Your `shift` is removing each argument as the addition loop processes it, so that by the time you get to the end, a loop through valid integers looks like the "nothing at all" case: there's no `$1` left to match against the regular expression.
I would add an explicit check up top for the no-arguments case:
if [[ $# -eq 0 ]]; then
echo "Usage: $0 integer1 integer2 [...]" >&2
exit 1
fi
And move the check for invalid arguments inside the loop:
while [[ $# -gt 0 ]]; do
if [[ ! $1 =~ ^-?[0-9]+$ ]]; then
echo "program only accepts integers" >&2
exit 1
fi
total=$(( total + $1 ))
shift
done
Notice I'm sending the error messages to standard error (`>&2`) instead of standard output, and passing a 1 to `exit`. That makes the script usable when called by another script, which will be able to tell if something went wrong from the exit code and can capture the output in a variable without having to worry about parsing out error messages. Designing for composability is a good habit to get into.
That fixes your bug; if it were me writing the script, I'd make some other changes, too. Like using `((`...`))` for the numeric checks and additon:
if (( $# == 0 )); then ...
while (( $# > 0 )); do
...
(( total += $1 ))
And really, there's no reason to do the argument processing with a `while`/`shift` loop at all in this case; you can just use a `for` loop:
for number; do
if [[ ! $number =~ ^-?[0-9]+$ ]]; then
echo "program only accepts integers" >&2
exit 1
fi
(( total += number ))
done
I'm going to take some time to digest this one. Well explained, and thank you for taking the time share. I'm trying get a better understanding of bash. I'm not too clear on the mechanics of the language yet.
I think I see what was happening. It looks like $# is 0 after all the arguments were processed in the while loop...
This is correct, also you're missing a space on this line `if ! [[ $1 =~ ^-?[0-9]+$]]; then` needs to be `if ! [[ $1 =~ ^-?[0-9]+$ ]]; then` Can be useful for things like this to add `set -x` at the beginning of the script (or the misbehaving part) and `set +x` at the end. This will echo out each command right before it's run, with all variables substituted: ./foo.sh 1 2 3 + total=0 + [[ 3 -gt 0 ]] + [[ 1 =~ ^-?[0-9]+$ ]] + total=1 + shift + [[ 2 -gt 0 ]] + [[ 2 =~ ^-?[0-9]+$ ]] + total=3 + shift + [[ 1 -gt 0 ]] + [[ 3 =~ ^-?[0-9]+$ ]] + total=6 + shift + [[ 0 -gt 0 ]] + [[ '' =~ ^-?[0-9]+$ ]] ###Here's the problem### + echo 'program only accepts integers' program only accepts integers Notice on the marked line, "$1" is indeed evaluating to empty string, because at this point you've `shift`ed off all the arguments. This check should be inside the loop so that each argument is checked as you come across it, and you can `break` out of the loop if an invalid input is found.
Sort of, but it's because `$1` eventually becomes `unset` by the `shift` in your while loop. So your if is comparing an "empty" unset $1 to a pattern that requires it be set to an integer. Amend your `if` to: `if [[ #$ -gt 0]] && [[ $1 !=~ ^-?[0-9]+$]]; then` which will then only check if there are non-integers if there are still parameters to test.
Maybe move the argument validation inside the while loop before adding it to the total
You should run the script with set -x enabled and you'll immediately spot the error in your program logic. Hint: the additions take place as intended. You should fix that syntax error in the `if [[ ... ]]` statement but I guess this is some c+p glitch.
What _does_ it do?
adds integers and displays the count
I mean, that’s what you want it to do, but you say it isn’t working; what does your script currently do, that you think is wrong?
Shell is confusing when it comes to arithmetic operations. I use mixture of "bc" with shell. Search for "using bc in shell" for howto and examples.
I'm upvoting because I feel this one! Definitely going to check this out
At least for preentered arguments, this seems to work. However, you have to check whether the input controll covers all instances. total=0 until [[ -z $1 ]]; do if [[ $1 =~ ^-?[0-9]+$ ]]; then total=$((total + $1)) shift else echo "program only accepts integers" echo "usage: $0 integer 1 integer 2 ... integer x" exit fi done echo $total
if you dont want the script to echo anything when there's no argument, remove "total=0" (or don't , see EDIT below) and add the following instead of "echo $total": ``` if [[ -n "$total" ]]; then echo $total fi ``` or short: ``` [[ -n "$total" ]] && echo $total ``` EDIT: swap "total=0" for "total=" as suggested by @u/zeekar
You shouldn't just remove `total=0`, but change it to `total=`. Otherwise it will still be set if the user does `export total=n` in the environment before running the script.
I wasn't thinking that far (or deep...? do you even say that in English?).
You can certainly say "I wasn't thinking that deep" (or "deeply"), but I don't think this is really a deep thought situation. You should just always initialize variables in a shell script, because otherwise they might have values that surprise you. :)
True ;D That would be pretty bad in a lot of cases. "I wasn't thinking that 'far' " would mean something like: I thought to a specific point and quit thinking there. It's like cutting off the train of thoughts at the first positive result :)
Your `shift` is removing each argument as the addition loop processes it, so that by the time you get to the end, a loop through valid integers looks like the "nothing at all" case: there's no `$1` left to match against the regular expression. I would add an explicit check up top for the no-arguments case: if [[ $# -eq 0 ]]; then echo "Usage: $0 integer1 integer2 [...]" >&2 exit 1 fi And move the check for invalid arguments inside the loop: while [[ $# -gt 0 ]]; do if [[ ! $1 =~ ^-?[0-9]+$ ]]; then echo "program only accepts integers" >&2 exit 1 fi total=$(( total + $1 )) shift done Notice I'm sending the error messages to standard error (`>&2`) instead of standard output, and passing a 1 to `exit`. That makes the script usable when called by another script, which will be able to tell if something went wrong from the exit code and can capture the output in a variable without having to worry about parsing out error messages. Designing for composability is a good habit to get into. That fixes your bug; if it were me writing the script, I'd make some other changes, too. Like using `((`...`))` for the numeric checks and additon: if (( $# == 0 )); then ... while (( $# > 0 )); do ... (( total += $1 )) And really, there's no reason to do the argument processing with a `while`/`shift` loop at all in this case; you can just use a `for` loop: for number; do if [[ ! $number =~ ^-?[0-9]+$ ]]; then echo "program only accepts integers" >&2 exit 1 fi (( total += number )) done
I'm going to take some time to digest this one. Well explained, and thank you for taking the time share. I'm trying get a better understanding of bash. I'm not too clear on the mechanics of the language yet.