2

Creating a simple C function that prevents the user from entering anything other than a number between 1 and 9. Any other input including letters symbols and any number that is either less than 1 and greater than 9 should not be accepted. So far its pretty straight forward; however, the part where the code is supposed to check that the entered character is not a symbol or letter is not working the way I wanted it to work.

int validateUserInput(){
    printf("%s\n", "Please enter a number from 1 to 9: ");

    char value = getchar();
    int numValue = value;
    char temp;

    int digitCounter = 0;

    while((temp = getchar()) != '\n'){
        digitCounter++;
    }
    //if there is more than 1 digit.
    if(digitCounter>0){
        printf("%s\n","Input too long!");
        return validateUserInput();
    }
    // if the char entered is not between 1 and 9
    // this part is giving me a hard time.
    else if(numValue < 49 || numValue > 57){
        printf("%s\n", "Imput is not within the valid parameters");
        return validateUserInput();
    }
    return numValue;
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Carlos Luis
  • 213
  • 2
  • 17
  • 2
    Don't use 49 and 57; use `'1'` and `'9'`. (Is that a misspelling of 'Impute' or 'Input'?) It's better to say something like "Enter a single digit between 1 and 9", clearly indicating the range. I might even be good to show the value you're rejecting. – Jonathan Leffler Aug 31 '16 at 03:42
  • 2
    Are you expecting the function to return the ASCII code, or the number itself? If it's the latter, you need return `numValue - '0';` – samgak Aug 31 '16 at 03:43
  • 1
    `` has `isdigit( ch )` and `` has `strlen( str )`, you can use that to solve your problem. – user2296177 Aug 31 '16 at 04:01

3 Answers3

1

There are a couple of additional points to look at when taking input with your input routine. What if the user needs to cancel input? (e.g. presses ctrl+d, or ctrl+z on windows) As you have it written, there is no way to cancel the loop. (while that works for your purpose of forcing only a single input of 1-9, it leaves no way to cancel) If you trap EOF, you provide both, a way to cancel, and a way to indicate cancellation back in the calling function (e.g. by checking the return against EOF)

While recursion has its place, be aware that each recursion is itself a separate function call that requires a separate stack, and all the other trappings of a function call. There are many times when that can be avoided with a simple goto statement. glibc makes regular use of goto (e.g., check the source of qsort, getdelim, etc..) In your case, a single goto label can completely eliminate the need for recursion. For example, you could do something similar to the following while meeting all of your criteria:

int validateuserinput()
{
    int c, extra, tmp;

    getinput:;  /* a simple goto can avoid recursion completely */
    extra = 0;
    printf ("Please enter a number from 1 to 9: ");
    /* prompt/loop while chars not between 1 and 9 */
    for (c = getchar(); (c < '1' || '9' < c ); c = getchar()) {
        if (c == '\n') {    /* no digits in input */
            fprintf (stderr, "error: invalid input.\n");
            goto getinput;
        }
        else if (c == EOF) {  /* trap EOF */
            fprintf (stderr, "\nerror: input canceled.\n");
            return c;
        }
    }
    /* empty input buffer -- increment extra count */
    for (tmp = getchar(); tmp != '\n' && tmp != EOF; tmp = getchar())
        extra++;

    if (extra) { /* if extra chars -- input too long */
        fprintf (stderr, "error: input too long.\n");
        goto getinput;
    }

    return c - '0';  /* return integer value instead of ASCII value */
}

(just a style note, C generally avoids the use of camelCase variable names in favor of lower-case, that's just a generality, it's entirely up to you)

You can check the function (and respond to a cancellation) with the following short bit of code:

#include <stdio.h>

int validateuserinput();

int main (void) {

    int n;

    if ((n = validateuserinput()) != EOF)
        printf ("\n valid input : %d\n", n);

    return 0;
}

Example Use/Output

Testing accepting input of only 1-9:

$ ./bin/inputhandler
Please enter a number from 1 to 9: Hello World!
error: invalid input.
Please enter a number from 1 to 9: ?
error: invalid input.
Please enter a number from 1 to 9: 0
error: invalid input.
Please enter a number from 1 to 9: 23
error: input too long.
Please enter a number from 1 to 9: 6

 valid input : 6

Testing input cancellation (e.g. ctrl+d, or ctrl+z on windows)

$ ./bin/inputhandler
Please enter a number from 1 to 9:
error: input canceled.

While there is nothing wrong with using recursion, it always helps to ask "Does this need to be a recursive function to begin with?" Sometimes the answer will be yes, but often there are simple ways to avoid the additional overhead. (note: the overhead with a few recursions is minimal so it isn't a big consideration in your case, but if you unwittingly call a recursive function that spins a million times, it rapidly becomes a concern)

Look over all answers, and if you have any questions, just let us know.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • I guess it would be better to use `for(;;)` loop instead of `getinput:` and replace `goto getinput` with `continue`. – j2ko Aug 31 '16 at 09:49
  • Yes, that's possible, but I'm not sure it necessarily improves anything. You basically move your `getchar()` above the existing `for`, turn the `for` into an `if` to which you wrap the bottom `extra` check in the `else` and wrap all of that in a `for(;;)` moving the `EOF` below the loop while also including an `EOF` check after the initial `getchar()`. You can reverse the `if` logic if you want your digit check first, but that kind of muddies the logic a bit. Either way will work. – David C. Rankin Aug 31 '16 at 21:48
  • `for(;;)` is an indication of infinite loop so reader will catch this from the beginning. So it will definitely improve readability. Other changes not needed. – j2ko Aug 31 '16 at 22:06
  • I guess I would just need to see an example of that. You still need to handle the `extra` loop above and empty the input buffer in the event of multiple digits inside the *infinite loop*. On each read you are going to need to check `EOF` or risk undefined behavior calling `getchar` again on a stream with an error condition set. What part am I over-thinking? Controlling when the prompt is displayed along with the desired error messages is another matter altogether. – David C. Rankin Aug 31 '16 at 23:05
0

You only set numValue once - for the first getchar() - looks like you expect it to be set after every getchar().

John3136
  • 28,809
  • 4
  • 51
  • 69
-1
 while((temp = getchar()) != '\n'){
        digitCounter++;
    }

could be

 while((temp = getchar()) != '\n'){
        if (temp < 49 || temp > 57 || ++digitCounter>0)
            return validateUserInput();
    }

This way makes it simple I think

Sush
  • 1,169
  • 1
  • 10
  • 26
  • Suppose the user types 567 and newline the first time around. This code would read the 5 outside the loop; it would read the 6 in your loop and make a recursive call to the function, which would then read 7 and a newline and would be happy with life. The user might be puzzled about why the number used was 7 when they typed 567, though. – Jonathan Leffler Aug 31 '16 at 03:48