Why does this month comparison code not work properly?

Question

I've been given homework and am almost done but this strip of code in a C program is preventing me from completion:

if ((month==1,3,5,7,8,10,12 && day>31)){
        printf("error only 31 days in that month");
        return 0;
    }
   else if (month==4,6,9,11 && day>30){
        printf("error only 30 days in that month");
        return 0;
      }
    else if (month==2 && leapyear && day>29){
        printf("error only 29 days in that month");
        return 0;
        }
        else if (month==2 && day>28){
            printf("error only 28 days in that month");
            return 0;
        }

I wanted to say that if the month was 1 and the input was greater than 31 then it would do an error and stop and that is the problem. If I input 4 in months and the day is 34, it will print the first statement. Is there anything I can do? Also, bear with me; I'm still kinda new to programming.

The notation `if ((month==1,3,5,7,8,10,12 && day>31)){` does not do what you want it to do. It evaluates `if (12 && day > 31)` which is equivalent to `if (day > 31)`. You have to write out the comparisons with each number, separated by `||` operators and surrounded by parentheses. Or use a `switch` statement. — Jonathan Leffler, Nov 18 '20 at 22:14

score 1 · Answer 1 · answered Nov 18 '20 at 22:14

This doesn't do as you expect:

if ((month==1,3,5,7,8,10,12 && day>31)){

It does not check if month is one of 1, 3, 5, etc. It's an instance of the comma operator where the left operand is evaluated and discarded and the right operand becomes the value of the expression.

You need to explicitly check month against each possible value:

if ((month==1 || month==3 || month==5 || 
     month==7 || month==8 || month==10 || month==12) && day>31) {

LSerni · Accepted Answer · 2020-11-19T12:39:25.860

In cases like these, you have a simple map where an integer in a not too long, contiguous sequence goes in (the month) and a number comes out (the number of days).

You can do this with several IF's, or using the OR syntax as has been suggested.

if ((month == 1) || (month == 3) || ... )

The above would be the best way if you had very few group of very few items (especially if noncontiguous) and very few possible outputs. The second condition is true (you only have 28/29, 30 and 31), the first... just barely, in my opinion.

In general, I find it safer to do this with a vector:

//        Vectors start at #0
int days[] = { 0,             31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
// Correct data later
if (leapyear) {
    days[2]++;
}
if (day > days[month]) {
    printf("Month %d has only %d days\n", month, days[month]);
}

If you had a fixed number of noncontiguous items, you could do this with two vectors. This works with any grouping, unless you have really many keys or can come up with an easy rule (of course, if you wanted to check which day of the week a random even day was, you would not enter all 182 or 183 even-numbered days; you'd check it was even and use modulo 7).

// I check all months except in summer.
// Keys holds the "question" (which month?), values the answer.
int keys   = {  1,  2,  3,  4,  5 ,       9, 10, 11, 12 };
int values = { 31, 28, 31, 30, 31,       30, 31, 30, 12 };
int i;
// I use 0 to indicate that the check does not apply (e.g. August)
days = 0;
for (i = 0; i < 12; i++) {
    if (month == keys[i]) {
        days = values[i];
    }
}
if (days != 0) {
    if (day > days) {
        printf("Month %d has only %d days\n", month, days);
    }
}

A fancier way of doing this (for months only) could be the knuckle mnemonic

//            1st hand    knuckle    february   leap
int days = 31-((month<8)^(month%2))-(2==month)*(month-leapyear);
if (day > days) {
    printf("Month %d has only %d days\n", month, days);
}

Why does this month comparison code not work properly?

2 Answers2