Reputation: 65
So this program is a simple decimal to binary converter.
I want my code to repeat until user presses ctrl + D
. I also want to let the user know that the number input is not a positive whole number if they put in a number like -2
or 1.1
.
The problem is when they enter a float
my code just infinitely prints my first print statement.
This is my code:
void DecToBin(int userInput){
int binary[32];
int i = 0;
while (userInput > 0) {
binary[i] = userInput % 2;
userInput /= 2;
i++;
}
for (int j = i - 1; j >= 0; --j) {
printf("%d", binary[j]);
}
}
int main(void) {
int userDec;
int res;
while(1) {
printf("Please enter a positive whole number (or EOF to quit): ");
res = scanf("%d", &userDec);
res = (int)(res);
if (res == EOF) break;
else {
if (!((userDec > 0) && (userDec % 1 == 0))) {
printf("Sorry, that was not a positive whole number.\n");
}
else {
printf("%d (base-10) is equivalent to ", res);
DecToBin(userDec);
printf(" (base-2)!");
printf("\n");
}
}
}
printf("\n");
return 0;
}
So when I input something like 11.1
or a negative value it shouldn't accept it and print "Please enter a positive whole number (or EOF to quit)"
in the terminal.
Inputs like 11.0
should be accepted.
So, I what I'm asking is what is wrong with my code that makes it do this, and how should I fix it? Should I use a float
instead?
Upvotes: 1
Views: 993
Reputation: 153348
The most robust approach does not use scanf()
to reads a line of user input. Instead use fgets()
and then parse the string with strto*()
and friends.
Since OP is looking for valid input as an integer or as a floating point number with a whole value, construct helper routines to do each job well.
#include <ctype.h>
#include <errno.h>
#include <stdio.h>
void base(int n) {
if (n == 0)
return;
base(n / 2);
printf("%d", n % 2);
}
// Return error flag, 0 implies success
int String2int(int *dest, const char *s, int min, int max) {
if (dest == NULL)
return 1;
*dest = 0;
if (s == NULL)
return 1;
errno = 0;
char *endptr;
long L = strtol(s, &endptr, 0);
if (s == endptr)
return 1; // no conversion
if (errno == ERANGE || L < min || L > max)
return 1; // out of range
while (isspace((unsigned char) *endptr))
endptr++;
if (*endptr)
return 1; // Extra text after input
*dest = (int) L;
return 0;
}
int String2Whole(double *dest, const char *s, double min, double max_plus1) {
if (dest == NULL)
return 1;
*dest = 0.0;
if (s == NULL)
return 1;
errno = 0;
char *endptr;
double d = strtof(s, &endptr);
if (s == endptr)
return 1; // no conversion
// Save whole part into dest
if (modf(d, dest))
return 1; // Test return of non-zero fractional part.
if (d < min || d >= max_plus1)
return 1; // out of range
while (isspace((unsigned char) *endptr))
endptr++;
if (*endptr)
return 1; // Extra text after input
return 0;
}
Armed with 2 helper functions that are picky about conversion, try one, then the other as needed.
char *doit(void) {
int i = 0;
char buf[100];
if (fgets(buf, sizeof buf, stdin) == NULL) {
return "Input closed";
}
if (String2int(&i, buf, 0, INT_MAX)) {
double x;
if (String2Whole(&x, buf, 0.0, (INT_MAX / 2 + 1) * 2.0)) {
return "Invalid input";
}
i = (int) x;
}
printf("O happy day! %d\n", i);
return 0;
}
int main(void) {
doit();
}
Upvotes: 1
Reputation: 23792
The condition userDec % 1 == 0
is always true, the remainder of any integer divided by 1 is always 0.
When scanf("%d", &userDec)
can't parse the inputed value, it returns 0, that happens when your input is not a number, you can take advantage of that.
To check if a value has a decimal part we can use math.h
library function modf
which separates the fractional part of a double from its integral part and returns it, so yes, it would be a good strategy to use double
(instead of float
which is less precise).
(In Linux, using gcc
you may need to use -lm
compiler flag to link math.h
header.)
Applying these changes to your code, you would have something like this:
#include <stdlib.h>
#include <math.h>
int main(void)
{
double userDec;
int res;
double integral;
while (1)
{
printf("Please enter a positive whole number (or EOF to quit): ");
res = scanf("%lf", &userDec);
if (res == EOF) break;
// if res = 0 input was not a number, if decimal part is 0 it's an integer
if (res == 0 || userDec < 0 || modf(userDec, &integral))
{
printf("Sorry, that was not a positive whole number.\n");
int c;
while ((c = getchar()) != '\n' && c != EOF){} // clear buffer
if (c == EOF)
{
return EXIT_FAILURE; // safety measure in case c = EOF
}
}
else
{
printf("%d (base-10) is equivalent to ", res);
DecToBin(userDec);
printf(" (base-2)!");
printf("\n");
}
}
printf("\n");
return EXIT_SUCCESS;
}
Considering your specs, an input of 11.0
will be valid with this code because its decimal part is 0
, while something like, for instance, 11.00000001
is not, because, of course, its decimal part is not 0
.
The above code has a vulnerability, if the inputed value is larger the INT_MAX
, scanf
does not have the tools to deal with it. If you really want to get serious about this, you would parse the input as a string and convert it with something like strtod
:
#include <stdlib.h>
#include <math.h>
#include <string.h>
#include <errno.h> // may be needed for errno
int main(void)
{
double userDec;
double integral;
char input[100];
char *endptr = NULL;
int c;
// parse input as string
while (printf("Please enter a positive whole number (or EOF to quit): ") && scanf("%99[^\n]", input) == 1)
{
errno = 0;
userDec = strtod(input, &endptr); // convert to double
// if negative or overflow or invalid input or fractional
if (userDec < 0 || (userDec == HUGE_VAL && errno == ERANGE) || *endptr != '\0' || modf(userDec, &integral))
{
printf("Sorry, that was not a positive whole number.\n");
}
else
{
printf("%.0lf (base-10) is equivalent to ", userDec);
DecToBin(userDec);
printf(" (base-2)!\n");
}
while ((c = getchar()) != '\n' && c != EOF) { continue; } // clear buffer
if (c == EOF) { return EXIT_FAILURE; } // c = EOF, treated as unrecoverable
}
return EXIT_SUCCESS;
}
Of course with something like this you would need to prop up your converter function to accept larger values, it now only accepts int
as argument.
Either way all these input limits in your code should be conveniently documented.
Upvotes: 1
Reputation: 123448
Remember that scanf
returns the number of successful conversions and assignments, or EOF
on end of file or read error.
The %d
conversion specifier will skip over leading whitespace and read decimal digits up to the first non-decimal digit character. If the first non-whitespace character is not a decimal digit, then you have a matching failure and scanf
will return a 0
, not EOF
.
When you enter 11.1
, scanf
successfully converts and assigns the 11
to userDec
and returns 1
. However it leaves the remaining .1
in the input stream - that never gets cleared.
On the next read, scanf
sees .1
and immediately stops reading and returns a 0
. However, you're only testing that scanf
doesn't return EOF
, so it never breaks out of the loop.
With scanf
you're better off testing against the number of items you expect to be successfully converted - in this case, your test should be
if ( res != 1 )
break;
I'd restructure your read loop as follows:
while ( 1 )
{
// prompt
if ( (res = scanf( "%d", &userDec)) != 1 )
{
// print error message
break;
}
else
{
if ( userDec <= 0 )
{
// print error message
}
else
{
// process input
}
}
}
if ( res != EOF )
{
// had a matching failure from bad input
}
else
{
// user signaled EOF
}
Since the %d
conversion specifier will only accept integers as input, and since userDec
can only hold integer values, there's no need to test userDec
itself for integer-ness. The userDec % 1 == 0
expression will always be true and is meaningless in this context.
If you want to validate that the user typed in an integer string vs. something else, then you'll have to do a bit more work. You'll have to check the return value of scanf
and you'll have to test the first character following what %d
accepted:
int itemsRead;
int userDec;
char dummy;
/**
* Read the next integer input and the character immediately
* following.
*/
itemsRead = scanf( "%d%c", &userDec, &dummy );
/**
* Here are the possible scenarios:
*
* 1. If itemsRead is 2, then we read at least one decimal digit
* followed by a non-digit character. If the non-digit character
* is whitespace, then the input was valid. If the non-digit
* character is *not* whitespace, then the input was not valid.
*
* 2. If itemsRead is 1, then we read at least one decimal digit
* followed immediately by EOF, and the input was valid.
*
* 3. If itemsRead is 0, then we had a matching failure; the first
* non-whitespace character we saw was not a decimal digit, so the
* input was not valid.
*
* 4. If itemsRead is EOF, then we either saw an end-of-file condition
* (ctrl-d) before any input, or there was a read error on the input
* stream.
*
* So, our test is if itemsRead is less than 1 OR if itemsRead is 2 and
* the dummy character is not whitespace
*/
if ( itemsRead < 1 || itemsRead == 2 && !isspace( dummy ) )
{
// input was not valid, handle as appropriate.
}
else
{
// input was valid, process normally
}
Upvotes: 0