Factorial case 13 and 16

[aitía]

My factorial program does not give me the correct results in cases, for instance 13 should be 6227020800 and my output is 1932053504. I display my C program below, is someone able to help me fix it for these cases? I know the formula for factorial is n × (n - 1).

int    factorial(int nb)
{
    int    res;

    res = 1;
    if (nb == 0 || nb == 1)
        return (1);
    while (nb > 1)
        res *= nb--;
    return (res);
}

 

[Antero360]

My factorial program does not give me the correct results in cases, for instance 13 should be 6227020800 and my output is 1932053504. I display my C program below, is someone able to help me fix it for these cases? I know the formula for factorial is n × (n - 1).

int    factorial(int nb)
{
    int    res;

    res = 1;
    if (nb == 0 || nb == 1)
        return (1);
    while (nb > 1)
        res *= nb--;
    return (res);
}

Hi there, first of, one thing you must consider: the limitations of positive ints. What you're going to need is an unsigned long, not an int. Also, you have the right idea as far as the while loop, but just need a bit of modification is needed. Rather than multiplying result by one less than the current number, you need to break that up into two statements. Do the multiplication first, then decrement by 1

 

[aitía]

Hi, I have made changes to my code according to your suggestions. One thing I am wondering is my function needs to return an int because this is how I declare it before the function name but you mentioned using unsigned long which I added as a local variable which means I have different types. Now I also wonder why I am receiving blank output, have I made the right changes?

int    factorial(int nb)
{
    unsigned long res;

    res = 1;
    if (nb == 0 || nb == 1)
        return (1);
    while (nb > 1)
        res *= nb;
        nb--;
    return (res);
}

 

[Antero360]

Hi, I have made changes to my code according to your suggestions. One thing I am wondering is my function needs to return an int because this is how I declare it before the function name but you mentioned using unsigned long which I added as a local variable which means I have different types. Now I also wonder why I am receiving blank output, have I made the right changes?

int    factorial(int nb)
{
    unsigned long res;

    res = 1;
    if (nb == 0 || nb == 1)
        return (1);
    while (nb > 1)
        res *= nb;
        nb--;
    return (res);
}

just change the return type you are expecting.. from int to long

 

[Deleted member 2829]

Actually the original while loop

    while (nb > 1)
        res *= nb--;

is perfectly ok. This is a post-decrement, meaning use the value first, then decrement it. No need to split it up.

The idea of changing int to unsigned long was good, but not good enough. unsigned long is still a 32-bit variable with a maximum value of 4294967295. So all results from fac(13) onwards will be wrong. You need to use unsigned long long which can go all the way up to 18446744073709551615. Needless to say you need a 64-bit computer and a 64-bit C compiler for this.

#include <stdio.h>

unsigned long long factorial(int nb)
{
    unsigned long long res;

    res = 1;
    if (nb == 0 || nb == 1)
        return (1);
    while (nb > 1)
        res *= nb--;
    return (res);
}


main ()
{
    int n;
    for ( n=0; n<=20; n++)
        printf("%2d! = %llu\n", n, factorial(n));
}

Output:

 0! = 1
 1! = 1
 2! = 2
 3! = 6
 4! = 24
 5! = 120
 6! = 720
 7! = 5040
 8! = 40320
 9! = 362880
10! = 3628800
11! = 39916800
12! = 479001600
13! = 6227020800
14! = 87178291200
15! = 1307674368000
16! = 20922789888000
17! = 355687428096000
18! = 6402373705728000
19! = 121645100408832000
20! = 2432902008176640000

 

[aitía]

This works, thank you for your informative responses!