Shell Script (Bash) to sum multiples of different arguments below a number

Question

I'm interested in learning how to create shell scripts with bash so here's one of my first exercises, taken from here. It is an extension of the problems Elementary - 4 and 5.

The goal of the program is to sum all multiples or ARG1 or ARG2 below the user's input number.

#!/bin/bash
echo "This program will find all the multiples of ARG1 or ARG2 below or equal to your number and sum them up. Please input your limit:"
read limit
commonMultiple=`expr 1ドル \* 2ドル`
sumArg1=0
sumArg2=0
sumCommonMultiple=0
for (( i = 0; i <= $limit; i+=1ドル )); do
 sumArg1=`expr $sumArg1 + $i`
done
for (( i = 0; i <= $limit; i+=2ドル )); do
 sumArg2=`expr $sumArg2 + $i`
done
for (( i = 0; i <= $limit; i+=$commonMultiple )); do
 sumCommonMultiple=`expr $sumCommonMultiple + $i`
done
echo "The result is `expr $sumArg1 + $sumArg2 - $sumCommonMultiple`."

This works perfectly but I'd like the input of more experienced programmers. I don't think my logic is efficient. There must be something obvious I'm missing that could have made my code a lot simpler.

Disclaimer: apparently some of the stuff I used is outdated. I'll take it into account for my future code.*

• Wildcard: You may find unix.stackexchange.com/a/169765/135943 interesting. The question isn't relevant, but the answer is—it's a a fascinating discussion of the actual purpose of shell scripting. i.e. "What is bash actually for?" (Written by the person who discovered the shell-shock bug, by the way.) bash really isn't designed for computational/algorithmic problems at all. It is a tool for specific types of jobs (see link) and in that domain it excels.
• fnune: That's interesting!

Answer 1

Arithmetic operations

If you look closely, something is a bit odd here:

for (( i = 0; i <= $limit; i+=1ドル )); do

Ideas? How about: i is written without a dollar, but limit is written with a dollar... In fact you can drop the dollar from limit too, and the expression will be slightly simpler. From 1ドル you cannot drop it, as the meaning of 1 would be taken literally, because it's numeric, unlike the others.

The same is true for commonMultiple here, you can drop the dollar:

for (( i = 0; i <= $limit; i+=$commonMultiple )); do

The general rule is, in arithmetic operations within ((...)) you don't need to use the dollars in variable names.

Don't use backticks

Instead of this:

commonMultiple=`expr 1ドル \* 2ドル`

This is the recommended writing style for sub shells:

commonMultiple=$(expr 1ドル \* 2ドル)

Don't use expr

It's better to avoid expr, and you can do the same thing easier using Bash arithmetics, for example:

((commonMultiple = 1ドル * 2ドル))

The for loops can be simplified similarly, instead of:

for (( i = 0; i <= $limit; i+=1ドル )); do
 sumArg1=`expr $sumArg1 + $i`
done

You can write:

for (( i = 0; i <= limit; i+=1ドル )); do
 ((sumArg1 += i))
done

Apply the same thing everywhere.

• fnune: Trying that out and posting back results thanks a lot!

Answer 2

Consider seq

Using seq makes your code significantly shorter in those for loops since you can just rely on seq to do your limit checking and the increments. beware that seq is non-standard though.

for (( i = 0; i <= $limit; i+=1ドル )); do

could simply be written as:

for i in $(seq 0 $i $limit) do

_{thanks to janos for pointing out the grievous flaws in the previous version of my answer}

To Space or not to space?

Compare your spacing in that for loop header:

for (( i = 0; i <= $limit; i+=1ドル )); do

stick to either always adding spaces around operators or not doing it at all. I recommend the former. This makes for a significant readability improvement, especially when not having syntax highlighting or similar.

I'm sure there's more, but that's about the whole extent of my bash knowledge, so I leave the rest in the hands of more competent reviewers :)

• fnune: seq does look a hell of a lot better! Thanks a lot for your answer. Everything I read about my own code is an eye opener at the moment.

Answer 3

In addition to the loop improvements proposed in @Vogel612's answer you may check in the beginning of the script if the script arguments are numbers actually.

expr will give unexpected results or errors otherwise.

You can do so as proposed in this Q&A from Stack Overflow:

re='^[0-9]+$'
if ! [[ 1ドル =~ $re ]] || ! [[ 2ドル =~ $re ]]; then
 echo "error: Please provide 2 numeric parameters" >&2; exit 1
fi

• fnune: It's gonna take a while for me to understand that snippet, but the idea is more than appropriate!
• πάντα ῥεῖ: @FaustoNA It's well explained in the linked Q&A, that's a check based on regular expressions. Glad to help.