Stuck on this, see if you can see the problem

[iAmSparta]

I am working on a project and I admit its an assingment but I have looked at it for a week and I am going crazy. I believe the problem is in the IsValidDay function but that is just a guess. I cannot seem to get any date to be valid.
I have to finish the program so that is validates any date put in weather its valid based on day, month, and year including leap. Any suggestions would be greatly appreciated. I cannot seem to find any help by the web.

Here is the code (it is not long)

#include <iostream>
#include <string>


bool IsValidYear( int );
bool IsValidMonth( int );
bool IsValidDay( int,int,int );
bool IsLeapYear( int );

using namespace std ;

int main()
{
	
	int year = 0 ;
	int month = 0 ;
	int day = 0 ;

	bool inputIsValid = false;

	do
	{
		cout << "\nEnter a year: " ;
		cin >> year ;

		inputIsValid = IsValidYear(year);

		if ( !inputIsValid)
			cout << "\n\n" << year << " is not a valid year. Try again.\n";

	} while (!inputIsValid);

	do
	{
		cout << "\nEnter a month in numbers: " ;
		cin >> month ;

		inputIsValid = IsValidMonth(month);

		if ( !inputIsValid)
			cout << "\n\n" << month << " is not a valid month. Try again.\n";

	} while (!inputIsValid);
	
	do
	{
		cout << "\nEnter a day in munbers: " ;
		cin >> day ;

		inputIsValid = IsValidDay(year,month,day);

		if ( !inputIsValid)
			cout << "\n\n" << day << " is not a valid day. Try again.\n";

	} while (!inputIsValid);
	
	
	if (inputIsValid)
	{
		cout << "\n\n" << month << "/" << day << "/" << year << " is a valid date.\n\n";
	}
	return 0 ;

}



bool IsValidYear( int year )
{
	if ( year >= 0 ) return true;

	else return false;
}

bool IsValidMonth( int month)
{
	if ( month > 0 && month <= 12) return true;

	else return false;
}

bool IsValidDay( int year, int month, int day)
{

	
	if ((month==1 || month ==3 || month == 5 || month == 7 || month== 8 || month== 10 || month==12) && (day >= 1 && day<=31))
		
		return true;
	else
		return false;
		

	if ((month==4 || month ==6 || month == 9 || month == 11)
			&& (day >= 1 && day<=30))
		return true;
	else
		return false;
		

	if ((month==2 && day == 29 && (year%4==0)) && ((year%100!=0)||(year%400==0)))
		return true;
	else if (month ==2 && day > 0 && day <= 28)
		return true;

	else 
		return false;
	

 

}


			

bool IsLeapYear( int year)
{
	if (((year%4==0)&&(year%100!=0))||(year%400==0)) 
		return true;

	else 
		return false;
	
}

Thanks in advance for any help.

Mike

[m4ster r0shi]

bool IsValidDay( int year, int month, int day)
{

	
	if ((month==1 || month ==3 || month == 5 || month == 7 || month== 8 || month== 10 || month==12) && (day >= 1 && day<=31))
		
		return true;
	//else
	//	return false;
		

	else if ((month==4 || month ==6 || month == 9 || month == 11)
			&& (day >= 1 && day<=30))
		return true;
	//else
	//	return false;
		

	else if ((month==2 && day == 29 && (year%4==0)) && ((year%100!=0)||(year%400==0)))
		return true;
	else if (month ==2 && day > 0 && day <= 28)
		return true;

	//else 
	//	return false;
	

       return false;

}

Nice nickname! :P

[iAmSparta]

Thanks m4ster r0shi,

works perfect now,
cant believe I didnt see that.

thanks for the name comment as well
300 rocks

[Disch]

I know this is already solved, I just wanted to chime in with a style thing...

These are largly personal preferences... but maybe they'll give you some insight:

1) if statements that are bogged down with tons of || and && are very hard to follow. Try to avoid large chains of those

2) You can use your functions in other functions. Specifically, you already wrote code to check for a leap year in your IsLeapYear function. No need to duplicate that code in IsValidDay... just call the function you already wrote.

3) when you have situations where you have several items that each need to be treated differently, you can simplify it by finding a way to treat them all the same way. This can often be accomplished by using a switch and assigning different things for each case, or by using a look up table. For example... since months all have a different number of days, and there's no clear pattern to them, you can make a lookup table or a switch to find the number of days, and then use the number of days as a single value.

I'd recommend something like this:

bool IsValidDay( int year, int month, int day)
{
    // this is an example of a look up table
    static const int dayspermonth[] = {31,29,31,30,31,30,31,31,30,31,30,31};

    // make sure the month and year are
    if(!IsValidMonth(month))    return false;
    if(!IsValidYear(year))      return false;
    
    // month and year are OK, make sure the day is valid
    if(day < 1)                         return false;
    if(day > dayspermonth[month-1])     return false;
    
    // special case for leap year
    if( (month == 2) && (day == 29) )   return IsLeapYear(year); // Feb 29 is only valid on leap years
    
    // everything checks out
    return true;
}

See how much simpler and cleaner it is?

Alternatively, here's the switch approach:

bool IsValidDay( int year, int month, int day)
{
    int daysinthismonth;
    
    // find out how many days are in this month
    switch(month)
    {
    case 1: case 3: case 5: case 7:
    case 8: case 10: case 12:
        daysinthismonth = 31;
        break;
        
    case 4: case 6: case 9: case 11:
        daysinthismonth = 30;
        break;
        
    case 2:
        daysinthismonth = IsLeapYear(year) ? 29 : 28;
        break;
        
    default:
        return false; // invalid month
    }
    // make sure the year is valid
    if(!IsValidYear(year))      return false;

    // make sure date is valid
    if(day < 1)                 return false;
    if(day > daysinthismonth)   return false;
    
    // otherwise everything checks out
    return true;
}

Not quite as clean... granted. But it's useful in other situations.

EDIT: I just saw you had IsValidYear and IsValidMonth functions. I changed the above to use those functions.