What is the best way to ensure a user only enters natural numbers?

Question

Just started learning C, and it would be great if you could help me with the following:

I just wrote a program that calculates the factorial of a natural number entered by the user. If the number is negative or a character, it should notify the user with the message You have not entered a natural number.. This message comes from the function check_if_valid_value.

So far, it displays that message when entering characters, but doesn't seem to work with negative values. I thought casting the variable as long long unsigned would do the trick, but doesn't seem to be the case. The thing is the scanf() function returns a 0 so not sure why the program doesn't run the function check_if_valid_value so that it returns the message:You have not entered a natural number.

I am looking forward to reading any suggestions to improve this piece of code!

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

void check_if_valid_value(int value_entered)
{
        if (value_entered != 1) 
        {
                printf("You have not entered a natural number.\n");
                exit(1);
        }
}          

void do_the_factorial(int p)
{  
        int i;
        unsigned long long int factorial=1;

        for (i = 1; i <= p; ++i)
        {
                factorial=factorial*i;
                printf("%llu\n", factorial);
        }
}


int main(void)
{
        int value_entered, p;

        printf("Enter a natural number:");

        value_entered=scanf("%llu",&p);

        check_if_valid_value(value_entered);

        do_the_factorial(p);

        return 0;
}

Read *carefully* the documentation of [scanf](https://en.cppreference.com/w/c/io/fscanf) and/or the [n1570](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf) C11 draft standard. Compile your code with all warnings and debug info (`gcc -Wall -Wextra -g` with [GCC](http://gcc.gnu.org/)...). Read [*How to debug small programs*](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) and [*about undefined behavior*](http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html) — Basile Starynkevitch, Apr 25 '20 at 17:32
What do you mean by "casting the variable"? If you mean the use of scanf format `%llu`, that's not a cast. The variable is an `int`, the argument to `scanf` is a "pointer to `int`", and the conversion code `%llu`, which implies that the argument is a "pointer to `long long unsigned`", is a lie. Lying about argument types is Undefined Behaviour. Don't do it. And enable compiler warnings to help you to not do it. — rici, Apr 25 '20 at 18:09
Why the downvotes? There's a programmatic problem and the OP shows what he tried. Let's cut Henry some slack and identify where the bugs are. — Jens, Apr 25 '20 at 18:28

Jens · Accepted Answer · 2020-04-25T18:58:33.707

Problems with your code:

scanf returns the number of successful conversions. So you should maybe rename that variable successful_conversions.
The scanf format for a pointer-to-unsigned-integer is %u, not %llu.

You could also start the factorial loop at 2, since multiplying by 1 is mostly useless. So we have:

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

void check_if_valid_value(int successful_conversions)
{
        if (successful_conversions != 1)
        {
                printf("You have not entered a natural number.\n");
                exit(1);
        }
}

void do_the_factorial(unsigned int p)
{
        unsigned int i;
        unsigned long long int factorial=1;

        for (i = 2; i <= p; ++i)
        {
                factorial=factorial*i;
        }
        printf("%llu\n", factorial);
}

int main(void)
{
        int successful_conversions;
        unsigned int p;

        printf("Enter a natural number:\n");
        successful_conversions=scanf("%u",&p);
        check_if_valid_value(successful_conversions);
        do_the_factorial(p);
        return 0;
}

A word of caution: scanning %u allows the number to have a sign, which can be negative. If you enter -1 then p will be the largest positive unsigned integer, UINT_MAX. Then do_the_factorial() will enter a very long loop. You should add some check if p is small enough so the factorial of p does not exceed the range of unsigned long long.

If you want to test the input more stringently, you should learn how to read in a short string and then test whether the string consists entirely of decimal digits. As a starting point, look at fgets().

I find it amazing that `scanf` accepts a negative number and returns `1` when given an `unsigned` specifier such as `%u`. — Weather Vane, Apr 25 '20 at 18:52
Thank you very much @Jens! :-) This really helps me improve much quicker! All the best! — Henry, Apr 25 '20 at 19:23
@WeatherVane That's because unsigned arithmetic in C is modulo 2^N, so can't underflow nor overflow. -1 is equivalent to `UINT_MAX` and that's what scanf converts. — Jens, Apr 25 '20 at 19:51

What is the best way to ensure a user only enters natural numbers?

1 Answers1