help me check why my 2/30 still valid==? which statment wrong

[tang]

#include <iostream>
#include <conio.h>
#include <string>
using namespace std;

string date ( int year , int month , int day )
{
if(year<1990||year>2009)
return "Invalid";
else if (month<1 || month>12)
return "Invalid";
else if ( month==4,6,9,11 & day>30)
return "Invalid";
else if ( month==2 & year%4==0 & day>29)
return "Invalid";
else if ( month==2 & year%4==!0 & day>28)
return "Invalid";
else
return "Valid";
}

int main (){
int y,m,d;
cout<<"Enter Year : ";cin>>y;
cout<<endl;
cout<<"Enter Month : ";cin>>m;
cout<<endl;
cout<<"Enter Day : ";cin>>d;
cout<<"The Date "<<d<<" " <<m<<" "<<y<<" is "<<date(y,m,d);
getch();
}

[Disch]

Problem number 1:

else if ( month==4,6,9,11 & day>30)

The comma operator does not do what you expect here. You probably wanted to do this:

else if( month == 4 || month == 6 || month == 9 .... etc)

Be sure to watch out for priority issues because you have a && on the same line. Maybe put all your ||s in parenthesis:

else if( day > 30 && (month == 4 || month == 6 || ... ) )

Or better yet... split this up so you don't have such a massive if statement. Giant if statements are confusing, hard to read, and easy to make mistakes in. See end of post for alternative ideas.

Problem #2:
You are using & instead of &&. You want to use && here.


Problem #3:
year%4==!0

This does not do what you expect. !0 is the same as 1. So this is the same as
year%4 == 1 which is not what you intended.

You want to use the != operator here: year%4 != 0



Alternative ideas which might make this easier / less error prone:

1) Maybe split this up into smaller functions. Rather than trying to do everything in a single date() function... write new functions to handle some of the smaller tasks. Like maybe a daysInMonth() function.

Think how much easier it'd be to do this:

if(day > daysInMonth(month))  return "Invalid";

2) Maybe use a lookup table to determine the days in the month. A lookup table is just an array that has fixed data that you can use to make things easier. Something like:

int daysinmonths[12] = {31,28,31,30,31,30, ...};

int daysinthismonth = daysinmonths[ month-1 ];

if(day > daysinthismonth)  return "Invalid";

Of course this doesn't account for leap years, but you can work that in. LUTs are much simpler than massive and confusing if statements.

[MiiNiPaa]

Also I must note that leap years are not deretmined by simple (year % 4 == 0)
For example year 1900 is not leap, but 2000 is.
Correct formula: bool is_leap = (year % 4 == 0) && ( (year % 100 != 0) || (year % 400 == 0) )