CODEWITHSUNDEEP
bashmacosshell
Aditya Singh
Aditya Singh

Reputation: 2443

A loop unexpectedly goes running infinitely in the Bash script

I have created a bash script on my Mac named Armstrong.sh.

This is a function which checks for a number to be an armstrong number. 

# This function works properly
armstrong() {

    num=$1    # Making a copy of the received number.
    sum=0     # This will store the sum of cubes of each digit from $num



    while [ $num -gt 0];      # This loop runs while $num is greater than 0
    do
        temp=`expr $num % 10`                      # Extract the last digit of the number
        sum=`expr $sum + $temp \* $temp \* $temp`  # Cube the last digit and add it to $sum
        num=`expr $num / 10`                       # Remove the last digit of the number
    done



    if [ $sum -eq $1 ];   # If $sum == $1, i.e., If the number is armstrong
    then   
        echo "$1 is an armstrong number"        # print the number
    else
        echo "$1 is not an armstrong number"
    fi
}

When I write the following code, 

armstrong 1     # this is an armstrong number
armstrong 153   # This is an armstrong number
armstrong 24    # This is not an armstrong number

Then it's output is as follows, 

1 is an armstrong number
153 is an armstrong number
24 is not an armstrong number

This was good till now.

But the PROBLEM lies here.
When I try to print all armstrong numbers within a range using a loop like this: 

# Accept start and end point of the range
echo -n "Enter start = "
read start
echo -n "Enter end = "
read end

# Loop from start to end point and call the armstrong() function
for ((num = $start; num <= $end; num++))
do
    armstrong $num    # Calling the function.
done

So my questions are:
1> How do I get the loop working as desired?
2> Is there any way I can write the code without using $temp in armstrong() function?
Just like sum += Math.pow(num%10, 3); in Java?
3> Please give me a cleaner way to write the armstrong function.

Upvotes: 0

Views: 170

Answers (2)

John B
John B

Reputation: 3646

The function is only checking for armstrong numbers of 3 digit length.

Armstrong numbers, also known as Narcissistic Numbers, are numbers that are the sum of each digit raised to the power of the number's length.

As @CharlesDuffy pointed out, to avoid unexpected behavior, variables inside the function should be defined as local variables. Unless, of course, they need to be accessed globally.

Also, when using bash's limited integer expressions, larger numbers will break any test or calculation of the number.

To fix that, you can use pattern matching for tests, and bc for calculations:

armstrong() {

    # Initialize all local variables
    local num=$1 sum=0 num_digits=${#1}

    # Make sure number is greater than 0
    while [[ $num == [1-9]* ]]
    do
        # Raise the last digit to the length of $num and add it to $sum
        sum=$(echo "$sum + (($num % 10)^$num_digits)" | bc)

        # Remove the last digit of the number
        num=$(echo "scale=0; $num / 10" | bc)
    done

    if [[ $sum == $1 ]]
    then   
        echo "$1 is an armstrong number"
    else
        echo "$1 is not an armstrong number"
    fi

}

Alternatively, you could iterate over each single digit in the num variable using parameter expansion:

 for ((i=0;i<=$((num_digits-1));i++)); do
     sum=$(echo "$sum + (${num:$i:1}^$num_digits)" | bc)
 done

Upvotes: 1

Charles Duffy
Charles Duffy

Reputation: 295373

Your function uses the variable num without declaring it local, so it's changing the same shell-global variable referred to by the loop, thus resetting the loop's state and preventing it from completing.

Inside the function, change

num=$1

to

local num=$1

...and, ideally, do the same for all other variables inside of functions unless you explicitly want your function to modify variables in global scope.

Upvotes: 5

Related Questions