0

I am new in this world of programming. I am learning programming at my school. My teacher recently ask the class to create a menu with limited selection that will end with a sentinel value. So here's my coding:

#include <stdio.h>

void menu(void);
void choice1();
void choice2();
void choice3();
char choice;

int main() {
    do {
        menu();
        if (choice =='1')
            choice1();
        else if (choice =='2')
            choice2();
        else if (choice =='3')
            choice3();
        else if (choice =='4')
            break;
        else
            printf("Invalid character.");
    } while (choice != '4');
    return 0;
}

void menu(void) {
    printf("\nMenu:\n\n");
    printf("1) Choice 1.\n");
    printf("2) Choice 2.\n");
    printf("3) Choice 3.\n");
    printf("Choose any of the above, or enter 4 to quit.\n\n");
    scanf("%c", &choice);
}

void choice1() {
    printf("\nChoice 1\n");
}

void choice2() {
    printf("\nChoice 2\n");
}

void choice3() {
    printf("\nChoice 3\n");
}

When I try to run it, by putting the number 1, 2, 3, the output came out but after that the function menu() and also the line "Invalid character." came out. As for as the other character, the menu() and the "Invalid character" came out twice. Number 4 does end the program. Is there any improvement that I can make to make sure the menu() and the line "Invalid character." does not come out unnecessarily?

4
  • 3
    Read documentation of scanf. So test its result (number of scanned items). Compile with all warning & debug info. Use a debugger. Avoid global variables (so have menu return the chosen value). Have every printf format string ending with \n. Commented Aug 28, 2016 at 6:40
  • 3
    Your menu() function should return the choice, not set a global variable: int menu(void) { char choice; …; return choice; } or thereabouts. Your scanf() should use " %c" where the blank before the %c is important and there should not be a blank after it. Commented Aug 28, 2016 at 6:51
  • regarding this line: scanf("%c", &choice);. The %c format specifier will not consume leading white space. suggest changing the format string to: " %c" <<-- notice the leading space Commented Aug 29, 2016 at 1:49
  • I have corrected my code according all of your guide and now its working!! Thank you all of you! :D Commented Aug 29, 2016 at 8:32

6 Answers 6

4

In line-buffered input, the newline character lingers in the buffer after you read a single character for 'choice' and hence you get Invalid character. unintentionally.

You are required clear the buffer after reading the choice

scanf("%c", &choice);
while(getchar()!='\n') 
/* Wasting the buffer
 * Hence ensuring that the character you enter
 * is indeed considered for 'choice'
 */
  ;; // Do nothing in particular

As a side note, your program looks like a typical use-case for the switch-case command and maybe your teacher expects you to use it.

Considering the scenario mentioned by @chqrlie in [ this ] comment, the workaround is to add after

scanf("%c", &choice);

the below lines

int c;
while ((c = getchar()) != EOF && c != '\n') 
    ;; //Wasting the buffer
Sign up to request clarification or add additional context in comments.

1 Comment

This solution is incorrect: it will loop forever if stdin does not end with a \n (for example: if it is the empty file). You must check for EOF: for (int c; (c = getchar()) != EOF && c != '\n';) continue;
2

The problem is simple: the terminal is line buffered: you must press enter for the input to become available to your program, the first scanf("%c", &choice) retrieves the character typed and the second call retrieves the linefeed ('\n') that was generated by the enter key.

There are multiple ways to avoid this problem. You can add a space in the scanf format before the %c: scanf(" %c", &choice); or you can read characters after the first until you get a '\n'.

Note that you must check the return value of scanf to avoid undefined behavior if the user enters an end of file. It is also advisable to avoid global variables: the function menu() should return the choice specified. Using a switch statement is also more idiomatic for this.

Here is a corrected version:

#include <stdio.h>

int menu(void);
void choice1(void);
void choice2(void);
void choice3(void);

int main(void) {
    int ch;
    for (;;) {
        switch (ch = menu()) {
        case '1':  choice1();  continue;
        case '2':  choice2();  continue;
        case '3':  choice3();  continue;
        case '4':
        case EOF:  break;
        default:   printf("Invalid character %c\n", ch);  continue;
        }
        break;
    }
    return 0;
}

int menu(void) {
    char choice;

    printf("\nMenu:\n\n");
    printf("1) Choice 1.\n");
    printf("2) Choice 2.\n");
    printf("3) Choice 3.\n");
    printf("Choose any of the above, or enter 4 to quit.\n\n");
    if (scanf(" %c", &choice) == 1)
        return choice;
    else
        return EOF;
}

void choice1(void) {
    printf("\nChoice 1\n");
}

void choice2(void) {
    printf("\nChoice 2\n");
}

void choice3(void) {
    printf("\nChoice 3\n");
}

Comments

2

As already mentioned in other answers the problem is the newline character.

When you press 1 followed by enter, you'll get two chars, i.e. 1 and \n. So your loop runs twice and prints Invalid character when \n is processed.

Here is an alternative solution for your problem. Just add a space before %c.

scanf(" %c", &choice);

This works because the space will match any number of white-space characters and thereby match the \n and remove it.

From the man page:

A sequence of white-space characters (space, tab, newline, etc.......). This directive matches any amount of white space, including none, in the input.

Additional comments

You should always check the value returned by scanf to make sure you read the correct number of values.

 if (scanf(" %c", &choice) != 1)
 {
     // Add error handling ....
     // For instance you could terminate the program like
     exit(1);
 } 

In your program choice is a global variable. In general global variables should be avoid if possible. In your case you could make choice a local variable in main and let menu return a char. Like:

// char choice;  Remove global variable

int main() {
    char choice; // Add local variable
    do {
        choice = menu();  // Assign to local variable
        .....
}

char menu(void) {    // Make the function return a char
    char choice;     // Add local variable

    printf("\nMenu:\n\n");
    printf("1) Choice 1.\n");
    printf("2) Choice 2.\n");
    printf("3) Choice 3.\n");
    printf("Choose any of the above, or enter 4 to quit.\n\n");
    if (scanf("%c", &choice) != 1) exit(1); // Assign to local variable
                                            // On failure -> exit

    return choice;  // Return value of local variable
}

5 Comments

You should also warn the user against undefined behavior if the file is empty or does not contain a non whitespace character: scanf() will return 0 or EOF and choice will be uninitialized, leading to undefined behavior.
@chqrlie - True. Update. I'm not sure what you mean by ... or does not contain a non whitespace character, though. The space also covers none.
Sorry, I should have avoided the double negative. I meant if the file is empty or contains only whitespace characters, then scanf(" %c", &choice) returns EOF or 0.
@chqrlie - okay, I see what you mean. However, on my system scanf doesn't return when my input is just whitespaces (including newline). I can get the EOF using ctrl-d but I'm not sure how to get the 0.
You are correct, for this particular format string, it can only return 1 or EOF: C11 7.21.6.2p16 The fscanf function The fscanf function returns the value of the macro EOF if an input failure occurs before the first conversion (if any) has completed.
1

You can write scanf(" %c",&choice); (with whitespace) instead of scanf("%c",&choice);

Comments

1

When I try to run it, by putting the number 1, 2, 3, the output came out but after that the function menu() and also the line "Invalid character." came out.

It's happening because of the new line character you press after each number. It's itself a character and loop is iterated one more time for this. As it is an invalid character, that's why "Invalid character." is showing.

Try using getchar() after scanf().

Comments

0

Edit: fixed my previous while loop which may exit incorrectedly:

#include <stdio.h>
int main() {
  int choice;
  printf("Menu:\n\n");
  printf("1) Choice 1.\n");
  printf("2) Choice 2.\n");
  printf("2) Choice 3.\n");
  printf("Choose any of the above, or enter 4 to quit.\n\n");
  while (1) {
    char c = scanf("%d",&choice);
    if (c == EOF || choice == 4) break;
    if (choice == 1 || choice == 2 || choice == 3) {
      printf("Choice %d.\n", choice);
    } else {
      printf("Invalid character.\n");
    }
  }
  return 0;
}

You can use function if you want, but not necessary in this case. You need to understand how many times your loop actually runs and compare it to that you expect.

My previous code:

#include <stdio.h>
int main() {
  int choice;
  printf("Menu:\n\n");
  printf("1) Choice 1.\n");
  printf("2) Choice 2.\n");
  printf("2) Choice 3.\n");
  printf("Choose any of the above, or enter 4 to quit.\n\n");
  while (scanf("%d", &choice) && choice != 4) {
    if (choice == 1 || choice == 2 || choice == 3) {
      printf("Choice %d.\n", choice);
    } else {
      printf("Invalid character.\n");
    }
  }
  return 0;
}

1 Comment

if stdin is an empty file, scanf will keep returning EOF and choice will be uninitialized, leading to undefined behavior. Check for scanf("%d", &choice) == 1. Also scanf will return 0 if the input is not a number: the diagnostic Invalid character will not be printed in the case and the program will be exited incorrectly.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.