patternjavaModerate
Finding the perfect number from 1 to n
Viewed 0 times
numbertheperfectfindingfrom
Problem
I have a program to find the perfect number from 1 to n:
How can this be reduced without changing it?
How can this be reduced without changing it?
class PerfectNumber
{
public static StringBuffer fact(int a)
{
StringBuffer sb=new StringBuffer(" ");
for(int i=1;i<a;i++)
if(a%i==0)
{
sb.append(i+",");
}
return sb;
}
public static String isPrime(int a)
{
int sum=0;
for(int i=1;i<a;i++)
{
if(a%i==0)
{
sum+=i;
}
}
if (sum==a)
{
return "true";
}
else
return "false";
}
public static int perfecrNo(int a)
{
int sum=0;
for(int i=1;i<a;i++)
{
if(a%i==0)
{
sum+=i;
}
}
return sum;
}
public static void main(String[] args)
{
System.out.println("--------------------------------------------------------------------------");
System.out.printf("%s%40s%15s%15s%n","NO","fact","sum","isPrime");
System.out.println("--------------------------------------------------------------------------");
for(int i=1;i<100;i++)
{
System.out.printf("%n %d %40s\b %15d %15s",i,fact(i),perfecrNo(i),isPrime(i));
}
}
}Solution
Indentation
To pick one example:
What's going on here? My best guess would be that you're mixing tabs and spaces with a tabstop of 8. Be consistent: tabs or spaces. Changing the tabstop shouldn't break the formatting.
Names
What do these three methods do?
Factoring out common code
All three of those methods have a loop which does the same thing: find the factors of
Then two of the methods sum the factors: that's already pulled out into a method, so the other method shouldn't duplicate it. Instead you should have a very simple method
Rethink method signatures
Why does
Why does
To pick one example:
for(int i=1;i<a;i++)
{
if(a%i==0)
{
sum+=i;
}What's going on here? My best guess would be that you're mixing tabs and spaces with a tabstop of 8. Be consistent: tabs or spaces. Changing the tabstop shouldn't break the formatting.
Names
public static StringBuffer fact(int a)
public static String isPrime(int a)
public static int perfecrNo(int a)What do these three methods do?
- The first one produces a string containing the factors. Before reading the implementation I had guessed that it calculated the factorial. Name could be more descriptive: e.g.
formatFactors.
- The second one tests whether the number is a perfect number or not. The name is simply wrong: it claims that the method does something different to what it does. It should be e.g.
isPerfectNumber.
- The third one calculates the sum of the factors. The name certainly doesn't communicate that to me. It should be e.g.
sumOfFactors(oraliquotSumif you don't mind the technical term).
Factoring out common code
All three of those methods have a loop which does the same thing: find the factors of
a. That should be pulled out into a single method, which should have a suitable return type (e.g. Collection, although there's a good case for Set, or maybe some integer stream type, but I'm not up to date on Java 8).Then two of the methods sum the factors: that's already pulled out into a method, so the other method shouldn't duplicate it. Instead you should have a very simple method
public static boolean isPerfectNumber(int a)
{
return a == sumOfFactors(a);
}Rethink method signatures
Why does
fact return a StringBuffer? I can't think of many situations in which it makes sense to return a StringBuffer (or a StringBuilder). Either you're appending to a StringBuffer/StringBuilder, in which case it should almost always be an argument and doesn't need to be returned, or you aren't, in which case it makes more sense to return a String.Why does
isPrime (isPerfectNumber) return a string "true" or "false"? The boolean type exists for precisely the type of things which are true or false.Code Snippets
for(int i=1;i<a;i++)
{
if(a%i==0)
{
sum+=i;
}public static StringBuffer fact(int a)
public static String isPrime(int a)
public static int perfecrNo(int a)public static boolean isPerfectNumber(int a)
{
return a == sumOfFactors(a);
}Context
StackExchange Code Review Q#163375, answer score: 10
Revisions (0)
No revisions yet.