11
\$\begingroup\$

This code was made to find all the perfect numbers between 1 and 1000. A perfect number, is a number where all of it's divisors add up to that number.

For example:

6 is divisible by 3,2, and 1 and3+2+1 = 6. Therefore 6 is a perfect number.

public class Perfect {
    public static void main(String[] args) {
        final int LIMIT = 1000;
        boolean isPerfect = false;
        int i;
        for(i = 2; i < LIMIT; i++) { 
            isPerfect = isNumPerfect(i);
            if(isPerfect) {
                System.out.println(i + " is a perfect number");
            }
        }
    }
    public static boolean isNumPerfect(int i) {
        boolean isPerfect = false;
        int sum = 1;
        int x;
        for(x = 2; x <= i / 2; x++) {
            if(i % x == 0) {
                sum += x;
            }
        }
        if(sum == i) {
            isPerfect = true;
        }
        return isPerfect;
    }
}

What are some ways that I can improve on this code? More specifically, ways to shorten it.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

You don't need variables like isPerfect. Loop variables should be declared in the for-loop header to limit their scope. Both of these changes simplify your code just a bit and improve readability.

I also suggest renaming in and xi. x has the connotation of being a floating-point number, and i has the connotation of being a loop counter.

Here is your program with just those recommended changes.

public class Perfect {
    public static void main(String[] args) {
        final int LIMIT = 1000;
        for (int i = 2; i < LIMIT; i++) { 
            if (isNumPerfect(i)) {
                System.out.println(i + " is a perfect number");
            }
        }
    }

    public static boolean isNumPerfect(int n) {
        int sum = 1;
        for (int i = 2; i <= n / 2; i++) {
            if (n % i == 0) {
                sum += i;
            }
        }
        return sum == n;
    }
}
\$\endgroup\$
4
\$\begingroup\$

If you're trying to shorten it, there are only 3 perfect numbers up to 1000: just list 6, 28, and 496.

If you're trying to speed it up, an array of 1000 numbers is small enough to stay reasonably cache-resident: you can compute all divisor sums together faster than you can compute each of them one at a time. This is similar to how you can sieve a long interval for primes much faster than you can test each number individually for primality. Code fragment:

for(int i=1; i<=n/2; i++) {
  for(int x=2*i; x<=n; x+=i) {
    divisor_sum[x] += i;
  }
}

The length of the inner loop is O(n/i), which gets small very quickly, making the total cost O(n log n), whereas your double loop is O(n^2).

\$\endgroup\$
2
  • 1
    \$\begingroup\$ You're right. My old answer made a silly mistake misreading the way numbers were generated. Thank you for correcting. It did not deserve as many upvotes as it did. \$\endgroup\$
    – Dair
    Commented Dec 21, 2015 at 4:56
  • \$\begingroup\$ I'm trying to shorten the length of the code, but I want the program to find out which are the perfect numbers, I don't want to just print them out. \$\endgroup\$
    – Jacob
    Commented Dec 21, 2015 at 12:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.