Search code examples
cvalidationinputwhile-loopscanf

proper use of scanf in a while loop to validate input


I made this code:

/*here is the main function*/
int x , y=0, returned_value;
int *p = &x;
while (y<5){
    printf("Please Insert X value\n");
    returned_value = scanf ("%d" , p);
    validate_input(returned_value, p);
    y++;
}

the function:

void validate_input(int returned_value, int *p){
    getchar();
    while (returned_value!=1){
        printf("invalid input, Insert Integers Only\n");
        getchar();
        returned_value = scanf("%d", p);
    }
}

Although it is generally working very well but when I insert for example "1f1" , it accepts the "1" and does not report any error and when insert "f1f1f" it reads it twice and ruins the second read/scan and so on (i.e. first read print out "invalid input, Insert Integers Only" and instead for waiting again to re-read first read from the user, it continues to the second read and prints out again "invalid input, Insert Integers Only" again...

It needs a final touch and I read many answers but could not find it.


Solution

  • If you don't want to accept 1f1 as valid input then scanf is the wrong function to use as scanf returns as soon as it finds a match.

    Instead read the whole line and then check if it only contains digits. After that you can call sscanf

    Something like:

    #include <stdio.h>
    
    int validateLine(char* line)
    {
        int ret=0;
        
        // Allow negative numbers
        if (*line && *line == '-') line++;
        
        // Check that remaining chars are digits
        while (*line && *line != '\n')
        {
            if (!isdigit(*line)) return 0; // Illegal char found
    
            ret = 1;  // Remember that at least one legal digit was found
            ++line;
        }
        return ret;
    }
    
    int main(void) {
        char line[256];
        int i;
    
        int x , y=0;
        while (y<5)
        {
            printf("Please Insert X value\n");
            if (fgets(line, sizeof(line), stdin)) // Read the whole line
            {
                if (validateLine(line))  // Check that the line is a valid number
                {
                    // Now it should be safe to call sscanf - it shouldn't fail
                    // but check the return value in any case
                    if (1 != sscanf(line, "%d", &x)) 
                    {
                        printf("should never happen");
                        exit(1);
                    }
    
                    // Legal number found - break out of the "while (y<5)" loop
                    break;
                }
                else
                {
                    printf("Illegal input %s", line);
                }
            }
            y++;
        }
        
        if (y<5)
            printf("x=%d\n", x);
        else
            printf("no more retries\n");
    
        return 0;
    }
    

    Input

    1f1
    f1f1
    
    -3
    

    Output

    Please Insert X value
    Illegal input 1f1
    Please Insert X value
    Illegal input f1f1
    Please Insert X value
    Illegal input 
    Please Insert X value
    x=-3
    

    Another approach - avoid scanf

    You could let your function calculate the number and thereby bypass scanf completely. It could look like:

    #include <stdio.h>
    
    int line2Int(char* line, int* x)
    {
        int negative = 0;
        int ret=0;
        int temp = 0;
        
        if (*line && *line == '-') 
        {
            line++;
            negative = 1;
        }
        else if (*line && *line == '+')  // If a + is to be accepted
            line++;                      // If a + is to be accepted
           
        while (*line && *line != '\n')
        {
            if (!isdigit(*line)) return 0; // Illegal char found
            ret = 1;
    
                // Update the number
            temp = 10 * temp;
            temp = temp + (*line - '0');
    
            ++line;
        }
        
        if (ret)
        {
            if (negative) temp = -temp;
            *x = temp;
        }
        return ret;
    }
    
    int main(void) {
        char line[256];
        int i;
    
        int x , y=0;
        while (y<5)
        {
            printf("Please Insert X value\n");
            if (fgets(line, sizeof(line), stdin)) 
            {
                if (line2Int(line, &x)) break;  // Legal number - break out
    
                printf("Illegal input %s", line);
            }
            y++;
        }
        
        if (y<5)
            printf("x=%d\n", x);
        else
            printf("no more retries\n");
    
        return 0;
    }