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

  1. #16
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    Quote Originally Posted by anon View Post
    I'm not a professional but wouldn't that be a sign of a very low quality product?
    I said more than once, but I don't mean "thousands of times" either. It's a QA tool. Most customers don't even run the product in a mode where they could see these messages. The more technically knowledgable ones figure out how to turn them on.

    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?
    Assuming the QA testers know what the hell they're doing

  2. #17
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Chappell Hill, Texas
    Posts
    2,332
    I like the try/catch/throw approach better.

    Code:
    #include <iostream>
    #include <iomanip> 
    using namespace std;
    
    // Function Prototypes 
    void year_ok(int year);
    void day_ok(int day, int month, int year);
    
    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 ;
    	try { 
    		year_ok(year) ; 
    		day_ok(day, month, year) ; 
    		cout << setfill('0') << "Date: " << 
    		setw(2) << month << "/" << 
    		setw(2) << day << "/" << 
    		setw(4) << year << " is valid" << endl ; 
    	}   
    	catch (...) {             
    		cout << setfill('0') << "Date: " << 
    		setw(2) << month << "/" << 
    		setw(2) << day << "/" << 
    		setw(4) << year << " is not valid" << endl ; 
    	}
    	
    	cin.get();
    	return 0;
    }
    
    void year_ok(int yyyy) {
        if (yyyy < 0 ) throw "Invalid Year" ;  
    }          
            
    void day_ok(int day,int month,int year) {
    	if (day > 0) { 
    		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 ; 
    				break ; 
    			case 4:   // Apr 
    			case 6:   // Jun
    			case 9:   // Sep 
    			case 11:  // Nov 
    				if (day <= 30 ) return ; 
    				break ; 
    			case 2:   // Feb 
    				if (day <= 28) return ; 
    				if ( day == 29) { 
    					if ( ( (year &#37; 4 == 0) && (year % 100 != 0) ) || ( year % 400 == 0) ) return ; 
    				break ; 
    			default:  
    				throw "Month is invalid." ; 
    			}
    		}
    	}
    	throw "Day is invalid" ; 
    }
    Todd

  3. #18
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    I like the try/catch/throw approach better.
    I think you are abusing exceptions. Consider:
    Code:
    void year_ok(int yyyy) {
        if (yyyy < 0 ) throw "Invalid Year" ;  
    }
    If an exception is not thrown, the function has no net effect. In other words, the function either throws an exception or does nothing. In terms of semantics, this means that if all went well, the function is absolutely useless. Only when some error condition unexpected by the caller occurs does it have a net effect.

    A quick search of the Web brings up what a blogger has to say about this topic: Exceptions are not exceptional. I do not agree with the terms (i.e., I consider "exceptional" in the sense that it is "unexpected"), but the gist of it is what I have in mind.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #19
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Only, you should probably throw an error instead of a string, or maybe a class or struct with the error and a friendly error message to display to the user.
    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.

  5. #20
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Chappell Hill, Texas
    Posts
    2,332
    Quote Originally Posted by laserlight View Post
    I think you are abusing exceptions. Consider:
    ...
    If an exception is not thrown, the function has no net effect. In other words, the function either throws an exception or does nothing. In terms of semantics, this means that if all went well, the function is absolutely useless. Only when some error condition unexpected by the caller occurs does it have a net effect.
    It absolutely does something. It tells the rest of the program logic that the year is valid.

    Is it an abuse of a feature? Well, that depends on your perspective. I see it as a leverage of a feature. I could of course invent my own messaging system, and add all the supporting logic and communication protocols and structures and global variables, but why, when it is so convenient of a mechanism? The way I see it, throws are there for me, the programmer, to use.

    Todd

  6. #21
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Chappell Hill, Texas
    Posts
    2,332
    I just read that blogger post. I hope he feels better now that he's aired that issue he was dealing with, not that is has anything to do with program structure or design. He has a semantic beef.

    Todd

  7. #22
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    It absolutely does something. It tells the rest of the program logic that the year is valid.
    It does so in an unusual way.

    Take a look at:
    Code:
    try { 
        year_ok(year) ; 
        day_ok(day, month, year) ; 
        cout << setfill('0') << "Date: " << 
        setw(2) << month << "/" << 
        setw(2) << day << "/" << 
        setw(4) << year << " is valid" << endl ; 
    }   
    catch (...) {             
        cout << setfill('0') << "Date: " << 
        setw(2) << month << "/" << 
        setw(2) << day << "/" << 
        setw(4) << year << " is not valid" << endl ; 
    }
    In the code above, we have two branches of logic. If the year is okay and the day is okay, we print the date with "is valid". Otherwise, we print the date with "is not valid". But the control flow is determined by whether or not year_ok or day_ok throws an exception. I daresay that this is akin to the use of goto. It would be clearer to write:
    Code:
    if (isValidDate(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 ; 
    }
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #23
    The larch
    Join Date
    May 2006
    Posts
    3,573
    I think that there is a difference between a function that does two things (despite what the name says) and a function that does one thing and produces some debugging output.

    I also think that there is a difference between high level error messages that are meant for the user to see, and low level error messages that should never reach the end user (unless there is some exceptional relationship between software provider and user).
    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).

  9. #24
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Chappell Hill, Texas
    Posts
    2,332
    Yes Laserlight, that is much simpler and obviously more straight forward.

    My point for the try/catch/throw example was (besides practice for myself) to incorporate the conversions-up-until-then comments about where the detail message should be issued and/or if the detail message should be displayed. With my example, main() can determine if the detail message should be displayed or not.

    Todd

  10. #25
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Yes Laserlight, that is much simpler and obviously more straight forward.
    Indeed, because now the program logic is dictated by normal logical structures. As I noted earlier, I think that having the normal program logic dictated by exceptions is an abuse of exceptions.

    My point for the try/catch/throw example was (besides practice for myself) to incorporate the conversions-up-until-then comments about where the detail message should be issued and/or if the detail message should be displayed.
    Ah, and I must say I did start by mentioning exceptions as a possibility myself, heheh. I think anon is right on this one: "there is a difference between high level error messages that are meant for the user to see, and low level error messages that should never reach the end user". As brewbuck has pointed out, the end user might be able to run the program in a mode where he/she can view the low level error messages (or perhaps have them logged).

    With the use of exceptions, if the main() function were to print the error message, it would be printing the low level error message contained in the exception caught. This would be fine if there was no other choice, but in the normal course of events, the program should just display an ordinary error message telling the user of the mistake.

    Considering the "do one thing and do it well" principle, this means that the function should leave it up to the caller (i.e., main() function) to decide on the error message to show to the user. It might print an error message by itself for logging etc, but that's a low level error message.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  11. #26
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by laserlight View Post
    It does so in an unusual way.

    Take a look at:
    Code:
    try { 
        year_ok(year) ; 
        day_ok(day, month, year) ; 
        cout << setfill('0') << "Date: " << 
        setw(2) << month << "/" << 
        setw(2) << day << "/" << 
        setw(4) << year << " is valid" << endl ; 
    }   
    catch (...) {             
        cout << setfill('0') << "Date: " << 
        setw(2) << month << "/" << 
        setw(2) << day << "/" << 
        setw(4) << year << " is not valid" << endl ; 
    }
    In the code above, we have two branches of logic. If the year is okay and the day is okay, we print the date with "is valid". Otherwise, we print the date with "is not valid". But the control flow is determined by whether or not year_ok or day_ok throws an exception. I daresay that this is akin to the use of goto. It would be clearer to write:
    Code:
    if (isValidDate(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 ; 
    }
    I don't really share your perception that it's as bad as goto.
    The use of goto is usually bad because it can create spaghetti code, that is, it can jump just about anywhere. In the try/catch example, we know where the catch block is and can easily see where it jumps if it fails.
    That mentioned, I don't see it wrong to use either way. It's a taste thing, so to say.
    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.

  12. #27
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    I don't really share your perception that it's as bad as goto.
    I do not think that it is as bad as goto either.

    The use of goto is usually bad because it can create spaghetti code, that is, it can jump just about anywhere. In the try/catch example, we know where the catch block is and can easily see where it jumps if it fails.
    The thing is, we are not evaluating exceptions as a language feature on its own. We are comparing it as a means to control the primary logic of a program, instead of being a means to break from the normal flow of a program due to some unexpected error condition (e.g., passing invalid arguments to a function despite the function contract specifying the range of valid arguments).
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  13. #28
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by laserlight View Post
    The thing is, we are not evaluating exceptions as a language feature on its own. We are comparing it as a means to control the primary logic of a program, instead of being a means to break from the normal flow of a program due to some unexpected error condition (e.g., passing invalid arguments to a function despite the function contract specifying the range of valid arguments).
    I understand, but I also think that throw can be used as either of these. If you use a proper "status" that depicts why the function threw the error, it might just be reasonable. It can also save time (ie if you want to break conditional statements and continue processing later) and avoid goto. I used it once.
    Anyway, that's my thoughts on the matter. Try/throw is not only for errors, but can be used to control the logic in a program, as well.
    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.

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