-5

I wrote the following code in C to make a program which calculate the factorial of any number.

I want to add to my program some validation/error handling, such as preventing random characters, floats or negative values from being entered, so I used the isdigit function.

Unfortunately there is a hidden problem which I don't know how to solve. When I enter any input it considers it to be false (i.e. not a digit) even if it's a positive digit.

#include <stdio.h>
#include <ctype.h>

int main()
{
    char choice;
    unsigned long long int factorial=1;
    int counter,number;
    for(;;)
    {
        printf("Please , enter a positive integer number only : ");
        scanf("%d",&number);
        fflush(stdin);
        if(isdigit(number))
        {
            for(counter=number;counter>1;counter--)
            factorial*=counter;
            printf("The factorial of number %d is %llu",number,factorial);
        }
        else
        {
            printf("\a\aError\n");
            continue;
        }
        printf("\n1-Press c or C if you want to calculate the factorial of a new number\n2-Press any key         if you want to exit the program\n ");
        scanf("%c",&choice);
        if(choice=='c'||choice=='C')
        {
            factorial=1;
            system("cls");
            continue;
        }
        else
        return 0;
    }
}
Qantas 94 Heavy
  • 15,750
  • 31
  • 68
  • 83
  • Note that 12! is the largest value that fits in a 32-bit (unsigned) integer, and 20! is the largest value that fits in a 64-bit (unsigned) integer. – Jonathan Leffler Sep 28 '14 at 03:03
  • In addition to the answers, suggest dropping the `fflush(stdin)` and changing `scanf("%c",&choice);` to `scanf(" %c",&choice);` (add space). This will consume optional leading white-space including the previous lines . – chux - Reinstate Monica Sep 28 '14 at 03:28

2 Answers2

1

You are using isdigit wrong. Read its documentation to find out what it actually does.

You probably meant:

 if ( number >= 0 && number <= 9 )

However you also need to check whether scanf succeeded or not. If they type in some words, then scanf("%d" fails and does not update number, so trying to access number in that case accesses an uninitialized variable. To deal with that you could either check the return value of scanf, or do:

int number = -1;
scanf("%d",&number);

because the value will be left unchanged if the input failed.

NB. Don't use fflush(stdin)

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
  • Note that if the platform is Windows, `fflush(stdin)` is perfectly well defined. If the platform is not Windows, it is a problem. – Jonathan Leffler Sep 28 '14 at 02:49
  • [O Really!](http://stackoverflow.com/questions/2979209/using-fflushstdin#comment31066899_2979209) — relevant link for MS in the comment referenced. – Jonathan Leffler Sep 28 '14 at 02:55
  • @JonathanLeffler that links is to MSVC documentation . It suggests that "the operating system" "usually" does the buffering but clearly there are cases where fflush(stdin) doesn't work in Windows. Especially if you are not programming in a commode! – M.M Sep 28 '14 at 03:01
  • What do commodes have to do with Windows, other than being where you should put machines that run Windows? I'm not sure what you're driving at; the MSVC manual for [`fflush()`](http://msdn.microsoft.com/en-us/library/9yky46tz.aspx) says _If the stream is open for input, fflush clears the contents of the buffer._ There isn't any wriggle room or iffery in that statement. It isn't even tied to `stdin`; it is any input stream. – Jonathan Leffler Sep 28 '14 at 03:09
  • @JonathanLeffler The MSVC manual should be taken to document the behaviour of MSVC , not other compilers – M.M Sep 28 '14 at 03:18
  • 1
    OK; correction accepted; using the libraries provided with MSVC on Windows… – Jonathan Leffler Sep 28 '14 at 03:19
0

isdigit checks a single character if that's a decimal digit character. But, your input could be say 25, multiple characters. So, I changed a portion:L

 char input[30];
 for(;;)
 {
  printf("Please , enter a positive integer number only : ");
  scanf("%s",input);
  if(isdigit(input[0]))
  {
   number = atoi(input);
   for(counter=number;counter>1;counter--)

Keeping rest of your program snippet same. Here, isdigit is used to check if the first character in input is a digit and therefore a valid candidate to be converted by atoi into an integer value number.

Dr. Debasish Jana
  • 6,980
  • 4
  • 30
  • 69
  • Do **not** use `fflush(stdin);` to flush the input buffer -- it does **not** do what you think it does. Instead declare and `int c;` and then place the following line after each `scanf` handling string input: `do { c = getchar(); } while (c != '\n' && c != EOF);`. That will remove all remaining characters from the input buffer. Prove it to yourself. Enter: `1 2 3` in the code above. It will loop 3 times never pausing after 'printf`. – David C. Rankin Sep 28 '14 at 03:27