Try Catch
-
I was cleaning up some old mailing system code developed by some contractors at my work about 3 years before I even joined the company. It was designed in .NET 1.1, but it worked, so we never really upgraded it... I'll be re-writing the whole thing from scratch now that I've seen what it looks like (after we encountered our first issue). Tell me if you can see the problem with this (pseudo code, as I cannot release the actual code for public display)...
try
{
/*
-- Insert 200 lines of:
Class declarations,
Internal function calls,
External function calls,
Database function calls,
String Concatination of variables without checking/handling types (insert the wrong type and it will fall over),
Send an email
*/
}
catch (Exception ec)
{
/*
-- Send email to bugs@ourcompany.com...
email address does not exist as far as any of the current dev team are aware,
there is no exception handling on the sending of the email, so if there mail server is down, it doesn't give you much insight
*/
}As a temp measure to view the outcome, I changed the bugs email address to my own email address, to find that when the email sent, the error merely pointed at the catch{} lines of code, without giving any indication at all as to where inside the 200 lines of code the error was occuring!
-
I was cleaning up some old mailing system code developed by some contractors at my work about 3 years before I even joined the company. It was designed in .NET 1.1, but it worked, so we never really upgraded it... I'll be re-writing the whole thing from scratch now that I've seen what it looks like (after we encountered our first issue). Tell me if you can see the problem with this (pseudo code, as I cannot release the actual code for public display)...
try
{
/*
-- Insert 200 lines of:
Class declarations,
Internal function calls,
External function calls,
Database function calls,
String Concatination of variables without checking/handling types (insert the wrong type and it will fall over),
Send an email
*/
}
catch (Exception ec)
{
/*
-- Send email to bugs@ourcompany.com...
email address does not exist as far as any of the current dev team are aware,
there is no exception handling on the sending of the email, so if there mail server is down, it doesn't give you much insight
*/
}As a temp measure to view the outcome, I changed the bugs email address to my own email address, to find that when the email sent, the error merely pointed at the catch{} lines of code, without giving any indication at all as to where inside the 200 lines of code the error was occuring!
Well, you know, exception handling automatically-magically-definitely solves the goto-spaghetti-code nightmare... :rolleyes:
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles] -
I was cleaning up some old mailing system code developed by some contractors at my work about 3 years before I even joined the company. It was designed in .NET 1.1, but it worked, so we never really upgraded it... I'll be re-writing the whole thing from scratch now that I've seen what it looks like (after we encountered our first issue). Tell me if you can see the problem with this (pseudo code, as I cannot release the actual code for public display)...
try
{
/*
-- Insert 200 lines of:
Class declarations,
Internal function calls,
External function calls,
Database function calls,
String Concatination of variables without checking/handling types (insert the wrong type and it will fall over),
Send an email
*/
}
catch (Exception ec)
{
/*
-- Send email to bugs@ourcompany.com...
email address does not exist as far as any of the current dev team are aware,
there is no exception handling on the sending of the email, so if there mail server is down, it doesn't give you much insight
*/
}As a temp measure to view the outcome, I changed the bugs email address to my own email address, to find that when the email sent, the error merely pointed at the catch{} lines of code, without giving any indication at all as to where inside the 200 lines of code the error was occuring!
He probably was told to use try-catch so he did, how could that be wrong? ;) I have seen this kind of thing from people who aren't used to try-catch and/or are just putting it in the code because someone told them to do it. Of course, if you want the try-catch to be useful for tracking down issues in the code then having a huge block where there could be dozens of points of failure and not catching the individual types of exceptions is not the way to go. Bill W
Just because code works, it doesn't mean that it is good code.
-
I was cleaning up some old mailing system code developed by some contractors at my work about 3 years before I even joined the company. It was designed in .NET 1.1, but it worked, so we never really upgraded it... I'll be re-writing the whole thing from scratch now that I've seen what it looks like (after we encountered our first issue). Tell me if you can see the problem with this (pseudo code, as I cannot release the actual code for public display)...
try
{
/*
-- Insert 200 lines of:
Class declarations,
Internal function calls,
External function calls,
Database function calls,
String Concatination of variables without checking/handling types (insert the wrong type and it will fall over),
Send an email
*/
}
catch (Exception ec)
{
/*
-- Send email to bugs@ourcompany.com...
email address does not exist as far as any of the current dev team are aware,
there is no exception handling on the sending of the email, so if there mail server is down, it doesn't give you much insight
*/
}As a temp measure to view the outcome, I changed the bugs email address to my own email address, to find that when the email sent, the error merely pointed at the catch{} lines of code, without giving any indication at all as to where inside the 200 lines of code the error was occuring!
Once, many moons ago, I got talked to by another developer because I wasn't using try catch blocks to do error handling. I checked and validated results as I went, using if/elses. No, it wasn't particularly elegant and I would do it differently now but it worked and handled failures well. A few days later I looked at some of his code to get an idea for how to structure it using try/catch and came upon something very much like what you posted. I was horrified, but because I knew try/catch was all the rage, and I was new, I thought his was more correct and didn't say anything. The company went out of business a few months later. (Although, to be fair, that was more likely a result of us having free arcade games in the office and spur of the moment Quake tournaments.) It was a long time before I realized the true benefits of properly structured exception handling, thanks to some early misguidance. I'm still appalled at how it gets frequently abused though.
He said, "Boy I'm just old and lonely, But thank you for your concern, Here's wishing you a Happy New Year." I wished him one back in return.
-
Once, many moons ago, I got talked to by another developer because I wasn't using try catch blocks to do error handling. I checked and validated results as I went, using if/elses. No, it wasn't particularly elegant and I would do it differently now but it worked and handled failures well. A few days later I looked at some of his code to get an idea for how to structure it using try/catch and came upon something very much like what you posted. I was horrified, but because I knew try/catch was all the rage, and I was new, I thought his was more correct and didn't say anything. The company went out of business a few months later. (Although, to be fair, that was more likely a result of us having free arcade games in the office and spur of the moment Quake tournaments.) It was a long time before I realized the true benefits of properly structured exception handling, thanks to some early misguidance. I'm still appalled at how it gets frequently abused though.
He said, "Boy I'm just old and lonely, But thank you for your concern, Here's wishing you a Happy New Year." I wished him one back in return.
I would not have minded if he made use of if/else statements or at least something to handle or track issues, but there was not of that, just a load of messy code! I found earlier this year, done by the same guy, some code which as a HUGE for-loop containing dozens of if/else statements to calculate values for paging data, when I was able to replace it all with one single line of math... I could understand if this guy was a junior, but he was a "professional contractor"!
-
Once, many moons ago, I got talked to by another developer because I wasn't using try catch blocks to do error handling. I checked and validated results as I went, using if/elses. No, it wasn't particularly elegant and I would do it differently now but it worked and handled failures well. A few days later I looked at some of his code to get an idea for how to structure it using try/catch and came upon something very much like what you posted. I was horrified, but because I knew try/catch was all the rage, and I was new, I thought his was more correct and didn't say anything. The company went out of business a few months later. (Although, to be fair, that was more likely a result of us having free arcade games in the office and spur of the moment Quake tournaments.) It was a long time before I realized the true benefits of properly structured exception handling, thanks to some early misguidance. I'm still appalled at how it gets frequently abused though.
He said, "Boy I'm just old and lonely, But thank you for your concern, Here's wishing you a Happy New Year." I wished him one back in return.
David Kentley wrote:
I'm still appalled at how it gets frequently abused though.
try { // some code } catch {} is quite common. :) There can be some cases in which such a pattern is fine, though usually you should be catching a specific exception, but you should always explain why/what you're doing in such cases.
Kevin
-
David Kentley wrote:
I'm still appalled at how it gets frequently abused though.
try { // some code } catch {} is quite common. :) There can be some cases in which such a pattern is fine, though usually you should be catching a specific exception, but you should always explain why/what you're doing in such cases.
Kevin
Kevin McFarlane wrote:
There can be some cases in which such a pattern is fine, though usually you should be catching a specific exception, but you should always explain why/what you're doing in such cases.
I wish .net included a few more 'tryDoSomething' methods. For example, if one thread has just changed some information that should be displayed be a control, a sensible thing to do is to BeginInvoke the control's update handler (preferably using a 'Threading.Interlocked.CompareExchange'ed flag to prevent excessive 'BeginInvoke's). Unfortunately, I know of no good way to avoid the risk of the control being disposed before the BeginInvoke. In a situation like that, a tryBeginInvoke would be extremely handy; if it works, great. If the control's been disposed or otherwise lacks a windowing context, there's no need to update it and the call may be safely ignored. If a try block contains a single BeginInvoke and the catch block is empty, is that not sufficiently clear "Try this, and if it works great--if not, meh."
-
I would not have minded if he made use of if/else statements or at least something to handle or track issues, but there was not of that, just a load of messy code! I found earlier this year, done by the same guy, some code which as a HUGE for-loop containing dozens of if/else statements to calculate values for paging data, when I was able to replace it all with one single line of math... I could understand if this guy was a junior, but he was a "professional contractor"!
Professional contractors follow Sturgeon's Law closely, except that the percentage is more likely to be 98% in my experience.
-
Once, many moons ago, I got talked to by another developer because I wasn't using try catch blocks to do error handling. I checked and validated results as I went, using if/elses. No, it wasn't particularly elegant and I would do it differently now but it worked and handled failures well. A few days later I looked at some of his code to get an idea for how to structure it using try/catch and came upon something very much like what you posted. I was horrified, but because I knew try/catch was all the rage, and I was new, I thought his was more correct and didn't say anything. The company went out of business a few months later. (Although, to be fair, that was more likely a result of us having free arcade games in the office and spur of the moment Quake tournaments.) It was a long time before I realized the true benefits of properly structured exception handling, thanks to some early misguidance. I'm still appalled at how it gets frequently abused though.
He said, "Boy I'm just old and lonely, But thank you for your concern, Here's wishing you a Happy New Year." I wished him one back in return.
David Kentley wrote:
I checked and validated results as I went, using if/elses.
I'm a fan of doing some error trapping this way. It's quick and you can log a very specific error message that says exactly what's wrong, like an object doesn't exist. That's good for the developer that has to fix the issue. Besides, having an exception thrown and being caught is expensive from a processor time point of view. I've also seen large blocks of code wrapped in a try catch block where the catch block logged an "unhandled" exception. Might as well have said "s&*t happened"!
-
David Kentley wrote:
I checked and validated results as I went, using if/elses.
I'm a fan of doing some error trapping this way. It's quick and you can log a very specific error message that says exactly what's wrong, like an object doesn't exist. That's good for the developer that has to fix the issue. Besides, having an exception thrown and being caught is expensive from a processor time point of view. I've also seen large blocks of code wrapped in a try catch block where the catch block logged an "unhandled" exception. Might as well have said "s&*t happened"!
That reminds me of a great piece of code and comment in Quake:
void(entity targ, entity attacker) ClientObituary =
{
local float rnum;
local string deathstring, deathstring2;
rnum = random();
...
if (targ.classname == "player")
{
if (attacker.classname == "player")
{
if (targ == attacker)
{
// killed self
attacker.frags = attacker.frags - 1;
bprint (targ.netname);if (targ.weapon == 64 && targ.waterlevel > 1) { bprint (" discharges into the water.\\n"); return; } if (targ.weapon == IT\_GRENADE\_LAUNCHER) bprint (" tries to put the pin back in\\n"); else bprint (" becomes bored with life\\n"); return; } else { attacker.frags = attacker.frags + 1; rnum = attacker.weapon; if (rnum == IT\_AXE) { deathstring = " was ax-murdered by "; deathstring2 = "\\n"; }...
bprint (targ.netname);
bprint (deathstring);
bprint (attacker.netname);
bprint (deathstring2);
}
return;
}
else
{
targ.frags = targ.frags - 1;
bprint (targ.netname);// killed by a montser? if (attacker.flags & FL\_MONSTER) { if (attacker.classname == "monster\_army") bprint (" was shot by a Grunt\\n"); if (attacker.classname == "monster\_demon1") bprint (" was eviscerated by a Fiend\\n");...
return;
}// tricks and traps if (attacker.classname == "explo\_box") { bprint (" blew up\\n"); return; } if (attacker.solid == SOLID\_BSP && attacker != world) { bprint (" was squished\\n"); return; }...
// fell to their death?
if (targ.deathtype == "falling")
{
targ.deathtype = "";
bprint (" fell to his death\n");
return;
}// hell if I know; he's just dead!!! bprint (" died\\n");The whole routine is about 250 lines long, but for whatever reason I particularly like the last comment.
-
I was cleaning up some old mailing system code developed by some contractors at my work about 3 years before I even joined the company. It was designed in .NET 1.1, but it worked, so we never really upgraded it... I'll be re-writing the whole thing from scratch now that I've seen what it looks like (after we encountered our first issue). Tell me if you can see the problem with this (pseudo code, as I cannot release the actual code for public display)...
try
{
/*
-- Insert 200 lines of:
Class declarations,
Internal function calls,
External function calls,
Database function calls,
String Concatination of variables without checking/handling types (insert the wrong type and it will fall over),
Send an email
*/
}
catch (Exception ec)
{
/*
-- Send email to bugs@ourcompany.com...
email address does not exist as far as any of the current dev team are aware,
there is no exception handling on the sending of the email, so if there mail server is down, it doesn't give you much insight
*/
}As a temp measure to view the outcome, I changed the bugs email address to my own email address, to find that when the email sent, the error merely pointed at the catch{} lines of code, without giving any indication at all as to where inside the 200 lines of code the error was occuring!
using try-catch blocks on a whole function may be ok in some situation (for example, if function is executed on a thread from the .Net Thread Pool). The reall coding horror here is that the method is more than 200 lines long (can't wait to debug that baby..)
