How can I improve this code or make it shorter?

This is a discussion on How can I improve this code or make it shorter? within the C++ Programming forums, part of the General Programming Boards category; Hi everyone! This is supposed to take a date in dd/mm/yyyy format from the keyboard and check whether its valid ...

  1. #1
    Super unModrator
    Join Date
    Dec 2007
    Posts
    321

    Smile 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!

  2. #2
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,428
    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&#37;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;                    
    }
    Try learning how to indent properly.
    fflush(stidin) is undefined; read FAQ.
    Name your variables appropriately. yyyy, mm, dd is not exactly the best names.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  3. #3
    The larch
    Join Date
    May 2006
    Posts
    3,573
    You might try using a lookup table for lengths of months:

    Code:
    int month_lengths[12] = {31, 28, 31, 30 ... };
    
    if (day >= 1 && day <= month_lengths[month-1])
    Except if the month is February, you'll need to check for leap-year and accept 29 days as well.

    (This code assumes that month is already validated. You may put an assert in there to be sure that the function is used as it is meant to be.)

    In addition it feels strange that you return false if something is valid (and don't use bool instead of int), and the function names could have less ambigous names: is_valid_day, is_valid_month, is_valid_year.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  4. #4
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    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

  5. #5
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,428
    Here's my take:
    Code:
    bool year_ok(int year);
    bool day_ok(int day, int month, int year);
    Omitting the names for the variables in the prototypes is just bad practice IMO. It screws up intellisense (no names for the variables!) and it makes it harder to read what functions are available.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  6. #6
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    Thanks for the feedback.

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,414
    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.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    Super unModrator
    Join Date
    Dec 2007
    Posts
    321
    Thanks!

  9. #9
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,235
    Quote Originally Posted by laserlight View Post
    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.

  10. #10
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Quote Originally Posted by brewbuck View Post
    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.
    Unless you reuse the function in a program that does not create a console window.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  11. #11
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,414
    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.
    It seems to me that exceptions could be used in such a case, though an isValidDate() function that throws an exception if the date is not valid seems rather strange, since I would consider checking an invalid date to be a normal course of events. 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.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  12. #12
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,235
    Quote Originally Posted by anon View Post
    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.

  13. #13
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,235
    Quote Originally Posted by laserlight View Post
    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

  14. #14
    The larch
    Join Date
    May 2006
    Posts
    3,573
    They will be "turned off" if there is no console window, so you can't say that the error is reported even if you don't check return values.

    But laserlight has a good point that this function is only called because you are interested in the return value in the first place.

    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.

    More than once, a customer has found a logic error by observing that the product printed some funny message
    I'm not a professional but wouldn't that be a sign of a very low quality product?

    If you failed to check the return value for a function that validates dates, the testers should discover that the program doesn't really validate dates before the product is launched?
    Last edited by anon; 01-13-2008 at 09:22 AM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  15. #15
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,235
    Quote Originally Posted by anon View Post
    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.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Can I improve my code?
    By advancedk in forum C Programming
    Replies: 1
    Last Post: 07-27-2008, 07:47 AM
  2. can someone double check and maybe improve my code
    By tommy69 in forum C Programming
    Replies: 23
    Last Post: 04-21-2004, 02:04 PM
  3. How to improve code
    By rugby in forum C Programming
    Replies: 3
    Last Post: 04-15-2003, 09:24 AM
  4. << !! Posting Code? Read this First !! >>
    By kermi3 in forum C Programming
    Replies: 0
    Last Post: 10-03-2002, 03:04 PM
  5. 20 Tips to Improve Your Code
    By Aran in forum C++ Programming
    Replies: 7
    Last Post: 08-16-2002, 05:42 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21