How to calculate the prime factors of a number using a function?

[Reireirei]

Hello. I'm new to c++ and trying to get used to using declaring and using functions. For practise I'm trying to make a program to take an integer input from the user to calculate the prime factors by implementing a function. I have gotten so far but keep getting more odd errors when compiling.

 

#include <iostream>

#include <cmath>

using namespace std;
/* This program takes a whole number x, finds the factors of 
x (y), finds the factors which are prime numbers and outputs 
these prime number factors*/

int factorpri(int x){ // factor is function to find prime factors

int y;

for(int y=1;y<=x;y++){ //find factors of x

if((x%y)==0){

for(int w=1;w<=y;w++){

int w;

if((y%w)>0){ // find if factors are prime
         
cout << y <<" is a prime factor of "<< x <<endl; //print the prime factors

}

else{

return 0;

}

}

}

else{

}


}


int main(){

int x;

cout << "Enter a number to be factored: " << endl;

cin >> x >> endl; //input number to be factored

if(x%1 == 0){ //check using modulus to see if number is positive integer

int res=factorpri(x); // perform function

}

else{

cout << "Please enter a positive integer." << endl; // print if number not integer

}

}

I now get 3 errors:

Q4c.cpp: In function ‘int factorpri(int)’:
Q4c.cpp:43: error: a function-definition is not allowed here before ‘{’ token
Q4c.cpp:10: warning: unused variable ‘y’
Q4c.cpp:67: error: expected `}' at end of input

It's even saying I cannot declare the function before main(){}!? Is it partly something to do with one of the else{} options being empty?

Any help is appreciated.

[jsmith]

Perhaps it was just copy-paste, but the formatting is horrible and that is what is causing you trouble.

Let me format it for you and then you can spot the problem.

#include <iostream>
#include <cmath>

using namespace std;

/* This program takes a whole number x, finds the factors of 
x (y), finds the factors which are prime numbers and outputs 
these prime number factors*/
int factorpri(int x) // factor is function to find prime factors
{ 
    int y;

    for(int y=1;y<=x;y++) //find factors of x
    {
        if((x%y)==0)
        {
            for(int w=1;w<=y;w++)
            {
                int w;  // <--- THIS IS BAD

                if((y%w)>0) // find if factors are prime
                {
                    //print the prime factors
                    cout << y <<" is a prime factor of "<< x <<endl; 
                }
                else
                {
                    return 0;
                }
            }
        }
        else
        {
        }
    }

// ------------ HMMMMMMMM ------------
    int main()
    {
        int x;

        cout << "Enter a number to be factored: " << endl;

        cin >> x >> endl; //input number to be factored

        if(x%1 == 0) //check using modulus to see if number is positive integer
        {
            int res=factorpri(x); // perform function
        }
        else
        {
            // print if number not integer
            cout << "Please enter a positive integer." << endl; 
        }
    }

[Reireirei]

Thanks very much. Formatting with indenting does help spot the different levels of loops. I have noticed I did not have a curly bracket } to close the for loop started on line13. I see that I have declared the integer w on line19 after I used it in the for loop. I don't believe I need to declare integers y and w on lines 19 and 11 as I have also declared them in the loop?

I now have:

#include <iostream>
#include <cmath>

using namespace std;

/* This program takes a whole number x, finds the factors of 
x (y), finds the factors which are prime numbers and outputs 
these prime number factors*/
int factorpri(int x) // factor is function to find prime factors
{ 
               
    for(int y=1;y<=x;y++) //find factors of x
    {
        if((x%y)==0)
        {
      int x;
            
            for(int w=1;w<=y;w++)
            {
                 
                if((y%w)>0) // find if factors are prime
                {
                    //print the prime factors
                    cout << y <<" is a prime factor of "<< x <<endl; 
                }
                else
                {
                    return 0;
                }
            }
        }
        else
        {
        }
     }
}


    int main()
    {
        int x;

        cout << "Enter a number to be factored: " << endl;

        cin >> x >> endl; //input number to be factored

        if(x%1 == 0) //check using modulus to see if number is positive integer
        {
            int res=factorpri(x); // perform function
        }
        else
        {
            // print if number not integer
            cout << "Please enter a positive integer." << endl; 
        }
    }

The only error I get now:

Q4c.cpp:49: warning: unused variable ‘res’

Probably means something is wrong with the syntax here or the function still isn't working properly.

[Mythios]

Your making res equal factorpri which doesn't always return something.

[jsmith]

Well, you declare and assign res a value on line 50, but you never do anything with the variable
beyond that. So the compiler is asking you why you are assigning res a value and then never
using it.

what value is factorpri() supposed to return? Currently, you return zero, but only in one particular
case. In other cases, you don't have return statements, so the value that will be assigned to res
in those cases is completely undefined.

[Reireirei]

Oh, doh! Ok, so I should put return 0; after the else by line 34 so I return 0 incase the number isn't a factor. I should put return y; instead of the cout statement on line 25. I need to get multiple outputs of y from the function factorpri(). So I need to get the y values outputted into an array and then interate through the array to cout each value. The y values need to be inputted into an array in the function or use a pointer to the array I am outputting. I'll have a go. Does it seem the correct way to proceed?

[jsmith]

I would use a std::vector<> or std::deque<> instead of an array; they are much easier to manipulate.