7
\$\begingroup\$

I'm reading a book about C programming, at the end of each chapter it has some training exercise and one of them was to make a program that tells if a date is valid or not, the code below is what I did and I would like to know if it can be improved?

#include <stdio.h>

int main(){
int dd,mm,yy;

printf("write a date(day/month/year):");
scanf("%d/%d/%d",&dd,&mm,&yy);

int bissextile=(yy%4==0)?1:0;

if (dd>31 || dd<1 || mm>12 || mm<1)
    printf("Invalid date.\n");
    else
        if((mm==2 && dd<30 && bissextile) || (mm==2 && dd<29 && bissextile==0))
            printf("valid date.\n");
            else
                if((mm==4 || mm==6 || mm==9 || mm==11)&& dd<31)
                    printf("valid date.\n");
                    else
                        if(dd<31 && mm!=2)
                            printf("valid date.\n");
                            else
                                printf("Invalid date.\n");
}
\$\endgroup\$
  • 1
    \$\begingroup\$ your code could be simplified by using a built in function on the input values e.g. mktime() returns -1 when it fails. \$\endgroup\$ – AndersK Sep 15 '15 at 5:18
  • 2
    \$\begingroup\$ Careful with leap year/century calcs. The first leap century was 1600, but they can be calculated for earlier centuries. But if they are, then the dates from 1582-10-05 through 1582-10-14 are invalid dates. IOW, truly functional/correct date validation is complex. Don't get over concerned with 'perfect'. \$\endgroup\$ – user2338816 Sep 15 '15 at 6:04
  • \$\begingroup\$ @CyberSpock mktime() does not validate dates. It readily accepts values outside the primary range. "...the original values of the other components are not restricted to the ranges indicated above ..." \$\endgroup\$ – chux - Reinstate Monica Sep 15 '15 at 21:36
  • 1
    \$\begingroup\$ Dates present a major set of problems if you try to treat historical dates accurately. In Sweden, there was a 30th February 1712; there were a bunch of mix-ups that lead to that state of affairs. Different countries switched between the Julian and Gregorian calendars at different times. Many Roman Catholic countries in Europe switched in 1582; Britain and its colonies switched in 1752; Russia didn't switch until the 20th century (which is why the October Revolution occurred in November). Don't forget that there wasn't a year 0; 1 AD (CE) was preceded by 1 BC (BCE). So much fun! \$\endgroup\$ – Jonathan Leffler Jan 3 '17 at 3:37
7
\$\begingroup\$

Writing a useful function should be the first thing that comes to mind when you have a challenge like this. Functions can be reused, and they can make the logic simpler.

In addition, a return value is easy to apply in a function, and you can return whenever the function value is determined.

In this case, a function like valid_date which returns a conditional-friendly value (int 1 for OK, 0 for not OK).

This function would check for invalid conditions, and return 0 when an invalid condition is found. If all checks pass, though, it returns 1.

Your leap-year check is not accurate though. Your code is:

(yy%4==0)?1:0;

A year is a leap year when:

  1. it is a multiple of 4
  2. but not if it is also a multiple of 100
  3. unless it is also a multiple of 400.

A more accurate check is:

yy % 400 == 0 || (yy % 4 == 0 && yy % 100 != 0)

Putting this advice together, I put up the following method:

int valid_date(int dd, int mm, int yy) {
    if (mm < 1 || mm > 12) {
        return 0;
    }
    if (dd < 1) {
        return 0;
    }

    int days = 31;
    if (mm == 2) {
        days = 28;
        if (yy % 400 == 0 || (yy % 4 == 0 && yy % 100 != 0)) {
            days = 29;
        }
    } else if (mm == 4 || mm == 6 || mm == 9 || mm == 11) {
        days = 30;
    }

    if (dd > days) {
        return 0;
    }
    return 1;
}

Note how I calculate the expected limit for days and then had just one check? That makes the code simpler to read too.

Finally, that can be used like:

int main(){
    int dd,mm,yy;

    printf("write a date(day/month/year):");
    scanf("%d/%d/%d",&dd,&mm,&yy);

    printf("%s Date\n", valid_date(dd, mm, yy) ? "Valid" : "Invalid");
}
\$\endgroup\$
  • \$\begingroup\$ Although I haven't reached the functions chapter(Which I can't wait to get there) I understood everything, it really makes the logic simpler as you said and I learned new things about a leap year, thank you a lot :) \$\endgroup\$ – n00b Sep 15 '15 at 13:39
9
\$\begingroup\$

Validation is a specific task, distinct from receiving input, so it deserves to have its own function.

The code suffers from excessive nesting, which could be improved by putting else if at the same indentation level. However, another problem is that some of the checks are affirmative and some of them are negative, which makes the code harder to follow.

Since the validation depends mostly on the month, I'd write it this way, with a switch:

int isValid(int dd, int mm, int yy) {
    if (dd <= 0) return 0;
    switch (mm) {
      case 1: case 3: case 5: case 7:
      case 8: case 10: case 12:
        return dd <= 31;
      case 4: case 6: case 9: case 11:
        return dd <= 30;
      case 2:
        return dd <= 28 + (yy % 4 == 0);
      default:
        return 0;  /* invalid month */
    }
}

Then, in main():

if ( 3 == scanf("%d/%d/%d",&dd,&mm,&yy) &&
     isValid(dd, mm, yy) ) {
    puts("Valid date");
} else {
    puts("Invalid date");
}

Note that validating the return value from scanf() is important too.

The code doesn't handle the Gregorian correction properly, such as for year 1900.

\$\endgroup\$
  • \$\begingroup\$ hi thank you for your improvements suggestions. I didn't accepted your answer because it doesn't handle the Gregorian correction, but doesn't make it a bad answer, it's actually a good one, I never thought about using the switch statement(I should use it more frequently) \$\endgroup\$ – n00b Sep 15 '15 at 13:47
  • 2
    \$\begingroup\$ @Alex_n00b Although this answer does not handle certain Gregorian years, the selected answer does not handle dates well before 1582. 2 issues:1) the post did not specify year range and 2) user2338816 is correct: date validation is complex. Don't get over concerned with 'perfect', especially when the post lacks specificity: "bissextile" was used before Gregorian Calendar, nuances like February 30th, etc. \$\endgroup\$ – chux - Reinstate Monica Sep 15 '15 at 21:50
  • \$\begingroup\$ @chux I see that leap years are very complex, still a lot to read about it. thanks \$\endgroup\$ – n00b Sep 15 '15 at 23:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.