0

I am writing a program to calculate the factorial of a number. I am using recursion to solve this problem. The problem I am running into is that once I reach number 13, it will throw garbage numbers because of INT's limit. What I want to do is implement a way to catch the error when it happens (without hard cording that at x=13 it has to stop, but rather by the output). This is my attempt:

#include <stdio.h>

int factorial( int n)
{
    printf("Processing factorial( %d )\n", n);

    if (n <= 1)
    {
        printf("Reached base case, returning...\n");
        return 1;
    }
    else
    {
        int counter = n * factorial(n-1);       //Recursion to multiply the lesser numbers

        printf("Receiving results of factorial( %d ) =  %d * %d! = %d\n", n, n, (n-1), counter);

        if( counter/n != factorial(n-2) ) //my attempt at catching the wrong output
        {
            printf("This factorial is too high for this program ");
            return factorial(n-1);
        }
        return counter;


        printf("Doing recursion by calling factorial (%d -1)\n", n);
    }


}


int main()
{
    factorial(15);
}

The problem with this is that the program now never terminates. It keeps on looping and throwing me random results.

Since I cannot answer my own question, I will edit with my solution:

int jFactorial(int n)
{
    if (n <= 1)
    {
        return 1;
    }
    else
    {
        int counter = n *jFactorial(n-1);
        return counter;
    }
}

void check( int n)
{
    int x = 1;
    for(x = 1; x < n+1; x++)
    {
        int result = jFactorial(x);
        int prev = jFactorial(x-1);
        if (((result/x) != prev) || result == 0 )
        {
            printf("The number %d makes function overflow \n", x);
        }
        else
        {
            printf("Result for %d is %d \n", x, result);
        }
    }
}
Thomas Dickey
  • 51,086
  • 7
  • 70
  • 105
  • Why would `counter / n` ever equal `factorial(n-2)`? Never mind the fact that adding an extra recursive case is a little questionable, your math is just wrong. – hobbs Jun 04 '14 at 17:54
  • @hobbs I am trying to check for factorial(N)/N not equal the factorial of the previous number because that would tell me if the output is correct or not. – user3708229 Jun 04 '14 at 17:56
  • yes, I understand that. – hobbs Jun 04 '14 at 17:56
  • "the previous number" isn't `factorial(n-2)`, it's `factorial(n-1)`. – hobbs Jun 04 '14 at 17:57
  • I take it you are aware that recursion is a bad way to calculate a factorial compared to simply calculating the product of 2..n. – Andrew Morton Jun 04 '14 at 17:59
  • Yes that is right, but it still loops infinitely – user3708229 Jun 04 '14 at 17:59
  • @user3708229 I've implemented the full C solution in my answer. – Spundun Jun 04 '14 at 18:46
  • @user3708229 Actually, you **can** answer your own questions. http://blog.stackoverflow.com/2011/07/its-ok-to-ask-and-answer-your-own-questions/ . Also the new code that you have posted, does it work as expected? Does it terminate? – Spundun Jun 04 '14 at 19:52
  • Why are these numbers signed? What is factorial(-9) supposed to *mean*? – wildplasser Jun 04 '14 at 23:02

2 Answers2

2

A better way to do it:

if (n <= 1) {
   return 1;
} else {
    int prev_fact = factorial(n - 1);
    if (INT_MAX / prev_fact < n) { /* prev_fact * n will overflow */
        printf("Result too big");
        return prev_fact;
    } else {
        return prev_fact * n;
    }
}

Uses a more accurate check (I hope) for whether the multiplication will overflow, and doesn't add any more calls to factorial.

hobbs
  • 223,387
  • 19
  • 210
  • 288
1

Update

After looking more closely, turns out I missed the fact that gmp is also implemented for C. Here is the solution in C

I've been able to run it on my macbook pro, using homebrew to install gmp (brew isntall gmp)

#include <gmp.h>

#include <stdio.h>

void factorial(mpz_t ret, unsigned n) {
  if (n <= 1) {
    mpz_set_ui(ret, 1);//Set the value to 1
  } else {
    //multiply (n-1)! with n
    mpz_t ret_intermediate;
    mpz_init (ret_intermediate);//Initializes to zero
    factorial(ret_intermediate, n-1);
    mpz_mul_ui(ret, ret_intermediate, n);
  }

  return;
}

int main(){
  mpz_t result;
  mpz_init (result);
  factorial(result, 100);
  char * str_result = mpz_get_str(NULL, 10, result);
  printf("%s\n", str_result);
  return 0;
}

Original Answer

After quick googling, I found the following solution. Note this is a C++ solution. I briefly descirbe how you could do the same thing in ANSI C at the bottom.

Big numbers library in c++

https://gmplib.org/ This c++ library can work on numbers arbitrarily large.

Checkout https://gmplib.org/manual/C_002b_002b-Interface-General.html

The whole code could look something like....

#include <gmpxx.h>

#include <iostream>

mpz_class factorial(unsigned n) {
  if (n <= 1) return mpz_class(1);

  return mpz_class(n) * factorial(n-1);
}

int main(){
  mpz_class result = factorial(100);
  std::string str_result = result.get_str();
  std::cout << str_result << std::endl;
  return 0;
}

The ANSI C Version

You could implement the same thing using ansi C, with a structure to hold expanding list of numbers(using linked-list or any other expandable arraylist containers), and you'd only need to implement three methods... initialize, multiply and convert to string.

Community
  • 1
  • 1
Spundun
  • 3,936
  • 2
  • 23
  • 36
  • But that would not work if the result overflows. And it doesn't look like ANSI C – user3708229 Jun 04 '14 at 18:18
  • yes you are right this is C++ solution. With something like factorial you'd have to implement the big number class like `mpz_class` by hand. Which is doable actually. – Spundun Jun 04 '14 at 18:20
  • @user3708229 I've added a brief guideline on adapting it to ANSI C. Let me know if there's still any confusion. – Spundun Jun 04 '14 at 18:25
  • check my edit and tell me if you think it is a better solution than yours – user3708229 Jun 04 '14 at 19:33
  • @user3708229 This answer will not overflow until the program completely runs out of memory. – Spundun Jun 04 '14 at 19:53