Finding perfect numbers My teacher wants me to find all the perfect numbers between 2 and 1000 and must use:


for loops
% operator to determine if a number is a divisor
static final variables for 2 and 1000
A static method to return a boolean value by finding and adding the divisors of the integer parameter and comparing the sum to the parameter


The code works and I just wanted a second opinion on it.

public class Problem_B {
    //Search between 2 and 1000
    public static final int minNum = 2; 
    public static final int maxNum = 1000; 

    public static void main(String[] args) {
        for (int num=minNum; num &lt;= maxNum; num++) 
        if (perfectNum(num)) {
            int i_sumDiv = 0;
            for (int j=1; j &lt;= (num/2); j++) {
                if(num%j == 0)
                    i_sumDiv = i_sumDiv + j; 
            }
            if (i_sumDiv == num) 
                System.out.println(num + " is a perfect number.");
        }
    }

    public static Boolean perfectNum(int num) {
        // a variable that holds the sum of the factors 
        int sum = 0;

        for (int factor = 1; factor &lt; num; factor++) {
            if (num % factor == 0) {
                sum += factor;
            }
        }

        if (sum == num) {
            return true;
        } else {
            return false;
        }
      }
    }
//Search between 2 and 1000
public static final int minNum = 2; 
public static final int maxNum = 1000; 



That comment doesn't say anything that the code doesn't already state IMO. Now if you need to change either value, you have to update the comment as well, otherwise you end up with this:


//Search between 2 and 1000
public static final int minNum = 2; 
public static final int maxNum = 10000; 



And would that be a typo in the code, or just a misleading comment?


for (int num=minNum; num &lt;= maxNum; num++) 



Spacing isn't consistent between operators, that's distracting when you read it.

for (int num = minNum; num &lt;= maxNum; num++) 


Reads more fluently. Now, you have a scope, but where's the opening brace? Code that's explicit won't omit them, even for a single statement.


if (perfectNum(num)) {



Might be I'm the verbose type, but I don't see a reason not to rename num to number or value, and perfectNum to isPerfectNumber, making the condition read like this:

if (isPerfectNumber(value)) {


Now, I like how minNum and maxNum are consistent with num, so I'd then rename them to minValue and maxValue to maintain that beautiful consistency.


int i_sumDiv = 0;



Now that's a bit of a mystery. why is there an i_ prefix? The Div abbreviation seems uncalled for, and makes the name a bit ambiguous.

Okay I need to stop the line-by-line analysis here.



You're doing the work twice. First here:


if (perfectNum(num)) {



and then if the function returns true, you do the work all over again - you know it's a perfect number already; you're ready to output right there and then:

if (isPerfectNumber(value)) {
    System.out.println(num + " is a perfect number.");
}


So let's look at that function.


public static Boolean perfectNum(int num) {
    // a variable that holds the sum of the factors 
    int sum = 0;

    for (int factor = 1; factor &lt; num; factor++) {
        if (num % factor == 0) {
            sum += factor;
        }
    }

    if (sum == num) {
        return true;
    } else {
        return false;
    }
  }
}



I like factor much more than j. I'm seeing braces on all scopes, and although sum isn't a completely descriptive noun, it's much better than i_sumDiv: I like this implementation much better than the superfluous one that executes after this function returns.

The indentation is broken though:


    }
  }
}



Should be:

        }
    }
}


Also, this is redundant:


    if (sum == num) {
        return true;
    } else {
        return false;
    }



It can easily be replaced with a single return statement:

return sum == num;



for (int factor = 1; factor &lt; num; factor++) {



That loop is inefficient; the (redundant) one in main is better.