I am not able to find out why my function returns the user input only rather then the factorial of the input.

#include <stdio.h>
#include <math.h>
int factorial(int x)
{
    //int x;
    int sum = 1;
    while (x!=0){
        sum = sum * x;
        x--;
    }
    return sum;
}

int main(){
    int x;
    printf("Enter value of x: ");
    scanf("%i",&x);
    factorial(x);
    printf("sum is %i", x);
    
    return 0;
}

CodePudding user response：

Your factorial function does return a new value, but then you don't actually use that value.

printf("sum is %i\n", factorial(x));

CodePudding user response：

Because you are printing x which is the variable that you have stored the user input in. Your factorial function returns the result, but you are not saving it.

CodePudding user response：

I think the variable names were not proper and you printed x instead of printing factorial.

#include <stdio.h>
int factorial(int x)
{
int fact = 1;
while (x!=0){
    fact = fact * x;
    x--;
}
return fact;
}

int main(){
   int x;
   printf("Enter value of x: ");
   scanf("%i",&x);
   printf("Factorial is %i",factorial(x));
   return 0;
}

CodePudding user response：

For starters the function as is

int factorial(int x)
{
    //int x;
    int sum = 1;
    while (x!=0){
        sum = sum * x;
        x--;
    }
    return sum;
}

can invoke undefined behavior if the user will pass a negative value, and the function accepts negative values.

The function argument should have an unsigned integer type instead of the signed integer type int

For non-negative values the maximum value of the types int or unsigned int for which the factorial can be calculated is equal to 12.

So to be able to calculate the factorial for greater values you should use the type unsigned long long int. In this case the maximum value for which the factorial can be calculated correctly is equal to 20.

The function can look the following way

unsigned long long int factorial( unsigned long long int x )
{
    unsigned long long int product = 1;

    for ( ; 1 < x; --x )
    {
        product *= x;
    }

    return product;
}

In your program you are not using the returned value of the function.

factorial(x);

The function main can look the following way

int main( void )
{
    unsigned int x;

    printf( "Enter value of x: " );

    if ( scanf( "%u",&x ) == 1 )
    {
        printf("The factorial of %u is equal to %llu\n, x, factorial( x ) );
    }
    
    return 0;
}

Now try the program by entering the value for x equal to 20 and see the program output.:)

You could check in the if statement that the user did not enter a value greater than 20 as for example

    if ( scanf( "%u",&x ) == 1 && !( 20 < x ) )

Though it would be better if the function itself will check the value of the passed argument.

CodePudding user response：

Failure to use function return value

As others have said:

//factorial(x);
//printf("sum is %i", x);
printf("sum is %i", factorial(x));

To improve factorial()

• Since factorial is a product, change the sum name.

• Cope with pathologic inputs like values less than 0 or very large. Code code exit, but maybe instead return a error value. Determination of the upper limit could be done beforehand (as below), at run time or with makefile magic.

• Use a wider type to handle large values. Maybe even use an unsigned type. uintmax_t has the greatest max value. It is at least 64 bits.

Example

#include <limits.h>
#include <stdint.h>

#if UINTMAX_MAX == 0xFFFFFFFFFFFFFFFFu
  #define FACTORIAL_MAX 20
#elif (UINTMAX_MAX >> 64) == 0xFFFFFFFFFFFFFFFFu
  #define FACTORIAL_MAX 34
#else
  #error TBD FACTORIAL_MAX
#endif

// Compute factorial.
// Return 0 on error.
uintmax_t factorial(int x) {
  if (x < 0 || x > FACTORIAL_MAX) {
    return 0;
  }
  uintmax_t product = 1;
  while (x > 0) {
    product = product * x;
    x--;
  }
  return product;
}