Reputation: 57
Hi everyone I edited my post because I have another problem with my code.I had a problem with the factorial function yesterday but I managed to solve it thanks to your answers , it was a ridiculous mistake. The problem now is that for some values that are higher than 15, the final results(not the factorials of individual numbers) are always 0 or -1 for the lesser values it works fine.Can someone tell me whats wrong with this code :
#include <iostream>
#include<time.h>
using namespace std;
int factorial(int a){
if(a==1)
return 1;
else if(a==0)
return 1;
else
return factorial(a-1)*a;
}
int main(){
std::cout.setf(std::ios_base::fixed, std::ios_base::floatfield);
std::cout.precision(5);
int n,k;
int x,y,z,w,v;
cout<<"give n : ";
cin>>n;
cout<<"give k : ";
cin>>k;
clock_t t;
t = clock();
if(n>=k&&k>1){
x=factorial(n-1);
y=factorial(k-1);
z=factorial(n-1-k);
w=factorial(n-k);
v=factorial(k);
cout<<"Result is "<<(x/(v*z))+(x/(y*w))<<endl;
}
else if (n==0||n==k)
cout<<"Result is 1"<<endl;
else
cout<<"Result is 0"<<endl;
t = clock() - t;
cout<<"It took "<<t<<" clicks ("<<((float)t)/CLOCKS_PER_SEC<<" seconds )"<<endl;
return 0;
}
Upvotes: 0
Views: 3609
Reputation: 263307
Integer overflow.
If int
is 32 bits on your system, the largest value it can represent is 2147483647
. 13 factorial is 6227020800
.
If you use a 64-bit integer type such a long long
, you can go up to 20 factorial.
Floating-point will give you more range, but with a loss of precision.
If you really need to compute large factorials, you'll need to use some multi-precision library like GMP, or use a language that has built-in arbitrary-precision integer arithmetic (C doesn't).
Upvotes: 3
Reputation: 500397
The following might call factorial
with a zero or negative argument:
z=factorial(n-1-k);
w=factorial(n-k);
You need to make sure that your factorial
function can handle such arguments without crashing (I suspect it doesn't).
Upvotes: 1