0

I have written a small program for determining if the number is an Armstrong number.

For most numbers it works well, but there are some numbers (e.g. 8208) which should return true, but they return false.

public static bool IsArmstrong(string numValue)
    {
        int sum = 0;
        int intValue = Int32.Parse(numValue);

        for (int i = intValue; i > 0; i = i / 10)
        {
            sum = sum + (int)Math.Pow(i % 10, 3.0);
        }
        if (sum == intValue)
            return true;
        else
            return false;
    }

I have looked through several other posts about Armstrong numbers. As far as I can see I am using the correct formula.

Am I missing something here?

The reason I am using a string value as input is I evaluate numbers from the text file.

Přemysl Šťastný
  • 1,676
  • 2
  • 18
  • 39
Claudi
  • 125
  • 1
  • 1
  • 9
  • 1
    Obviously your algorithm is incorrect, because C# isn't going to return `true` when something is `false`. At a first glance, you're omitting the first number because you already start by dividing `i` by 10. Debug your code, step through it and inspect your variables. – CodeCaster Jul 18 '16 at 12:28
  • @GiladGreen a typo, should be corrected – Claudi Jul 18 '16 at 12:29
  • 1
    See also [Is there an easy way to turn an int into an array of ints of each digit?](http://stackoverflow.com/questions/829174/is-there-an-easy-way-to-turn-an-int-into-an-array-of-ints-of-each-digit), which seems to be the core of your problem. – CodeCaster Jul 18 '16 at 12:33

2 Answers2

1

Your algorithm always uses 3.0 as the number of digits, where "8028" has 4 digits. Since you pass the input as a string, you could use its length as the power (provided there are no whitespaces and such): (int)Math.Pow(i % 10, numValue.Length)

An alternative, since the input already is a string, you can enumerate its chars to do the summing: (ascii value - '0' )

public static bool IsArmstrong(string numValue)
{
    int pow = numValue.Length;
    return numValue.Sum(c=> Math.Pow(c-'0', pow)) == int.Parse(numValue);
}
Me.Name
  • 12,259
  • 3
  • 31
  • 48
0

Here is a variant, to check armstrong numbers. based of this and it's probably a little bit faster

public static bool IsArmstrong(string numValue)
{
        long n;
        if (!long.TryParse(numValue, out n))
        {
            throw new Exception($"{numValue} not a number");
        }
        long c = 0, a;
        long temp = n;
        while (n > 0)
        {
            a = n % 10;
            n = n / 10;
            c = c + (int)Math.Pow(a,numValue.Length);
        }
        if (temp == c)
            return true;
        else
            return false;
}

A funny thing, I ran your code without any problems, here is the test code I did.

    static void Main(string[] args)
    {
        Stopwatch sw = new Stopwatch();
        sw.Start();
        for (long i = 0; i < 100000; i++)
        {
            if (IsArmstrong(i.ToString()))
                Console.WriteLine(i + " " + IsArmstrong(i.ToString()));
        }

        Console.WriteLine($"DONE in {sw.ElapsedMilliseconds} ms");
        sw.Restart();
        for (long i = 0; i < 100000; i++)
        {
            if (IsArmstrong2(i.ToString()))
                Console.WriteLine(i + " " + IsArmstrong2(i.ToString()));
        }
        Console.WriteLine($"DONE in {sw.ElapsedMilliseconds} ms");

        Console.WriteLine(IsArmstrong2("548834"));
        Console.ReadKey();
    }
    public static bool IsArmstrong(string numValue)
    {
        long sum = 0;
        long longValue = long.Parse(numValue);

        for (long i = longValue; i > 0; i = i / 10)
        {
            sum = sum + (long)Math.Pow(i % 10, numValue.Length);
        }
        if (sum == longValue)
            return true;
        else
            return false;
    }
    public static bool IsArmstrong2(string numValue)
    {
        long n;
        if (!long.TryParse(numValue, out n))
        {
            throw new Exception($"{numValue} not a number");
        }
        long c = 0, a;
        long temp = n;
        while (n > 0)
        {
            a = n % 10;
            n = n / 10;
            c = c + (int)Math.Pow(a,numValue.Length);
        }
        if (temp == c)
            return true;
        else
            return false;
    }
Thomas Andreè Wang
  • 3,379
  • 6
  • 37
  • 53
  • this logic i get, and it seems to be working, still i wonder how others got what i used to work, seeing as i found several numbers that returned false, even though they were armstrong numbers – Claudi Jul 18 '16 at 12:43
  • The funny thing is, i ran your code, and it runs flawlessly. – Thomas Andreè Wang Jul 18 '16 at 12:48
  • @Claudi i added my testing code IsArmstrong2 is the new one anv IsArmstrong is your original one. – Thomas Andreè Wang Jul 18 '16 at 12:53
  • i just debugged it with 8208, with the improvement from you code, and for some reason the c value keeps ending up as 1032. but im now sure that its not the math that is failing, just cant figure out what it is – Claudi Jul 18 '16 at 12:54
  • I tried setting up a loop like in your example but for some reason it only prints numbers: 0 1 153 370 371 407; with 10000 iterations – Claudi Jul 18 '16 at 13:04
  • @Claudi im having trouble saving on stack today, number of digits os statically set to 3, fixed now – Thomas Andreè Wang Jul 18 '16 at 13:08