Factorial Recursive

Question

I'm trying to write an algorithm to calculate the factorial of a number using recursive function.

This is my code :

#include <stdio.h>
#include <conio.h>
#include <iostream>
using namespace std;
int factorial(int n) {
    cin >> n;
    if (n == 1)
        return 1;
    return n*factorial(n - 1);
}
int main() {
    int n = 0;
    cout<<"Enter a number:";
    cout << factorial(n);
    return 0;
}

It does nothing and I don't know why, it only lets me to give the number, but it's not calculating.

for a start, put the cin in main instead of the actual recursive function. — Max, Mar 11 '18 at 18:10

taskinoor · Accepted Answer · 2018-03-11T18:15:41.623

2

Inside the factorial function you are waiting for another input which is not needed. Remove this cin >> n; from inside the factorial method.

Some other points which are not asked in the question:

Factorials grow very fast and by using int you will quickly get an overflow. You may consider to use 64 bit long long instead.
conio.h is not standard and should be avoided.
Calling using namespace std in global scope is very bad.

edited Mar 11 '18 at 18:15

answered Mar 11 '18 at 18:11

taskinoor

45,586
12
116
142

1

12! is the largest factorial that can be held in a 32-bit signed or unsigned `int`. – Weather Vane Mar 11 '18 at 18:15
can you tell me why is bad to call using namespace std in global? – Mari Andrei Mar 11 '18 at 18:16
@MariAndrei Why is “using namespace std” considered bad practice? https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice – taskinoor Mar 11 '18 at 18:18
... and you can't go very much further with 64-bit `int`, only to 20! – Weather Vane Mar 11 '18 at 18:19
@WeatherVane yes but I guess using a 3rd party big number library at this stage will be difficult for OP. – taskinoor Mar 11 '18 at 18:20

score 1 · Answer 2 · answered Mar 11 '18 at 18:54

you initialised

n=0;

which doesn't taking input from you in main fn and calling factorial fn always with

factorial(0);

and also remove that cin>>n from fact fn and do something like

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

main()
 {
  cin>>n;
  cout<<factorial(n);
  }

score 0 · Answer 3 · answered Mar 11 '18 at 18:19

Your program isn't doing nothing. It is doing what you do not expect. cin >> n should be in the main function, not in the factorial function.

What it is actually doing is placing each new number as a function call to factorial on the stack. Every time you re-input you're changing the terms of the factorial. For example, you input "4", and then "6", and then "8"... what you have on the stack is factorial(8) on top, then factorial(6), then factorial(4). Eventually you'll have to input "1" so that your program will end.

score 0 · Answer 4 · edited Jun 05 '21 at 14:08

0

Do not ask for user input inside factorial method. Do this instead

int factorial(int n) {
    if (n == 0 || n == 1)
        return 1;
    return n*factorial(n - 1);
}
int main() {
    int n;
    cout<<"Enter a number:";
    cin >> n;
    cout << factorial(n);
    return 0;
}

edited Jun 05 '21 at 14:08

Amisha Sahu

89
6

answered Mar 11 '18 at 18:21

Mahesh Nepal

1,385
11
16

rauprog · Answer 5 · 2018-08-11T02:33:58.043

For large factorials, use floating point which supports exponential notation for numbers with a lot of 0s.

Also, what recursion does is push the numbers passed in number factorials to the stack and then return them once a function call finally returns, which is when it is 1.

For illustrative purposes, try this program.

// ConsoleApplication1.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>
using std::cout;
using std::endl;
using std::cin;

int factorialtimes2(int n) {
    if (n <= 1)
        return 1;
    return n*factorialtimes2(n - 1)*2;
}

int main()
{
    cout << factorialtimes2(5) << endl;
    cout << 5 * (4 * 2)*(3 * 2)*(2 * 2)*(1 * 2) << endl;
    return 0;
}

Factorial Recursive

5 Answers5

Linked