Checking if a number is Armstrong number or not

Question

here is another basic interview question that I had:

Armstrong Numbers

public class ArmstrongCheck {

  public static void main(String[] args) {

    Scanner in = new Scanner(System.in);

    if (!in.hasNextInt()) {
      System.out.println("error");
    }

    int number = in.nextInt();

    armstrong(number);
    System.out.println(armstrong(number));
  }

  private static String armstrong(int number) {
    double sum = 0.0;
    int oldNumber = number;

    while (number > 0) {

      int temp = number % 10;
      number = number / 10;
      sum = sum + Math.pow(temp, 3);
    }

    if (sum == oldNumber) {
      return "armstrong number";
    } else {
      return "not armstrong  number";
    }
  }
}

code is working but I want to simplfy/improve it, any suggestions?

Answer 1

In addition to the points made by justjofe, I also have a few suggestions.

You might consider making the method armstrong(int) return a boolean instead of a String, and then using the returned boolean to construct the output of your program. That way, you separate the logic of determining whether a number is an Armstrong number from the program's interface. In this case, a better name for the method might be "isArmstrong" instead of "armstrong":

public static void main(String[] args) {
    Scanner in = new Scanner(System.in);

    // ...

    if (isArmstrong(in.nextInt())) {
        System.out.println("armstrong number");
    } else {
        System.out.println("not armstrong number");
    }
}

private static boolean isArmstrong(int number) {
    // ...
    return sum == oldNumber;
}

And now about the code that checks a number for being an Armstrong number itself. I think assigning a new value to the method parameter number is confusing, because it forces you to introduce a local variable that serves no other purpose than to store the original value of the method parameter. So why not do it the other way around, using the variable declared inside the method for the loop and leaving the method parameter number assigned to the original value?

private static boolean isArmstrong(int number) {
    double sum = 0.0;
    int n = number;

    while (n > 0) {
        int temp = n % 10;
        n = n / 10;
        sum = sum + Math.pow(temp, 3);
    }

    return sum == number;
}

After this change, the while loop combined with the declaration of n (or whatever you want to call it) actually lends itself to being expressed as a for loop instead:

private static boolean isArmstrong(int number) {
    double sum = 0.0;

    for (int n = number; n > 0; n = n / 10) {
        int temp = n % 10;
        sum = sum + Math.pow(temp, 3);
    }

    return sum == number;
}

I would now even go so far as to skip the introduction of the temp variable and just invoke Math.pow(n % 10, 3), since you only need to evaluate n % 10 once per loop iteration, but that is purely a matter of taste.

Finally, some comments on points mentioned by justjofe:

• First, about closing the scanner: Be aware that calling close() on the Scanner will also call System.in.close() which might not be desirable. There are several threads concerning this at stackoverflow, and one answer says this:

  Generally speaking, the code that creates a resource is also responsible for closing it. System.in was not instantiated by by your code, but by the VM. So in this case it's safe to not close the Scanner […]

  However, this doesn't address the issue of garbage collection with regards to the Scanner object raised by justjofe, and I don't know enough to give specific advice here, so I'll just make you aware of this issue.

• Second, a small heads-up on compound assignment operators such as += and /=: While it might appear that

  • sum = sum + Math.pow(temp, 3); and
  • sum += Math.pow(temp, 3);

  are equivalent, this is actually not the case. If you make sum an int instead of a double, the second statement will compile, however, the first one will not. The reason for this is that the second statement is actually equivalent to

  • sum = (int) (sum + Math.pow(temp, 3));

  (as stated here in the Java Language Specification). Since Math.pow(double, double) returns a double, the result needs to be cast to an int before being assigned to an int variable. The compound assignment operator does this for you, but when using an ordinary =, you have to do the casting yourself.

Answer 2

I'll concentrate mainly on shortening lines and removing unnecessary code lines as it heavily increases the readability of the source code.

#main(String[] args)

The first thing I noticed is that you call the method #armstrong(int number) two times when you only need to call it one time. This is important because it affects your performance as Java will have to process the method but you don't do anything with the result.

System.out.println(armstrong(number));

Instead of:

armstrong(number);
System.out.println(armstrong(number));

You don't even have to save the number to a variable but you can do that for the extra bit of readability. I would simply directly call Scanner#nextInt() as it shortens the main method.

System.out.println(armstrong(in.nextInt()));

Closing the Scanner

You should also close the Scanner after you used it for the last time for several reasons:

• Garbage collection can only manage memory, not other system resources. If your Java program has plenty of free memory, garbage collection will not be triggered automatically and your Scanner will still remain open.
• A stream, which a Scanner uses, kept open, can sometimes stay open until the kernel decides to close it.

in.close();

#armstrong(int number)

In the while loop you can shorten some mathematical operations by using /= and +=. This adds a bit more readability to the program as it shortens those lines.

number /= 10;
sum += Math.pow(temp, 3);

Data types

You can change the data type of the variable sum to an int as you don't make anything with the variable that would require numbers that are not natural numbers:

int sum = 0;

Return statements

Here, you have two possibilities of shortening this, one is using inline checks and the otherone is a rather standard method of avoiding else blocks.

if (sum == oldNumber) {
    return "armstrong number";
}
return "not armstrong number";

This is basically the same but you don't need to include an else-statement as everything that applies to the if-check is already returned and the rest is automatically the else case. The other method is the following:

return (sum != oldNumber ? "not " : "") + "armstrong number";

Personally, I prefer the second method because I got used to using these inline-checks.

Putting it all together

import java.util.Scanner;

public class ArmstrongCheck {

    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        if (!in.hasNextInt()) {
            System.out.println("error");
        }
        System.out.println(armstrong(in.nextInt()));
    }

    private static String armstrong(int number) {
        int sum = 0, oldNumber = number;
        while (number > 0) {
            int temp = number % 10;
            number /= 10;
            sum += Math.pow(temp, 3);
        }
        return (sum != oldNumber ? "not " : "") + "armstrong number";
    }
}