2

This factorial function starts giving wrong results with 13 and above. I have no idea why.

#include <stdio.h>

int fatorial (int p);

int main() {
    int x = 13;
    int test = fatorial(x);
    printf("%d", test);
}  

int fatorial (int p) {
    if (p <= 0)
        return 1;
    else
        return p*fatorial(p-1);
}

for x = 0, 1, 2 ...12 it prints the right result, but for 13! it prints 1932053504 which is not correct. For x=20 it prints -210213273 for example.

I know that this is not the best way to do a factorial. Its my homework tho, it HAS to be this way.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
bakun
  • 448
  • 1
  • 9
  • 17
  • 2
    The result gets too big for type `int`. – Bodo Sep 15 '20 at 14:33
  • 1
    Try with `unsigned long long int` instead of `int` and you'll get a few more correct factorials. – Stef Sep 15 '20 at 14:35
  • int overflow occurs in your code, you may try to print INT_MAX to check the limit of integer. you may try with unsigned long long int for the highest support of int – ashchk Sep 15 '20 at 14:35

4 Answers4

4

If you try this you will get the maximum value that int can hold:

#include <stdio.h>
#include <limits.h>
int main(void) 
{
    printf("%d\n", INT_MAX);
}

Your code causes overflow.

You could get a few more numbers if you use a bigger type, but not by very much. You could use this:

unsigned long long fatorial (unsigned long long p) {
    if (p <= 0)
        return 1;
    else
        return p*fatorial(p-1);
}

It won't get you far though. If you want bigger than that you need to find a library for bigger integers or create some custom solution. One such library is https://gmplib.org/ but that is likely out of scope for your homework.

And btw, a condition like p <= 0 is not good. It indicates that the factorial of a negative number is always one, which is false.

klutt
  • 30,332
  • 17
  • 55
  • 95
1

It is because after 12, the result of factorial of any number exceeds the size of int.

you can try the following code:

#include<stdio.h>
int main()
{
    int a[100],n,counter,temp,i;
    a[0]=1;
    counter=0;
    printf("Enter the number: ");
    scanf("%d",&n);
    for(; n>=2; n--)
    {
        temp=0;
        for(i=0; i<=counter; i++)
        {
            temp=(a[i]*n)+temp;
            a[i]=temp%10;
            temp=temp/10;
        }
        while(temp>0)
        {
            a[++counter]=temp%10;
            temp=temp/10;
        }
    }
    for(i=counter; i>=0; i--)
        printf("%d",a[i]);
    return 0;
}
Aakash Dinkar
  • 332
  • 1
  • 10
  • if you see in the 5th line I have initialised the value of `a[0] = 1`, so when the first time `a[i]` is used in `(a[i]*n)+temp` there the value of `i = 0` and `a[0] = 1` which i declared in the starting. – Aakash Dinkar Sep 15 '20 at 18:00
0

The result of the function is too big. I think big int would work better for your purposes. Big int allows you to have bigger numbers. Also, this is what I would do.

int x = the number you want to factorialize
int ans = 1;
(Then instead of all of those functions)
for(var i = x; i > 0; i--) {
ans = ans*i;
}

System.out.println(ans);

Javascript link: https://jsfiddle.net/8gxyj913/

Promaster
  • 162
  • 1
  • 12
  • 2
    Ehm, that's javascript? – klutt Sep 15 '20 at 14:45
  • 1
    I an at a complete loss trying to imagine why anyone would upvote this "answer": This question is about C, not about Java, and most certainly not about Javascript. There is no "big int" in C, and the question clearly states that the form of the solution (the algorithm) is due to constraints of their homework. As such, this answer is worthless for the OP, and does not help other C students one bit. – cmaster - reinstate monica Sep 15 '20 at 14:58
  • 1
    @cmaster-reinstatemonica I agree. This answer is unfortunately of absolutely no help – klutt Sep 15 '20 at 19:19
0

I need to get to 100!

100! is about 9.332622e+157. Simply using standard integer types is insufficient. 32-bit int is good to 12!. With 64-bit integer math, code could get to about 21!

Could use floating point math and give up precision.

Instead consider a string approach.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256