CODEWITHSUNDEEP
cwhile-loop
user1421612
user1421612

Reputation:

Loop showing menu never ends

I'm trying to write a simple menu in C.

This is my proposed code:

while (end == 0) {

    printf("Main menu\n");
    printf("=========\n");
    printf("1) Option 1.\n");
    printf("2) Option 2.\n");
    printf("3) Option 3.\n");
    printf("4) Option 4.\n");
    printf("0) Exit");
    printf("\n");
    printf("Choose an option: ");
    fgets(&menu_option,1,stdin);

    switch (menu_option) {
        case '0': end = 1; break;
        case '1': option1(); break;
        case '2': option2(); break;
        case '3': option3(); break;
        case '4': option4();
    }

}

However, when executing this code, the loop never ends.

What's wrong?

Thank you.

EDIT: when I say "the loop never ends", I want to say that I can't write any option because the menu appears again and again.

Upvotes: 0

Views: 116

Answers (4)

hyde
hyde

Reputation: 62797

Since in the comments you expressed dislike for using strings, here's alternative implementation with minimal changes to the question code:

while (end == 0) {

    printf("...menu print snipped...\n");

    int ch = getchar();

    switch (ch) {
        case EOF: // end of file or error, fall through to exit option
        case '0': end = 1; break;
        case '1': option1(); break;
        case '2': option2(); break;
        case '3': option3(); break;
        case '4': option4();
        // default: // add error message? 
    }

    // ignore any extra chars in the same line
    while ((ch = getchar() != '\n') {
        // test for end of file and set end to true
        if (ch == EOF) {
            end = 1;
            break;
        }
    }
}

Note the extra loop, which will read and discard rest of the input line. Without it, user can enter several options on one line, which is probably not desirable. Also note, if the option functions read more user input, consider how you handle long lines then. For example move the extra loop to be before switch (and add another variable to read discarded chars). Actually you should probably put the extra loop to it's own function discard_until_newline.

Upvotes: 0

Olaf Dietsche
Olaf Dietsche

Reputation: 74028

From fgets

The fgets() function shall read bytes from stream into the array pointed to by s, until n-1 bytes are read, or a is read and transferred to s, or an end-of-file condition is encountered. The string is then terminated with a null byte.

Since you give a length of 1, n-1 will be 0 and so your input is never stored in menu_option. Change it to a char array 

char menu_option[10];
// ...
fgets(menu_option, sizeof(menu_option), stdin);

switch (menu_option[0]) {
    case '0': end = 1; break;
    case '1': option1(); break;
    case '2': option2(); break;
    case '3': option3(); break;
    case '4': option4();
}

Update:

It is necessary to use char arrays, because you need enough space to store the input. The minimum size for a buffer is two characters, e.g. char buf[2], because fgets stores your input plus a terminating NUL character.

The loop iterates twice or even more times, because when you input e.g. 1 return or 1 3 2 return, it will first return 1, then 3, 2 and finally return. If you want to read the whole line and use the first digit only, your buffer must be larger than just 2, like e.g. 10 or even 256.

Upvotes: 2

Klaas van Aarsen
Klaas van Aarsen

Reputation: 474

From: http://www.cplusplus.com/reference/cstdio/fgets/

fgets

char * fgets ( char * str, int num, FILE * stream );

Get string from stream.

Reads characters from stream and stores them as a C string into str until (num-1) characters have been read or either a newline or the end-of-file is reached, whichever happens first.

A newline character makes fgets stop reading, but it is considered a valid character by the function and included in the string copied to str.

A terminating null character is automatically appended after the characters copied to str.

Since you have specified 1 for the number, 0 bytes are read, and the only thing that happens is that a terminating null character is appended. The result is an infinite loop in which nothing is read.

Upvotes: 0

haccks
haccks

Reputation: 106012

fgets reads a string instead of character. The statement

fgets(&menu_option,1,stdin);

Will store a string to menu_option but to store a string you need a size at least 2 bytes. 

char menu_option[2];  
...
fgets(menu_option,2,stdin);   
int c;
while((c = getchar()) != '\n' && c != EOF);  // This will be needed to flush your input buffer 
switch (menu_option[0]) {
        ...
        ...
}

Upvotes: 0

Related Questions