Factorial program in c

Question

Trying to make a code that gets the factorial of the inputted number.

int factorial(int number, int i)
{
    int endval;
    for(i = number - 1; i>0; i--){
        endval = number * i;
    }
    if (endval == 0){
        printf("1");
    }
    return endval;
}

int main()
{
    int endvalue, numA, numB;
    char userchoice[1];
    printf("Enter a choice to make (f for factorial): \n");
    scanf("%s", userchoice);
    if(strcmp(userchoice, "f")== 0){
        printf("Enter a value to get it's factorial: ");
        scanf("%d", &numA);
        endvalue = factorial(numA, numB);
        printf("%d", endvalue);
        return 0;}


    getch();
    return 0;
}

For some reason the whole for loop doesn't do anything in the function when I set the answer (number*i)= endval. It just prints out the same number I inputted and gives me an absurd answer for 0!.

int factorial(int number, int i)
{
    int endval;
    for(i = number - 1; i>0; i--){
            endval = number * i;
    }
    if (endval == 0){
        printf("1");
    }
    return endval;
}

However the code works perfectly fine when I remove endval variable entirely (with the exception that it gets 0! = 10)

int factorial(int number, int i)
{
    for(i = number - 1; i>0; i--){
        number = number * i;
    }
    if (number == 0) {printf("1");}
    return number;
}

Is there anything I missed in the code that's causing these errors?

Just run thru it in a debugger or even on a piece of paper. What is the value of `number` and `endval` on the first iteration? On the second iteration? On the third iteration? etc. It should be very obvious once you write it out line by line. — kaylum, Sep 19 '21 at 20:37
regarding: `char userchoice[1]; ... scanf("%s", userchoice);` this will always result in an error as the `%s` always appends a NUL byte to the result. That means the size of `userchoice` must be AT LEAST 2 bytes. Also, there needs to be a max lennght modifier that is 1 less than the length of the target variable, Otherwise the user can keep entering characters resulting in a buffer overflow (and the resulting undefined behavior — user3629249, Sep 22 '21 at 03:37

Jaguar Nation · Answer 1 · 2021-09-19T22:50:28.110

1

A definiton of factorial is:
factorial(0) = 1
factorial(n) = n * factorial(n-1)
Note: Factorial is legal only for number >= 0

In C, this definition is:

 int factorial(int number)
    {
         if (number < 0)
             return -1;

         if (number == 0)
              return (1);
    
         /*else*/
         return (number * factorial(number-1));
    }

edited Sep 19 '21 at 22:50

answered Sep 19 '21 at 22:33

Jaguar Nation

122
11

miroslav licko · Answer 2 · 2021-09-19T21:34:21.900

0

There are possibly few things to correct. See please attached code.



    int factorial(int number)
    {
    
        if (number == 0){ return 1; }
    
        int endval=1, i;
    
        for(i = 1; i<=number; i++) { endval *= i; }
    
        return endval;
    
    }
    
    int main() {
    
        int endvalue, numA;
        char userchoice[1];
        printf("Enter a choice to make (f for factorial): \n");
        scanf("%s", userchoice);
    
        if(strcmp(userchoice, "f")== 0) {
    
            printf("Enter a value to get it's factorial: ");
            scanf("%d", &numA);
            endvalue = factorial(numA);
            printf("%d", endvalue);
            return 0;
    
        }
    
    getch();
    return 0;
    }

edited Sep 19 '21 at 21:34

answered Sep 19 '21 at 21:24

miroslav licko

81
6

1

Ah I see. So the whole problem with endval was that I didn't initialize it. Well then I'll remove it from the code for good and keep in mind the `return 1;` in future codes. The part with `for(i = 1; i<=number; i++) { endval *= i; }` will be remodified to `for(i = number - 1; i>0; i--) { number *= i; }` for the code to give the correct output (your code gave both the "good" and "wrong" values wrong with the exception that the good one was only getting close from the correct answer. Either way thanks a lot. – Chroma Sep 19 '21 at 21:40
1

if `number` is greater than (about 43) then the `int number` will overflow. SUggest either using a `long long int` and/or checking that `number` is not too big – user3629249 Sep 22 '21 at 03:41

score 0 · Accepted Answer · answered Sep 19 '21 at 21:35

#include <stdio.h>
#include <string.h>

int factorial(int number)
{
    int endval=1;
    for(int i = number ; i>0; i--){
        endval *= i;
    }
    return endval;
}

int main()
{

    int endvalue=0;
    int numA=0;

    char userchoice[1];
    printf("Enter a choice to make (f for factorial): ");
    int ret=scanf("%s", userchoice);
    if (!ret){
        printf("Error in scanf: %d", ret);
    }
    if(strcmp(userchoice, "f")== 0){
        printf("Enter a value to get it's factorial: ");
        scanf("%d", &numA);
        endvalue = factorial(numA);
        printf("%d", endvalue);
        return 0;
    }

    getchar();
    return 0;
}

Code with some changes will work

factorial() function can get only one argument.
As a good habit all variables must be initialized.
Add include statement to source and be explicit not rely on compiler. As we use strcmp() we must include string.h
use standard getchar() instead of getch()
Also can check return value of library function scanf() to ensure reading is correct or not.
You can use warnings from compiler to get most of above notes. In gcc: gcc -Wall code.c
Use a debugger to run program line by line and monitor variables value in each steps or use as many printf() to see what happens in function call.

Thanks a lot, I'm still new to the coding realm so I'll consider all of this. Also, I did include all the necessary header files in my IDE but I thought they'd be redundant over here lmao — Chroma, Sep 19 '21 at 21:44
`scanf()` can return EOF which would cause the posted code to fail — user3629249, Sep 22 '21 at 03:44
check the returned value from `scanf("%d", &numA);` is 1, other wise an error has occurred — user3629249, Sep 22 '21 at 03:46

Factorial program in c

3 Answers3