Checking Loop in C - scan() is not running by the second run

Question

void openMenu(int *op) {//edited
    do {
        printf("Your turn...\t\n");
        scanf(" %d", op);
        if (*op > 14 || *op < 1 ) {
            printf("Only enter a number between 1 and 14!\n");
        }
    } while (*op > 14 || *op < 1 );
}

I am trying to make a checking loop which controls if the value which was entered is between 1 and 14. If there are letters it has to repeat too, the enter process.

Perhaps it isn't working and everytime it goes in the second run the scanf didn't run.

I checked the thing to set a space in front of the %d, but it isn't working too... Did u have maybe a nice idea?

Working with Xcode on Mac 11.1

Answer 1

You need to check the returning value of your scanf:

#include <stdio.h>

 void openMenu(int *op) {//edited
    do {
        printf("Your turn...\t\n");
        if (scanf(" %d", op) != 1 || *op > 14 || *op < 1 ) {
            while(getchar()!='\n'); // clean the input buffer
            printf("Only enter a number between 1 and 14!\n");
        }
    } while (*op > 14 || *op < 1 );
}       

int main()
{
    int op;
    openMenu(&op);
    printf("Number Read {%d}\n", op);
    return 0;
}

A more robust (and complicated) solution would be the following:

int isNumber(char buffer[]){
    for(int i = 0; buffer[i] != '\0'; i++)
        if(!isdigit((unsigned char) buffer[i]))
           return 0;
    return 1;
}

int readInput(char buffer[]){
    int result = scanf("%99s", buffer); 
    while(getchar()!='\n');
    return result;
}

int isInRange(int *op, char buffer[]){
    *op = atoi(buffer);
    return *op <= 14 && *op >= 1;
}
           
           
void openMenu(int *op) {
    do {
        char buffer[100];
        if(readInput(buffer) && isNumber(buffer) && isInRange(op, buffer)) {
           break;
        }
        printf("Only enter a number between 1 and 14!\n");
    } while (1);
}         

This would avoid that input such as 4odgjlda would be considered a valid number. Nevertheless, with the current approach inputs such as 4 odgjlda would still be considered as a valid input, since scanf would read the first word and not the entire line. For a more robust solution you should use fgets instead. One can see an example of such solution on the answer provided by Andreas Wenzel.

Answer 2

The problem is that if you enter something like "sdfokhs" the first time, then scanf will not be able to match any integer and will return 0. Because scanf did not consume this invalid input from the input stream, calling scanf a second time won't cause the user to be prompted for new input. Instead, scanf will attempt again to match an integer from the non-consumed input, and will fail again for the same reason as the fist time. This means you have an infinite loop.

Therefore, to fix this, you must consume the rest of the line before calling scanf again, for example like this:

while ( fgetc( stdin ) != '\n' ) ;

Or, if you want more robust error checking:

int c;

do
{
    c = fgetc( stdin );
    if ( c == EOF )
    {
        printf( "Unrecoverable error reading input!\n" );
        exit( EXIT_FAILURE );
    }
} while ( c != '\n' );

Also, it is always a good idea to check the return value of scanf.

However, in this case, I don't recommend using scanf. It would make more sense to always read exactly one line of input per loop iteration, using fgets. The disadvantage of using scanf is that it may read several lines of input per iteration, or only part of one line, which requires you to consume the rest of the line.

The following solution is longer than the solution of all other answers, but it is also the one with the most robust input validation and error handling.

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

#define MAX_LINESIZE 100

void openMenu(int *op)
{
    char buffer[MAX_LINESIZE];
    char *p;
    long converted;

//goto label
try_again:

    //prompt user for input
    printf( "Please enter number between 1 and 14: " );

    //read line of input into buffer
    if ( fgets( buffer, MAX_LINESIZE, stdin ) == NULL )
    {
        printf( "Unrecoverable error reading input!\n" );
        exit( EXIT_FAILURE );
    }

    //make sure that a full line was read and remember position of newline character
    p = strchr( buffer, '\n' );
    if ( p == NULL )
    {
        int c;
        printf( "Input was too long!\n" );

        //attempt to consume input until newline character found
        do
        {
            c = fgetc( stdin );
            if ( c == EOF )
            {
                printf( "Unrecoverable error reading input!\n" );
                exit( EXIT_FAILURE );
            }
        } while ( c != '\n' );

        goto try_again;
    }

    //remove newline character from string
    *p = '\0';

    //convert string to number
    converted = strtol( buffer, &p, 10 );

    //make sure conversion was successful
    if ( p == buffer )
    {
        printf( "Only enter a number!\n" );
        goto try_again;
    }

    //verify that remainder of line is whitespace
    while ( *p != '\0' )
    {
        if ( !isspace( (unsigned char)*p ) )
        {
            printf( "Only enter a number!\n" );
            goto try_again;
        }

        p++;
    }

    //verify that number was in the correct range
    if ( converted < 1 || converted > 14 )
    {
        printf( "Only enter a number between 1 and 14!\n" );
        goto try_again;
    }

    //since all tests were passed, write the value
    *op = converted;
}

Note that using goto should normally not be done, if a loop can be used just as well. However, in this case, I believe it is the cleanest solution.

Answer 3

as i can see you are comparing the *op (which i assume is a pointer). so, check if you have already assigned the value to the predefined variable or not. It somewhat should look like this.

int value = 0;
int *op = &value;
do {
printf("Your turn...\t\n");
scanf(" %d", op);
if (*op > 14 || *op < 1 ) {
    printf("Only enter a number between 1 and 14!\n");
    
}
}while (*op > 14 || *op < 1 );