I am learning C from C Primer Plus, and am still a beginner. I have been introduced to only two functions for input, scanf()
and getchar()
.
According to me, when stdin
is associated with input from a keyboard, there are 4 possibilities for user input in this program:
- User provides valid input and presses enter.
- User provides valid input and triggers EOF.
- User provides invalid input (including no input) and presses enter.
- User provides invalid input (including no input) and triggers EOF.
For (1) and (2), I want the program to continue normally. For (3), I want the program to keep asking the user for valid input. For (4), I want the program to abort.
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#define PROMPT "Enter a non-negative number: "
double get_valid_input(void);
bool flush(void);
int main(void)
{
double inputNum = get_valid_input();
printf("Your input = %.2f\n", inputNum);
return 0;
}
double get_valid_input(void)
{
double inputNum;
bool endOfFile;
int a;
while ((printf(PROMPT), (a = scanf("%lf", &inputNum)) != 1)
|| ((a == 1) && (inputNum < 0.0)))
{
a = 0;
endOfFile = flush();
if (endOfFile == true)
break;
}
if (a == 0)
exit(EXIT_FAILURE);
else
endOfFile = flush();
return inputNum;
}
bool flush(void)
{
int f;
while ((f = getchar()) != '\n' && f != EOF)
continue;
if (f == EOF)
{
printf("\n");
return true;
}
else
return false;
}
3 Answers 3
Non-exported functions
Declare get_valid_input
and flush
as static
since you're in a single translation unit.
Foot-cannon one-liners
while ((printf(PROMPT), (a = scanf("%lf", &inputNum)) != 1)
takes the common C antipattern of in-condition assignment even further: it's an entire code-block in a condition. Please consider rewriting this to
while (true) {
printf(PROMPT);
a = scanf("%lf", &inputNum);
if (a == 1 && inputNum >= 0)
break;
// ...
scanf
It's a mess. It pollutes the input buffer on failure, among other nasty surprise behaviours. Any serious attempt at comprehensive input validation ends up ditching scanf
in favour of sscanf
. Read http://c-faq.com/stdio/scanfprobs.html and http://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html for flavour.
General Observations
- The code is quite readable.
- Using void in the function declarations is good.
- Using stdlib.h for
EXIT_FAILURE
is good, but thereturn
frommain()
should bereturn EXIT_SUCCESS;
Avoid Global References of Any Form
There are good reasons to use #define CONSTANT
to define constants used throughout programs, but doing this for prompt strings probably isn't a good idea. If the program being developed needs to communicate with users in multiple languages it is better to pass the C equivalent of a string char *str
into a function instead. It would be better to define prompt
as a local variable in main and pass it by reference into the function get_valid_input()
.
Function Prototypes
In a small program such as this it is better to put the functions in the proper order to reduce the amount of code to be written. Reducing the amount of code necessary is better because if the function declarations need to change they only need to be changed in one place rather than multiple places. Function prototypes are best when linking multiple code files together, then the prototypes are in a header file associated with the source file.
Bug
"User provides ... no input and presses enter.". scanf("%lf", &inputNum)
does not return in that case. Waits for non-white-space input.
Beware % in the prompt
printf(PROMPT)
can lead to undefined behavior (UB) with when PROMPT
has a '%'
in it. Use fputs()
// printf(PROMPT)
fputs(PROMPT, stdout)
Bug
"Enter a non-negative number" --> Rejecting values with inputNum < 0.0
still allows input like "-0.0"
and "NaN"
.
inputNum < 0.0
--> (signbit(inputNum) || isnan(inputNum))
or the like.
Bug
scanf("%lf", &inputNum)
allows an input line like "123.4xyz\n"
as scanf("%lf"...
does not work with a line of input.
Debug with %g
"%g"
is more informative with small values and less noise with large ones.
// printf("Your input = %.2f\n", inputNum);
printf("Your input = %.2g\n", inputNum);
Pedantic: stdin
error not handled clearly
Code fails to distinguish between end-of-file and rare stdin
error. stdin
error is different than textual input error.
Pedantic: flush()
may consume good data
Rare: scanf("%lf", &inputNum)
returns EOF
due to a transient input error. The following flush()
then may consume good input.
Alternative
Some unchecked sample code to explore other ideas.
#include <ctype.h>
#include <float.h>
#include <math.h>
#include <stdio.h>
// Reasonable input perhaps as long as DBL_MAX in non-exponential notation.
#define GET_POSITIVE_LINE_N DBL_MAX_10_EXP
double get_postive_double(const char *prompt) {
// Readily handle inputs up to 2x reasonable length.
// More code needed to detect nefarious super long inputs.
char buf[GET_POSITIVE_LINE_N * 2];
// Possible to construct other loop styles.
// I find the below clear.
while (1) {
if (prompt) {
fputs(prompt, stdout);
}
if (fgets(buf, sizeof buf, stdin) == NULL) {
// Suggest some note about early exit and where
fprintf(stderr, "%s:%u end-of-file", __FILE__, __LINE__);
exit(EXIT_FAILURE);
}
// As reasonable, declare objects when needed
char *endptr;
errno = 0; // See note below
double inputNum = strtod(buf, &endptr);
// No conversion
if (buf == endptr) {
continue;
}
// Look for trailing non-white-space junk
while (isspace(*(unsigned char* )endptr)) {
endptr++;
}
if (*endptr) {
continue;
}
// Allow only positive values
if (signbit(inputNum) || isnan(inputNum)) {
continue;
}
return inputNum;
}
// Code never reaches this point
}
strtod()
could first use a errno==0
to later detect various errors. I find the default handling sufficient and detailed handling of errno
, especially with tiny values not that portable, so left that pedantic corner out.
-
\$\begingroup\$ I'll definitely incorporate all of these wonderful ideas once I'm done with the book :-) \$\endgroup\$Kushagr Jaiswal– Kushagr Jaiswal2021年05月13日 16:41:22 +00:00Commented May 13, 2021 at 16:41
(a == 1)
check is redundant, since it is in the 2nd part of an||
expression. (This comment is redundant, given @Reinderein's reply below, but I include it for completeness.) Also, if you make thePROMPT
, thescanf
format string, and a validation function into parameters, you could have the start of a generic input validation module. \$\endgroup\$