Factorial program displaying the inital value and not the factorial itself

Question

Below is my program to calculate the factorial of a given int. It compiles but only displays the initial value of the fact. It most likely has something to do with the if(argc>1) statement, but that is required for the assignment.

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char* aragv[])
{
   int fact = 1;
   int n = atoi(char* aragv[]);
   if(argc>1)
   {
      for(int i = 1; i < n ; i++)
    {
      n = n * i;
    }
   }
   printf("%d\n", fact);
   return 0;
}

How are you running the program you create from this code? To get something other than `7` printed, you need to run `./program anything` or something similar. However, your loop condition isn't going to work well. You're multiplying `fact` by `i` and then checking whether `i` is still less than `fact`. You need to include ``; you don't need to include ``. — Jonathan Leffler, Mar 01 '22 at 03:46
is alreay included it must have cut out that part. I am running it via ubuntu terminal — Matthew Darragh, Mar 01 '22 at 03:49
Don't put code on the `\`\`\`` lines! My question about "how are you running it" was oriented towards "what does the command line look like?" Are you using `./program` (with no arguments) or `./program something` or `./program somebody anybody nobody everybody` or something else? Note that the conventional name for the second argument to `main()` is `argv` — `aragv` isn't wrong, but it is unusual. There's also a `return -1;` in there confusing the results (or absence of results). — Jonathan Leffler, Mar 01 '22 at 03:50
gcc factoiral.c o-factorial ./factorial. I am new it C and ik this is the default way — Matthew Darragh, Mar 01 '22 at 03:58
Please [edit](https://stackoverflow.com/posts/71303482/edit) the post with that info so that it can be properly formatted. Looks like the command is wrong but not sure whether that is your actual command or just a typo. Please make sure it is exact. — kaylum, Mar 01 '22 at 04:10
Do you know what `argc` is and how/when that value changes? If not, why did you write the code `if(argc>1)`? What did you intend for that to do? — kaylum, Mar 01 '22 at 04:12
Updated question with recent coding changes. anything in between the if(argc>1) is changeable. Anything else we are not allowed to change — Matthew Darragh, Mar 01 '22 at 04:31
You see the problem right? `int fact = 1;` and `for(int i = 1; i < fact; i++)` -- the scenery never changes... from `i = 1` to `i < 1` doesn't do much... — David C. Rankin, Mar 01 '22 at 04:32
It takes that value of i, multiply it by fact until i reaches fact. — Matthew Darragh, Mar 01 '22 at 04:35
but... for the first iteration `i == 1` and `fact == 1` and `i !< fact` so nothing happens `:)` — David C. Rankin, Mar 01 '22 at 04:37
If you run the code with `./factorial`, then `argc` is 1, and everything inside the `if` statement is skipped. So my guess is that your instructor actually wants you to use a command line argument. — user3386109, Mar 01 '22 at 04:48
You were correct, it was a command line argument. I updated the code to reflect this, its just a matter of debugging now. — Matthew Darragh, Mar 02 '22 at 16:59

जलजनक · Answer 1 · 2022-03-01T04:56:26.033

Heavily commented code :

#include <stdio.h>
#include <stdlib.h>

#define USAGE "USAGE: ./program [+ve number upto 20]" // define constants & repetitive texts

int main(int argc, char* argv[]) // argc & argv have universal appeal
{
    //int fact = 7; // use larger integer
    unsigned long int factorial = 1; // 0! = 1

    //if(argc>1) // instead check program is run with an argument
    if (argc != 2) { // invalid program run
        printf ("\n\t%s\n", USAGE);
        return 1; // return non-zero to indicate error to caller
    }
    int number ; //= atoi (argv[1]); // convert string to integer; number is in argv[1]
    // @DavidC.Rankin correction
    if (sscanf (argv[1], "%d", &number) != 1) { // to make sure we read "a number"
        printf ("\n\tERROR: Invalid Number [%s]\n\t%s\n", argv[1], USAGE);
        return 3;
    }
    if (number > 20 || number < 0) { // long int (64 bits) can't store more than 20!
        printf ("\n\tERROR: [%d] Out of valid range [0 to 20]\n\t%s\n", number, USAGE);
        return 2; // different number for different scenarios
    }
    for (int i = 2; i <= number; i++) // you can skip 1 as doesn't make a difference
        factorial = factorial * i; 

    printf("\nAnswer:\t %d! = %lu\n", number, factorial); // %lu for unsigned long int

    return 0;
}

When you're learning, it always helps if you learn in increments.

For quick error check, suggest `if (sscanf (argv[1], "%d", &number) != 1) { /* handle error */ }`. Or for full diagnostics use `strtol()` or `strtoul()`. The problem with `atoi()` is it will happily accept `atoi ("my cow")` silently returning `0` with no indication of error. (just a thought...) — David C. Rankin, Mar 01 '22 at 04:35

Factorial program displaying the inital value and not the factorial itself

1 Answers1