Factorial Calculator C++ issues

[jwebb]

Hello, I'm trying to create a program to calculate the factorial of variable int input. I am using xcode, and it's giving me an error saying "factorial was not declared in this scope" on the line within the else statement. I have found other programs on the internet, but I would simply like to understand why mine won't work. I would be grateful for any constructive suggestions or comments.
Thanks.
Here is what I have so far:

#include <iostream>

using namespace std;

int main(int input)
{
    cin >> input;

    if (input == 0) 
    {
        cout << "1" << endl;
    }

    else
    {
        cout << factorial << endl;
    }

}

int factorial(int input)
{
    cin >> input;
    input * (input - 1);
}

[burgercho]

There are multiple problems:
You need a forward declaration of factorial since it is called before it is declared (add the line 'int factorial(int input);' after the 'using...' line)
You have to write factorial as a function call with input as the argument (as opposed to a variable):

cout << factorial(input) << endl;

Also, factorial does not have a return statement to return the number so it won't print, you would need to add 'return input;' at the end of the function.

[daviddoria]

Also, please use code tags when posting code.

[tesuji]

Well, and the body of function factorial(...) should be better completely replaced by

{
  if(input<=1) return 1;
    else return input*factorial(input-1);
}

[jwebb]

Ok I took a bit of all of your advice and revised it so there is no longer an error, but now I realize it's it's not looping the multiplication, for example, "5" (enter) = "20," not 120, which is 5!
Thanks!
Here is the revised code:

#include <iostream>

using namespace std;

int factorial(int input)
{
	cin >> input;
	
	if(input == 0)
	{
		cout << "1" << endl;
	}
	
	else
	{
		return input * (input - 1);
	}
}

int main(int input)
{
	cout << factorial(input) << endl;
}

[burgercho]

That is because the factorial function is only multiplying the input by itself minus 1. You would need to do something similar to what tesuji suggested. Also the input doesnt need to be a commanline argument, just leave it as an argument to factorial and put the 'cin' command in main (like you had it structured before)
Alternatively, for a quick fix, you can just change line 16 with:

for(int i=input-1;i>1;i--) {
    input *= i;
}

[tesuji]

well jwebb, this is kind of "disimproving".

Why didn't you exactly do what I suggested?

int factorial(int input)
{
// my suggestion and NOTHING else!
}
int main(){
int input;
cin >> input;
cout << factorial(input);
return 0;
}

-- tesu

[sfuo]

Recursion is cool but when I was testing out using loops vs it any number 4 and above took longer to calculate when using the recursive function (time taken is not noticeable) and recursion takes up more memory because it keeps adding to the stack.

This is the code I used

unsigned int factorial( unsigned int num )
{
	if( num == 0 || num == 1 )
		return 1;
	unsigned int result = 1;
	for( unsigned int i = num; i > 1; i-- )
		result *= i;
	return result;
}

I used unsigned int just because you cannot use negative values or get a negative value from the factorial function.

[jwebb]

Ok so I changed the code according to tesuji's suggestion, and it works! But isn't that recursion? I was trying to get around that. But thank you!
This is my first real experience posting to a forum, and I'm really pleased with the speed of responses and quality of assistance thus far, so thank you!
jwebb

Here's the code:

#include <iostream>

using namespace std;

int factorial(int input)
{
	
	if(input == 0)
	{
		return 1;
	}
	
	else
	{
		return input * factorial(input - 1);
	}
}

int main(int input)
{
	cin >> input;
	cout << factorial(input) << endl;
}