patternbashModerate
Shell Script (Bash) to sum multiples of different arguments below a number
Viewed 0 times
scriptargumentsnumbershelldifferentmultiplesbelowsumbash
Problem
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.
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.*
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.*
Solution
Arithmetic operations
If you look closely, something is a bit odd here:
Ideas? How about:
The same is true for
The general rule is, in arithmetic operations within
Don't use backticks
Instead of this:
This is the recommended writing style for sub shells:
Don't use
It's better to avoid
The for loops can be simplified similarly, instead of:
You can write:
Apply the same thing everywhere.
If you look closely, something is a bit odd here:
for (( i = 0; i <= $limit; i+=$1 )); doIdeas? 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 )); doThe 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
exprIt'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`
doneYou can write:
for (( i = 0; i <= limit; i+=$1 )); do
((sumArg1 += i))
doneApply the same thing everywhere.
Code Snippets
for (( i = 0; i <= $limit; i+=$1 )); dofor (( i = 0; i <= $limit; i+=$commonMultiple )); docommonMultiple=`expr $1 \* $2`commonMultiple=$(expr $1 \* $2)((commonMultiple = $1 * $2))Context
StackExchange Code Review Q#112806, answer score: 10
Revisions (0)
No revisions yet.