continue and scanf

[creeps]

I'm trying to validate a day of the month, so that if the user enters a negative value or a value above 31, the program prints an error message and prompts for another day. The code below doesn't work as expected. Instead, it gets caught inside an infinite loop, printing something of the form Enter day and reminder: Invalid Date , and I can't seem to figure out why. The same technique worked like a charm previously.

while(1) {

        if(num_remind == MAX_REMIND) {

            printf("-- No Space Left --\n");
            break;
        }

        printf("Enter day and reminder: ");
        scanf("%2d", &day);
        if(day == 0) {

            break;
        } else if(day < 0 || day > 31) {

            printf("Invalid Date\n");
            continue;
        }

        sprintf(day_str, "%2d", day);

        read_line(msg_str, MSG_LENGTH);

        for(i = 0; i < num_remind; i++) {

           if(strcmp(day_str, reminders[i]) < 0) {

                break;
            }
        }

        for(j = num_remind; j < i; j--) {

            strcpy(reminders[j], reminders[j - 1]);
        }

        strcpy(reminders[i], day_str);
        strcpy(reminders[i], msg_str);

        num_remind++;
    }

[pheininger]

I'm trying to validate a day of the month, so that if the user enters a negative value or a value above 31, the program prints an error message and prompts for another day. The code below doesn't work as expected. Instead, it gets caught inside an infinite loop, printing something of the form Enter day and reminder: Invalid Date , and I can't seem to figure out why. The same technique worked like a charm previously.

while(1) {

        if(num_remind == MAX_REMIND) {

            printf("-- No Space Left --\n");
            break;
        }

        printf("Enter day and reminder: ");
        scanf("%2d", &day);
        if(day == 0) {

            break;
        } else if(day < 0 || day > 31) {

            printf("Invalid Date\n");
            continue;
        }

        sprintf(day_str, "%2d", day);

        read_line(msg_str, MSG_LENGTH);

        for(i = 0; i < num_remind; i++) {

           if(strcmp(day_str, reminders[i]) < 0) {

                break;
            }
        }

        for(j = num_remind; j < i; j--) {

            strcpy(reminders[j], reminders[j - 1]);
        }

        strcpy(reminders[i], day_str);
        strcpy(reminders[i], msg_str);

        num_remind++;
    }

1) Too many goto's (3 breaks and 1 continue). Are you sure you want continue? If you restructure this code to avoid goto's (break and continue), you will have probably solved your problem a long the way.

For example, the following code eliminates the continue.

/* Get a day between 0 and 31.  */
        do {
            printf("Enter day and reminder: ");
            scanf("%2d", &day);
            if(day < 0 || day > 31) {

                printf("Invalid Date\n");
            }
        } while (day < 0 || day > 31);

        /* For 0, we are done. */
        if(day == 0) {
            break;
        }

This probably does not eliminate your problem.

Simililarly, you could eliminate your first break by structuring the while loop correctly. In fact, the while loop is a good candidate for a for loop. That would take you down to two break's.
2) May not be enough of the code for good review comments.
3) To what value does num_remind get initialized before the loop?

[creeps]

Here's the complete code. I can't understand your suggestions in the context.

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

#define MAX_REMIND 50
#define MSG_LENGTH 60

int read_line(char str[], int n);

int main(void) {

    char reminders[MAX_REMIND][MSG_LENGTH + 3];
    char day_str[3], msg_str[MSG_LENGTH + 1];
    int day, i, j, num_remind = 0;

    while(1) {

        if(num_remind == MAX_REMIND) {

            printf("-- No Space Left --\n");
            break;
        }

        printf("Enter day and reminder: ");
        scanf("%2d", &day);
        if(day == 0) {

            break;
        } else if(day < 0 || day > 31) {

            printf("Invalid Date\n");
            continue;
        }

        sprintf(day_str, "%2d", day);

        read_line(msg_str, MSG_LENGTH);

        for(i = 0; i < num_remind; i++) {

           if(strcmp(day_str, reminders[i]) < 0) {

                break;
            }
        }

        for(j = num_remind; j < i; j--) {

            strcpy(reminders[j], reminders[j - 1]);
        }

        strcpy(reminders[i], day_str);
        strcpy(reminders[i], msg_str);

        num_remind++;
    }

    printf("\nDay Reminder\n");
    for(i = 0; i < num_remind; i++) {

        printf(" %s\n", reminders[i]);
    }

    return 0;
}

int read_line(char str[], int n) {

    int ch, i = 0;

    while((ch = getchar()) != '\n') {

        if(i < n) {

            str[i++] = ch;
        }
    }

    str[i] = '\0';

    return i;
}

[Adak]

Lots of problems with this code:

1) The logic is poor - an endless loop, then a break here, a break there, another break someplace else...

Is this logic for a program, or a Chinese fire drill? ;)

One break, maybe two, OK. Three? No. This is WAY too simple a loop for three breaks.

2) "%2d" is a good format specifier for printf(), but not for scanf(). Delete the 2 from it.

3) After each scanf() lines, add a getchar() to remove the newline char (caused by hitting the enter key), from the keyboard buffer.

That will stop your endless looping (along with #2).

Do while() loops work well for user input:

do {
  day = -1;
  printf("\n Enter Your Day's Number: ");
  scanf("%d", &day);
  getchar();
}while(day < 0);