perfect numbers

I am suppose to write a program that finds all the perfect numbers from 1 to 1000. When I try to run this program all I get is:

Perfect Numbers
Press any key to continue.......

Code:

---

#include <iostream>

using namespace std;

int perfect( int number );

int main()
{
    int num = 1, answer;
   
    cout << "Perfect Numbers" << endl;
   
    for ( int z = 1; z <= 1000; z++ )
    { 
        answer = perfect( num );
        if ( answer != 1 )
            cout << answer << endl;           
        num++;
    }
    system("pause");
   
    return 0;
}

int perfect( int a )
{
    int num1, test0, test1, test2, test3 = 1, counter, divider = 1;
   
    num1 = a;
   
    test0 = num1 % 2;
   
    if ( test0 == 0 )
        counter = num1 / 2;
    else
        return 1;
       
    for ( counter; counter >= 1; counter--)
    {
        test1 = num1 % divider;
       
        if ( test1 == 0 )
        {
            test2 = num1 / divider;
            test3 += test2;
        }
        divider++;
    }
   
    if ( test3 == num1 )
        return num1;
    else
        return 1;
}

---

I think the problem is in the prefect function, but i have gone thru this function over and over and it seems to me to be sound. But I have been wrong many times before, please help :confused:

Manual Debugging 101:

Since I don't really have time to check through it too well, let me teach you how to error check your code. When getting unexpected results with if statements, always put an error output in the else statement. This should contain which error it is and the important variables. It's basically like a quick and easy core dump without all of the gibberish you don't need.

Code:

---

int perfect( int a )
{
    int num1, test0, test1, test2, test3 = 1, counter, divider = 1;
   
    num1 = a;
   
    test0 = num1 % 2;
   
    if ( test0 == 0 )
        counter = num1 / 2;
    else {
        cout << "Error 1: " << num1 << "    "  << test0 << endl;
        return 1;
    }
       
    for ( counter; counter >= 1; counter--)
    {
        test1 = num1 % divider;
       
        if ( test1 == 0 )
        {
            test2 = num1 / divider;
            test3 += test2;
        }
        divider++;
    }
   
    if ( test3 == num1 )
        return num1;
    else {
        cout << "Error 2: " << test3 << "    " << num1 << endl;
        return 1;
    }
}

---

Run this and I think you'll find the results quite interesting.

Cool, I'll be back

1)

Code:

---

for ( int z = 1; z <= 1000; z++ )
{ 
        answer = perfect( num );
        if ( answer != 1 )
                cout << answer << endl;           
        num++;
}

---

Write down the value of z and num the first ten times through the loop? What conclusions can you draw from the results?
---
2)

Code:

---

int num = 1, answer;

---

Why did you initialize num and not answer? Rule #1 is: always initialize your variables.
---
3)

Code:

---

system("pause");

---

Don't use system().
---
4)

Code:

---

int num1, test0, test1, test2, test3 = 1, counter, divider = 1;

---

Always initialize your variables-all of them. Also, when you name variables test0, test1, test2, test3 that's an indication you should be using an array: int test[4] = {0}.
---
5)

Code:

---

num1 = a;

---

Why is that necessary? If you don't like the name 'a', then declare your parameter variable name as 'num1'.
---
6)

Code:

---

for ( counter;

---

What do you believe you are doing with 'counter' there?
---

General comments:

a)Your variable names are atrocious. Rule #2 is: use descriptive variable names.

b) I'm pretty sure you are testing your divisors incorrectly, which has to do with your inability to use for-loops correctly(see 1 above). To test for a perfect number, you start at 1 less than the potential perfect number and count down to 1, all the while testing for a factor that has no remainder. When you find a factor with 0 remainder, you add the factor to a running total variable. At the end of the loop, if the running total equals the number, then you have a perfect number.

You are obviously taking some steps to optimize your code by starting your search for factors at (potential perfect number)/2. However, 'counter' is the factor you should be testing to see if it divides evenly into your potential perfect number--not divider.

By the way, I added braces after you replied. I hope you noticed they were missing, use the code that's there now.

It works!!!!!! :D

used your debugging method, SlyMaelstrom.

7stud, thanks for your input. I was already thinking some of what said in your post before I saw your post, but it did reinforced what I was thinking.

Heres the new and working code:

Code:

---

#include <iostream>

using namespace std;

int perfect( int number );

int main()
{
    int num = 1, answer;
   
    cout << "Perfect Numbers" << endl;
   
    for ( int z = 1; z <= 1000; z++ )
    { 
        answer = perfect( num );
        if ( answer != 0 )
            cout << answer << endl;
        num++;           
    }
    system("pause");
   
    return 0;
}

int perfect( int num_2 )
{
    int num_1, check_1, check_2, check_3, start, addtoperfectnum, perfectnum = 1;
   
    num_1 = num_2;
       
    check_1 = num_1 % 2;
       
    if ( check_1 == 0 )
    {
        start = num_1 / 2;
        check_3 = start * 2;
               
        for ( int x = start; x > 1; x-- )
        {
            check_2 = num_1 % start;
                       
            if ( check_2 == 0 )
            {
                addtoperfectnum = num_1 / start;
                perfectnum += addtoperfectnum;
            }
            start--;
        }
        if ( perfectnum == check_3 )
            return perfectnum;
        else
            return 0;
    }
    else
        return 0;
}

---

Thanks again for all your help, guys.
much appreciated

Quote:

---

7stud, thanks for your input. I was already thinking some of what said in your post before I saw your post, but it did reinforced what I was thinking.

---

Unfortunately, you didn't follow most of them. If I was your teacher, I would take off at least 40 pts. for all those deficiencies

1) -5 pts.

2) -5 pts.

3) -5 pts.

4) -5pts.

5) -2pts.

6) -2pts

a) -5pts

b) -10pts

The biggest problem I see is that your program demonstrates you don't know how to use a for-loop. The rest of the stuff is a result of laziness and could be corrected with just a little more effort.

You're also cheating a little bit by including some of the factors(i.e. 1) in your sum. Your sum should start at 0.

...or a little less effort. When 7stud said:

Quote:

---

Originally Posted by 7stud

If you don't like the name 'a', then declare your parameter variable name as 'num1'.

---

He didn't mean change it to something you do like and use the same process. If you don't know this, the parameter variable IS A USEABLE VARIABLE. There is no need to assign it to something else in the function. There is no way to change the value of it in main and you can do whatever you want with it in the function. You should go through his list again and correct those things one at a time. They were all good tips.

:confused: okay, I'm all ears. I'm not really understanding whats lazy about the program, and what is meant by not knowing how to use a for-loop? :(

proper usage of the for-loop:
int num = 1, answer;

Code:

---

    for ( int z = 1; z <= 1000; z++ )
    { 
        answer = perfect(z);
        if ( answer != 1 )
            cout << answer << endl;           
        // num++; YOU DONT NEED num, you have a for loop with that z - so use z!!!
    }

---

same mistake in your other for loop.


also i guess some ppl (like me) didnt even try to understand your code, because your variable names are not descriptive.
calling a variables just test_0, test_1, test_2, ... is almost as bad as:

Code:

---

int salt, water, tomatoes, vinegar;
// ... code ...
int ketchup = salt + water + tomatoes + vinegar; // could you imaging what is actually calculated here?

---

just not as amusing.

instead of:
test_x = a % b;
write
remainder = a % b;

instead of
test_y = a /b;
write something like
quotient = a / b;
so whenever remainder is used anyone who reads that knows that were talking about a remainder of some operation.


also: in for loops usually the variable i is used - this is not a neccessity, but it makes code more readable.
(whenever you see an i, you can assume that there is a for-loop that increases i every time)


note that these are NO GENERAL rules! just examples.
so don't call every result of a modulo operation remainder. give your variables reasonable names.
(e.g. when iterating over elements of a matrix, you may have 2 for loops with the variables row and col)

Code:

---

for(unsigned row = 0; row < n; ++row) {
  for(unsigned col = 0; col < m; ++ col) {
    matrix[row][col] = blah....
  }
}

---

note that n and m are reasonable names, because now we can assume were talking about NxM matrices

Quote:

---

okay, I'm all ears. I'm not really understanding whats lazy about the program,

---

Did you initialize all your variables after I told you Rule #1 is: initialize all your variables?

Quote:

---

and what is meant by not knowing how to use a for-loop?

---

Did you do this:

Quote:

---

Code:

---

for ( int z = 1; z <= 1000; z++ )
{ 
        answer = perfect( num );
        if ( answer != 1 )
                cout << answer << endl;           
        num++;
}

---

Write down the value of z and num the first ten times through the loop? What conclusions can you draw from the results?

---

What were your conclusions?

It was explained to me that a variable intialized in the "()" of a "for" statement can only be used in the "for" statement. Also I was told that passing parameter was in the same category as the "for" variable. But I now know other wise :)

I didn't worry about intializing some of the variables cause they were going to have a value assigned to them before they were going to be used. I do know it can be dangerous not to cause logic errors can be a result of that. But I was most definite that wouldn't happen in this case.

I named my variables the second time I modified the code to make better understanding of the calculations for myself as I was trying to figure out how to get the mathmatics right. I'll work on naming them better along the way of learning the ways of C++

I have definitely learn how to better program, thanks a bunch guys......Is there anythings else?

I did some modifications to the program:

Code:

---

#include <iostream>

using namespace std;

int perfect( int number );

int main()
{
    int num = 1, answer;
   
    cout << "Perfect Numbers from 1 to 1000" << endl;
   
    for ( int z = 1; z <= 1000; z++ )
    { 
        answer = perfect( z );
        if ( answer != 0 )
            cout << answer << endl;         
    }
    system("pause");
   
    return 0;
}

int perfect( int num_1 )
{
    int check_1, check_2, check_3, start, addtoperfectnum, perfectnum = 1;
       
    check_1 = num_1 % 2;
       
    if ( check_1 == 0 )
    {
        start = num_1 / 2;
        check_3 = start * 2;
               
        for ( start; start > 1; start-- )
        {
            check_2 = num_1 % start;
                       
            if ( check_2 == 0 )
            {
                addtoperfectnum = num_1 / start;
                perfectnum += addtoperfectnum;
            }
        }
        if ( perfectnum == check_3 )
            return perfectnum;
        else
            return 0;
    }
    else
        return 0;
}

---

I know some of you don't like the variable names or that the variables aren't initialized, other than that what do you think?

As a next step or optimisation I would get rid of all the unnecessary stuff.

Code:

---

int perfect( int num ){
    int perfectnum = 1;       
    if ( num % 2 == 0 ) {
        for (int start = num / 2 ; start > 1; start-- )
            if ( num % start == 0 )
                perfectnum += num / start;
        if ( perfectnum == ( num / 2 ) * 2 )
            return perfectnum;
    }
    return 0;
}

---

and 7stud would have to give you a lot more points.
Kurt

That does look a whole hell alot better, and I can understand it! I never realized that "if" and "for" statements could have equations like that. It does make things easier and a lot better to read.