C++ Perfect Number. Need some help revising.

Question

I need some help revising this. It keeps only displaying 0s as the temp. Thank you.

// A program to determine whether the input number is a perfect number
// A perfect number is defined by the sum of all its positive divisors excluding itself
// 28: 1+2+3+7+14 = 28. 

int perfect, limit, divisor;

cout << "Please enter a positive integer in order to define whether it is a perfect integer or not: " ;
 cin >> perfect;
 cout << endl;

 int temp = 0;
 int prevtemp = 0;
  limit = 1;
  divisor = 1;
 while (limit < perfect)
 {

  if ((perfect % divisor) == 0)
   {
   divisor = prevtemp;
   temp = prevtemp + temp;
   }

  limit++;
  divisor++;
 }

 if (perfect == temp)
  cout << "Your number is a perfect number!" << endl;
 else
  cout << "Your number is not a perfect number" << endl;

 return 0;

Answer 1

You are never setting prevtemp to anything other than 0, so adding it to temp does nothing.

I believe you meant to say

if ((perfect % divisor) == 0) 
    temp += divisor; // not "divisor = prevtemp;"

The line "temp = prevtemp + temp" should also be removed with this solution; there is no longer any need for the prevtemp variable.

Also, there is no need to keep separate limit and divisor variables, since they are always the same. Just remove limit and change the loop condition to use divisor.

Also, as Mark Byers pointed out, the loop would be simpler to understand if you refactored it into a for loop rather than a while.

Answer 2

You are never assigning anything to prevtemp after initializing it to 0, so there is nothing to add to temp on the line that reads temp = prevtemp + temp.

Answer 3

I'm not sure, but I'd guess that in the code:

if ((perfect % divisor) == 0)
    divisor = prevtemp;

you intended this to be prevtemp=divisor instead. That fixes an obvious problem, but still leaves quite a bit that doesn't look like it's doing that you probably intended. For example, I can't quite figure out what limit is intended to accomplish -- you initialize it and increment it, but as far as I can see, you never use its value (well, I guess you use it, but its value is always the same as divisor's so I'm not sure why you think you need both, or how limit makes any sense as its name).

Edit: It would make sense to have a limit. In particular, factors always come in pairs: one that's less than or equal to the square root of the number, and one that matches the first that's always greater than or equal to the square root of the number. As such, you don't need to scan all the way up to the number itself looking for factors -- you can set the square root of the number as the limit, and scan only up to that point. For each factor you find up to that point, the matching factor will be perfect/divisor. Since you've already gotten one working example, I guess I might as well just hope this isn't homework, and post an example as well:

bool is_perfect(int number) { 
    int limit = sqrt((double)number);
    int sum = 1;

    for (int i=2; i<=limit; i++)
        if (number % i == 0) 
            sum += i + number/i;
    return sum == number;
}

Answer 4

It seems like you are making it too complicated. Here's how you could do it:

int total = 0;
for (int i = 1; i < perfect; ++i)
{
    if (perfect % i == 0)
        total += i;
}

if (perfect == total)
    cout << "Your number is a perfect number!" << endl;
else
    cout << "Your number is not a perfect number" << endl;

Note that the running total is kept in a variable called total (you called this variable temp) and it is only increased when the number is an exact divisor.