HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Printing out Armstrong numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
armstrongnumbersprintingout

Problem

This is my first Java program to print out the Armstrong numbers.


An Armstrong number is a number that is the sum of its own digits each raised to the power of the number of digits.

I want to get better, so be harsh and straightforward.

public class Armstrong {

    public static void main (String arg[]){

        int Max=2000000000;
        for (int i=0; i1){
        for (int count=length; count>=0; count--){

            dg = (int) (d/(Math.pow(10, count)));
            d= (int) (d % (Math.pow(10, count)));
            dg =(int) Math.pow(dg, length);
            r = r + dg;
        }

        }
     else {
         r=sp;
     }
     return r;

        }
}

Solution

Your indenting is weird. In getdig, you deindent, then reindent.

Variables should always be camelCase or, if they're constants, UPPER_SNAKE_CASE -- int Max should be int max or final int MAX, depending on if you want to make it clear that it's a constant. I also think it should be a class variable, not a local variable in a method.

Put a space on either side of your binary (two-argument) operators -- so int n=x becomes int n = x, i<max becomes i < max, etc. Note that things like ++ and ! are not binary, since they only take one argument.

Method names are camelCase as well, where the humps are on word boundaries. isarmstrong should be isArmstrong.

Use meaningful names! sp, getdig, n, sp -- none of these mean anything. It's fine to have long names -- don't use any abbreviations but the most obvious ones. Id sit, getIP would be fine; you don't need to write getInternetProtocolAddress.

if (/* condition */) {
    return true;
}
return false;


is a pretty common way to add lots of characters with no meaning. Just write

return /* condition */;


replacing / condition / with the actual code. In this case, that's n == getDig(n).

You have a bunch of lines with nothing but whitespace; the convention I've seen is to separate methods and logically distinct blocks of code. You have a bunch of whitespace just before the method ends, or just after it begins.

In isArmstrong, you assign n to be equal to x, then never use x again. Why not ditch n and just use the original variable?

In getdig's for, a couple of things.

Firstly, a = a + b can be shortened to a += b and it's understood what that means.

Secondly, since none of the other code relies on the changing of d -- in fact, you moved one line to keep it from interfering -- why not visually separate it by moving it to the bottom and plopping a blank line between?

As for your actual math, though, it looks good. It's rather hard to tell, since your names mean absolutely nothing, but no matter.

With all of these applied (except the better names -- you should do that yourself, since you know what they are) here's how your code looks:

public class Armstrong {
    public static void main (String arg[]){
        int max = 2000000000;
        for (int i = 0; i  1){
            for (int count = length; count >= 0; count--){
                dg = (int) (d / Math.pow(10, count));
                dg = (int) Math.pow(dg, length);
                r += dg;

                d = (int) (d % Math.pow(10, count));
            }

        }
        else {
            r = sp;
        }
        return r;
    }
}

Code Snippets

if (/* condition */) {
    return true;
}
return false;
return /* condition */;
public class Armstrong {
    public static void main (String arg[]){
        int max = 2000000000;
        for (int i = 0; i < max; i++){
            if (isArmstrong(i)){
                System.out.println(i);
            }
        }
    }

    private static boolean isArmstrong(int n) {
        return n == getDig(n);
    }

    private static int getDig(int sp){
        int d = sp;
        int r = 0, dg = 0;
        int length = String.valueOf(sp).length();
        if (length > 1){
            for (int count = length; count >= 0; count--){
                dg = (int) (d / Math.pow(10, count));
                dg = (int) Math.pow(dg, length);
                r += dg;

                d = (int) (d % Math.pow(10, count));
            }

        }
        else {
            r = sp;
        }
        return r;
    }
}

Context

StackExchange Code Review Q#94123, answer score: 5

Revisions (0)

No revisions yet.