0

I wrote the following code to print armstrong numbers between two integers. But I am not able to find the mistake as the code looks fine to me. Please help.

void main()
{
    int a,b;
    printf("Enter the starting limit");
    scanf("%d",&a);
    printf("Enter the ending limit");
    scanf("%d",&b);
    int i;
    int sum=0;
    for(i=a+1;i<b;i++)
    {
        char word[50];
        sprintf(word,"%d",i);
        int temp=strlen(word);
        int j;
        for(j=0;j<temp;j++)
        {
            int c=i%10;
            sum+=pow(c,temp);
            i=i/10;

        }
        if (sum==i)
        {
            printf("%d",i);
        }
    }
}
J...S
  • 5,079
  • 1
  • 20
  • 35
  • `pow` is using floating point and might for example produce `24.9999999` instead of `25`. – Bo Persson Oct 07 '17 at 08:40
  • Is there any alternative you might suggest? –  Oct 07 '17 at 08:56
  • For integers it is just multiplication, for example `pow(5,3)` is the same as `5*5*5`. – Bo Persson Oct 07 '17 at 09:00
  • what was the mistake in my code? –  Oct 07 '17 at 09:02
  • This `if (sum==i)` is in conflict with what Bo mentions in the first comment, which is caused by using `pow()` for integers logic. See https://stackoverflow.com/questions/588004/is-floating-point-math-broken?s=1|1451.3580 which might be considered a duplicate. – Yunnosch Oct 07 '17 at 09:05
  • what should i do to remove this problem? –  Oct 07 '17 at 09:08
  • @James - Phrasing it differently, instead of `pow(5,3)` you could use `5*5*5`. Unlike floating point calculations, integer math always produces whole numbers without rounding errors. – Bo Persson Oct 07 '17 at 09:09

2 Answers2

0

As someone has answered - I will answer as well. Use only the integer arithmetic. No strings or doubles needed

#include <stdio.h>

typedef unsigned long long ull;    // for convenience only


ull sum_cubes(ull num)
{
    ull result = 0;
    while(num)
    {
        unsigned digit = num % 10;
        result += (ull)digit * digit * digit;
        num /= 10;
    }
    return result;
}

#define MIN  0           //or 100 depending if 001 is the 3 digints number or not.
#define MAX  1000


int main(void)
{
    for(ull num = MIN; num < MAX; num++)
    {
        if(num == sum_cubes(num))
        {
            printf("Hurray!! Found - %llu\n", num);
        }
    }
}
0___________
  • 60,014
  • 4
  • 34
  • 74
  • Just saying, but if the number and the sum of the cubes of its digits are equal, the number is Armstrong only if the number is a 3 digit number. I suppose the OP used `strlen()` to find the number of digits. – J...S Oct 07 '17 at 15:01
  • @J...S Yea very difficult to change indeed. Change MAX to 1000 and you may change typedef from the `unsigned long long` to `unsigned` – 0___________ Oct 07 '17 at 15:06
-1

You use i in the control expression of the outer for loop but in the inner loop you are modifying i with the i=i/10;.
Even a single execution of that line will ensure that the value of i is lesser than its initial value.
Then this modified i is incremented with the i++ in the outer for loop. If a is less than b, i<b will always be true, resulting in an infinite loop.

But since you have the number in the form of string in word, you could use that.

 for(j=0;j<temp;j++)
 {
     int c=word[j]-48;

The -48 is used to convert the encoded character value (eg: 2 in ASCII is 50) to the actual number. If the encoding you use is not ASCII, you might have to do something different.

You could also make a copy of i before entering the inner loop and use that variable instead of i in the inner loop like

    int t=i, j;
    for(sum=j=0;j<temp;j++)
    {
        int c=t%10;
        sum+=pow(c,temp);
        t=t/10;
    }

You are not resetting the value of sum to 0 before each iteration of the inner loop. You could do that in the first part of the loop (ie, before the first semi-colon) as in the above loop.

Also as pointed out by Bo, pow() returns a float and due to reasons expounded here, inaccuracies might creep in.

So make the type of sum to be float instead of int.

Use of void as the return type of main() is not considered good practice. Use int main() instead. See here.

The part where you take the digits using % operator is also a mistake.

 int c=i%10;

On all iterations of the inner loop, value of c would be the last digit of i and you won't get any other digit in c.

J...S
  • 5,079
  • 1
  • 20
  • 35