Why doesn't my program that computes perfect numbers print anything?

Question

I got an assignment to create a program which displays the perfect integers between one and 100. Here is the actual assignment:

Create a PerfectIntegers application that displays all perfect integers up to 100. A perfect integer is a number which is equal to the sum of all its factors except itself. For example, 6 is a perfect number because 1 + 2 + 3 = 6. The application should include a boolean method isPerfect().

I tried and came up with this:

import java.util.ArrayList;
public class PerfectIntegers {
public static boolean isPerfect(int a){
    ArrayList<Integer> factors = new ArrayList<Integer>();
    int sum=0;
    boolean is;
    for (int i=1; i<=100; i++){
        double r=a/i;
        if (r%1==0){
            factors.add(i);
        }
    }for (int i=0;i<factors.size();i++){
        sum+=factors.get(i);
    }if (sum==a){
        is=true;
    }else{
        is=false;
    }return is;
}
public static void getInts(){
    for (int i=2; i<=100; i++){
        boolean is=isPerfect(i);
        if (is!=false){
            System.out.print(i+" ");
        }
    }
}
public static void main(String[] args) {
    getInts();
}

}

Eclipse did not show any errors but when I try to run it, the program is terminated and I get nothing.

The problem is likely with double r, as it is not dividing properly 100% of the time.

Maybe `is` is always false. Have you tried debugging it? – Robert Harvey Feb 06 '14 at 22:50 — Robert Harvey, Feb 06 '14 at 22:50

Sergey Kalinichenko · Accepted Answer · 2014-02-06T23:14:45.927

Your factorization code is wrong. You can fix it like this:

for (int i = 1 ; i < a; i++) {
    if (a % i == 0) {
        factors.add(i);
    }
}

One reason your old code did not work is that you misunderstood the workings of the % operator. It computes the remainder of the division of the left-hand side by the right-hand side, so r % 1 == 0 will be true for all numbers, because 1 divides everything; r % 2 == 0 is a way to detect even numbers, and so on.

The other reason is that you went all the way to 100 in search of divisors. This necessarily includes a, which automatically puts the total above the number itself, because 1 is already on the list.

Once you get this working, you could simplify the code by dropping the list of factors. Since the sum of all factors is all that you need, you might as well compute it in the factorization loop, and drop the loop that follows it:

sum = 0;
for (int i = 1 ; i < a; i++) {
    if (a % i == 0) {
        sum += i;
    }
}

mweisz · Answer 2 · 2014-02-06T23:16:13.813

dasblinkenlight has already provided the correct answer. Let me just add that since this seems to be a (potentially graded) assignment you might consider refactoring the getInts() method as well.

boolean is=isPerfect(i);
if (is!=false){
    System.out.print(i+" ");
}

is actually equal to

if (isPerfect(i)){
    System.out.print(i+" ");
}

since isPerfect() already returns a boolean that can be used inside the condition of the if-statement. It can be argued (even though I strongly disagree in this concrete case) that it might be more readable to first store the return value in a variable like in in the first variation. But even then you should never have to check for

if (is!=false) { //...

but should be using

if (is) { // ...

instead.

Why doesn't my program that computes perfect numbers print anything?

2 Answers2