Follow

Keep Up to Date with the Most Important News

By pressing the Subscribe button, you confirm that you have read and are agreeing to our Privacy Policy and Terms of Use
Contact

C function gives wrong result when function call on main is refactored

I’m writing a factorial function for practice. On main I declare the variables and call it like this:

int main(){
    unsigned long n = 0;
    scanf("%lu", &n);
    unsigned long result = factorial(n);
    printf("%lu \n", result);
    return 0;
}

Which works and gives the result I want. But I thought it looked kinda bad so I refactored it like this:

int main(){
    unsigned long n = 0;
    printf("%lu \n", factorial(scanf("%lu", &n)));
}

Which somehow always gives 1 as a result no matter what. Why? Here’s the factorial function just in case:

MEDevel.com: Open-source for Healthcare and Education

Collecting and validating open-source software for healthcare, education, enterprise, development, medical imaging, medical records, and digital pathology.

Visit Medevel

unsigned long factorial(unsigned long n){
    if(n == 1){
        return n;
    }
    else if(n <= 0){
        printf("No factorial for negative numbers");
    }
    else{
        return (n * factorial(n - 1));
    }
    return 0;
}

I did this same exercise on Python and formatted the function call in the same way and it worked, so I’m confused as to why C doesn’t accept it. Does it not call functions withing functions from the innermost first (scanf, then factorial, then printf in my case)?

>Solution :

  1. Missing include.

  2. As @RetiredNinja said the issue is that scanf() returns the number of arguments parsed not the value of n. I added error checking here to clarify this further.

  3. factorial(): n is unsigned so n <= 0 is better written as n == 0 or just !n.

  4. factorial(): The placement of the return 0; statement is odd as it only executes for the case n <= 0.

  5. factorial(): return n is not wrong but as it’s a base case it’s clearer to use return 1.

  6. (Not fixed) factorial(): Prefer an iterative implementation as C doesn’t guarantee tail call optimization (hence with sufficiently large input value your stack may overflow). As @Fe2O3 points out in this case you will overflow long before stack overflow, though.

  7. (Not fixed) factorial() on my system returns 0 for n > 65 due to overflow. Maybe add an upper limit check?

  8. Prefer the signature main(void) to main(). In practice they will do the same but only the former is defined in the standard (along with main(int, char *[])).

  9. (Not fixed) You may only want to call factorial() if n > 0, or store the value in a variable like your initial implementation so you can omit the output for n == 0 instead of:

    0
    No factorial for zero
    0
    
#include <stdio.h>

unsigned long factorial(unsigned long n) {
    if(!n) {
        printf("No factorial for zero\n");
        return 0;
    }
    if(n == 1)
        return 1;
    return n * factorial(n - 1);
}

int main(void) {
    unsigned long n;
    if(scanf("%lu", &n) != 1) {
        printf("scanf failed\n");
        return 1;
    }
    printf("%lu\n", factorial(n));
}

and example run:

6
720 
Add a comment

Leave a Reply

Keep Up to Date with the Most Important News

By pressing the Subscribe button, you confirm that you have read and are agreeing to our Privacy Policy and Terms of Use

Discover more from Dev solutions

Subscribe now to keep reading and get access to the full archive.

Continue reading