Thread: How can I improve this code or make it shorter?

Hi everyone!
This is supposed to take a date in dd/mm/yyyy format from the keyboard and check whether its valid or not.It works for me but just wanted to make it a little shorter(If that's possible) and more readable.

Code:

#include<iostream>
using namespace std;
int checkyr(int);
int checkmon(int);
int checkdd(int,int,int);
int main()
{
    int dd=1,mm=1,yyyy=1970;
    cout<<endl<<"Enter the day:";
    cin>>dd;
    cout<<endl<<"Enter Month:";
    cin>>mm;
    cout<<endl<<"Enter year:";
    cin>>yyyy;
    if(!checkyr(yyyy))
    {
                  if(!checkmon(mm))
                  {
                    if(!checkdd(dd,mm,yyyy))
                                            cout<<endl<<"Date is valid";
                  }               
    }
    fflush(stdin);
    cin.get();
    return 0;
}

int checkyr(int yyyy)
{
    if(yyyy>0)
              return 0;
    else
              {
                     cout<<"\nInvalid Year";
                     return 1;
              }
}          
        
int checkmon(int mm)
{
    if((mm>=1)&&(mm<=12))
                         return 0;
    else
    {
        cout<<"\nInvalid Month";
        return 1;
    }
} 
                                             
int checkdd(int dd,int mm,int yyyy)
{
    int lp=0;
    if((yyyy%4==0 && yyyy%100!=0)||(yyyy%400==0))
                                                 lp=1;
    if((mm%2==1 && mm<=7)||(mm%2==0 && mm>=8))
                   {
                                               if((dd>=1)&&(dd<=31))
                                                                    return 0;
                   }
    if(mm==4||mm==6||mm==9||mm==11)
    {
                                   if((dd>=1)&&(dd<=30))
                                                        return 0;
    }
 
    if(mm==2)
    {
    switch(lp)
    {
              case 0:
                   if((dd>=1)&&(dd<=28))
                                        return 0;
                   
                    break;               
              case 1:
                   if((dd>=1)&&(dd<=29))
                                       return 0;
                   break;
    }
    }
    cout<<endl<<"Invalid Day";
    return 1;                    
 }

Thanks!

Here's my take.

Code:

#include <iostream>
#include <iomanip> 
using namespace std;

// Function Prototypes 
bool year_ok(int);
bool day_ok(int,int,int);

int main()
{
	int day = 1, month = 1, year = 1970 ;

	cout<<endl<<"Enter the day:";
	cin >> day ;
	cout << endl << "Enter Month:" ;
	cin >> month ;
	cout << endl << "Enter year:";
	cin >> year ;

	if ( year_ok(year) && day_ok(day, month, year) ) {
		cout << setfill('0') << "Date: " << 
		setw(2) << month << "/" << 
		setw(2) << day << "/" << 
		setw(4) << year << " is valid" << endl ; 
	}               
	else {
		cout << setfill('0') << "Date: " << 
		setw(2) << month << "/" << 
		setw(2) << day << "/" << 
		setw(4) << year << " is not valid" << endl ; 
	}
	
	cin.get();
	return 0;
}

bool year_ok(int yyyy) {
    if (yyyy >= 0 ) return true ; 
	cout << "Invalid Year" << endl ; 
    return false ; 
}          
        
bool day_ok(int day,int month,int year) {
	if (day < 1) { 
		cout << "Day is not valid" << endl ; 
		return false ; 
	} 
	switch(month) {  
		case 1:   // Jan  
		case 3:   // Mar 
		case 5:   // May 
		case 7:   // Jul 
		case 8:   // Aug 
		case 10:  // Oct 
		case 12:  // Dec 
			if (day <= 31) return true ; 
			break ; 
		case 4:   // Apr 
		case 6:   // Jun
		case 9:   // Sep 
		case 11:  // Nov 
			if (day <= 30 ) return true ; 
			break ; 
		case 2:   // Feb 
			if (day <= 28) return true ; 
			if ( day > 29) return false ; 
			// Handle Leap Year 
			if ( ( (year % 4 == 0) && (year % 100 != 0) ) || ( year % 400 == 0) ) return true ; 
			break ; 
		default:  
			cout <<  "Month is invalid." << endl ; 
			return false ;   // Month was not found 
		} 
	cout << "Day is invalid" << endl ; 
	return false ; 
}

Todd

Thanks for the feedback.

Thanks!

Originally Posted by laserlight

I think that in this case, it is better to just write a single isValidDate() function that takes the day, month and year. Instead of printing error messages, it could return true if the date is valid, or false if it is invalid. If more precision as to the type of error is needed, it could return an error code of some sort (e.g., an enum). Only in the main() function would error messages be printed.

On the other hand, printing errors at the lowest possible level ensures that they ALWAYS get printed even if upper level code forgot to check for errors. Although you need some way of turning them off.

Originally Posted by anon

Unless you reuse the function in a program that does not create a console window.

Like I said, you have to have a way of turning it off.

Originally Posted by laserlight

It seems to me that exceptions could be used in such a case,

Putting print statements in the constructors for your exceptions might not be a bad idea. Exceptions are safe in the sense that if they aren't caught, the program terminates abnormally, but I still like to see a specific message printed for every error condition even if the caller is too lazy to do so.

More than once, a customer has found a logic error by observing that the product printed some funny message which didn't correspond to any visible GUI status change. Somebody had forgotten to check an error condition somewhere. Such bugs would never be found (until truly bad things started happening) without those messages.

Even without a console, the messages can go into a log somewhere.

The very function is designed so that one can check for an error (it serves no other purpose), so to not check for an error code seems... irrational or highly careless.

In this specific case, yes

Originally Posted by anon

The idea is that you may want to inform the user in different ways in different places of the program, so the function shouldn't do anything more than its immediate task.

IMHO this is actually the argument FOR reporting errors at the lowest level. In principle, all error conditions should be reported to the user through the main interface. Printing errors on the console actually HELPS to make sure that's always true. If you see an error message on the console with no corresponding interface change you know that you've missed an error condition somewhere.

Combine this with the concept of a cancellable error message (the upper layer can cancel the printing of the lower layer's message and report the error some other way) and you have a decently robust method of making sure that the application actually checks error conditions.

Even with C++ exceptions a lazy programmer can put empty catch blocks.