Catch an Exception... then throw it?
-
An inexperienced developer I use to work with kept telling me that she, "..wanted to add exception handling.." to her code. For various reasons, I wasn't quite sure what she meant by that and for other reasons, I didn't bother asking... A couple weeks later, I did a diff on one of her check-ins to find that she had gone into a particular class (C#) and added try/catch blocks to every function in the class that looked like this:
public void FunctionName()
{
try
{
// code that was previously contained in the function...
}
catch (Exception e)
{
throw e
}
} -
An inexperienced developer I use to work with kept telling me that she, "..wanted to add exception handling.." to her code. For various reasons, I wasn't quite sure what she meant by that and for other reasons, I didn't bother asking... A couple weeks later, I did a diff on one of her check-ins to find that she had gone into a particular class (C#) and added try/catch blocks to every function in the class that looked like this:
public void FunctionName()
{
try
{
// code that was previously contained in the function...
}
catch (Exception e)
{
throw e
}
}Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
-
Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
Rod Kemp wrote:
What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
...and a code horror. :)
-
Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
:thumbsup:
-
Rod Kemp wrote:
What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
...and a code horror. :)
aspdotnetdev wrote:
and a code horror
Personally I would only view this as a true horror if it was done by an experienced developer who should know better, such as that use of
Goto
that DD shared the other day. This is a rookie developer that according to the OP kept telling me that she, "..wanted to add exception handling.." to her code which would suggest she didn't know where to start and didn't get any help/tips/pointers on how to go about it. I think a good way to judge if something is a coding horror is the 2x4 test. If you take into account the experience of the developer and look at the code and think that maybe you should explain how something works, then it's not a horror as such, if however your first thought is, where is that 2x4 so you can beat them over the head with it, then it is a horror.People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
-
An inexperienced developer I use to work with kept telling me that she, "..wanted to add exception handling.." to her code. For various reasons, I wasn't quite sure what she meant by that and for other reasons, I didn't bother asking... A couple weeks later, I did a diff on one of her check-ins to find that she had gone into a particular class (C#) and added try/catch blocks to every function in the class that looked like this:
public void FunctionName()
{
try
{
// code that was previously contained in the function...
}
catch (Exception e)
{
throw e
}
}It does actually get worse. I've seen
public int FormatSomethingOrRather(string txt)
{
try
{
// Convert string to int
}
catch (Exception)
{}
}That took about a day to track down the bug it was causing. So after fixing I ran FxCop and found dozens :omg:
-
An inexperienced developer I use to work with kept telling me that she, "..wanted to add exception handling.." to her code. For various reasons, I wasn't quite sure what she meant by that and for other reasons, I didn't bother asking... A couple weeks later, I did a diff on one of her check-ins to find that she had gone into a particular class (C#) and added try/catch blocks to every function in the class that looked like this:
public void FunctionName()
{
try
{
// code that was previously contained in the function...
}
catch (Exception e)
{
throw e
}
} -
An inexperienced developer I use to work with kept telling me that she, "..wanted to add exception handling.." to her code. For various reasons, I wasn't quite sure what she meant by that and for other reasons, I didn't bother asking... A couple weeks later, I did a diff on one of her check-ins to find that she had gone into a particular class (C#) and added try/catch blocks to every function in the class that looked like this:
public void FunctionName()
{
try
{
// code that was previously contained in the function...
}
catch (Exception e)
{
throw e
}
}But is she a HPOA?
-
Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
Clearly there are uses for catching an exception of a particular type or of any type, doing some processing and then re-throwing the exception. However, that was not the case here. She just re-threw it. I recognize that it is technically harmless but this individual was supposedly weeks away from graduating with a CS degree - at that point you should understand exceptions, scope and all sorts of fundamentals that this kind of coding shows she does not.
-
But is she a HPOA?
One can onlya hopa.
-
Clearly there are uses for catching an exception of a particular type or of any type, doing some processing and then re-throwing the exception. However, that was not the case here. She just re-threw it. I recognize that it is technically harmless but this individual was supposedly weeks away from graduating with a CS degree - at that point you should understand exceptions, scope and all sorts of fundamentals that this kind of coding shows she does not.
doja93 wrote:
graduating with a CS degree - at that point you should understand exceptions, scope and all sorts of fundamentals
:laugh: :laugh: Thanks for that I needed a good laugh. :laugh: :laugh: Seriously though a CS graduate generally graduates with a shed load of theory but their ability to implement it in a practical sense is limited that is where working with a more senior developer comes in so that they can learn the correct way to implement the theory, and learn things such as defensive programming, and to undo some of the rubbish that some instructors teach them.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
-
Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
I'd also like to point out that of all the post I read in this thread, the parent to this post is the only one that was actually rethrowing the exception. Everyone else was repackaging the exception then throwing the repackaged exception which usually has the unintended consequence of losing the stack on the throw. Using the method the parent used ("throw" without a exception type reference) rethrows the last exception, full stack spool and all. i.e. Good:
try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-throw the exception, the stack stays safe.
throw;
}Bad:
try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-package the exception then throw it
// probably losing the stack along the way
throw ex;
} -
Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
To your second example, the easier solution would be not to have a "general" catch block at all, right?
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception.
//Other exceptions will remain uncaught.
}On the other hand, the opposite really does require a rethrow:
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//We really want to rethrow just that exception for some reason.
throw;
}
catch
{
//Whereas for everything else, we want to handle the exception here...
} -
To your second example, the easier solution would be not to have a "general" catch block at all, right?
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception.
//Other exceptions will remain uncaught.
}On the other hand, the opposite really does require a rethrow:
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//We really want to rethrow just that exception for some reason.
throw;
}
catch
{
//Whereas for everything else, we want to handle the exception here...
}You could leave out the general catch but personally I prefer to have it there as I think it leads to the readability of the code, also I generally don't have a general catch that re-throws the exception without logging the exception in some manner such as the first example I showed.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
-
I'd also like to point out that of all the post I read in this thread, the parent to this post is the only one that was actually rethrowing the exception. Everyone else was repackaging the exception then throwing the repackaged exception which usually has the unintended consequence of losing the stack on the throw. Using the method the parent used ("throw" without a exception type reference) rethrows the last exception, full stack spool and all. i.e. Good:
try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-throw the exception, the stack stays safe.
throw;
}Bad:
try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-package the exception then throw it
// probably losing the stack along the way
throw ex;
}HI, I do not really understant why we have to catch the exception and throw it. I mean if a (first)function/sub is caling another (Second)function/sub if any exception is coming in sencond function why we have to catch it and throw it. It will automatically throw the exception right?
function1()
{
try
{
calling function2()
}
catch(exception e)
{
//I handled the exception thrown from the functions/subs called
}}
function2()
{
//if any error/exception comes here .net handler automatically throws it to the calling function/sub where it was handled.
}modified on Monday, November 29, 2010 7:18 AM
-
To your second example, the easier solution would be not to have a "general" catch block at all, right?
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception.
//Other exceptions will remain uncaught.
}On the other hand, the opposite really does require a rethrow:
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//We really want to rethrow just that exception for some reason.
throw;
}
catch
{
//Whereas for everything else, we want to handle the exception here...
}You can also use re-throwing an exception to add more information e.g. public void SomeFunction() { try { // Do stuff } catch (Exception e) { // Do logging throw new Exception("Some additional informative text.", e); } } I've used this technique before, you just need to remember to walk the nested exceptions (InnerException) when you finally process the Exception.
Kevin Rucker, Application Programmer QSS Group, Inc. United States Coast Guard OSC Kevin.D.Rucker@uscg.mil "Programming is an art form that fights back." -- Chad Hower
-
I'd also like to point out that of all the post I read in this thread, the parent to this post is the only one that was actually rethrowing the exception. Everyone else was repackaging the exception then throwing the repackaged exception which usually has the unintended consequence of losing the stack on the throw. Using the method the parent used ("throw" without a exception type reference) rethrows the last exception, full stack spool and all. i.e. Good:
try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-throw the exception, the stack stays safe.
throw;
}Bad:
try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-package the exception then throw it
// probably losing the stack along the way
throw ex;
}With regard to rethrowing exceptions and preserving the stack trace I found this helpful: rethrowing exceptions and preserving the full call stack trace
-
Clearly there are uses for catching an exception of a particular type or of any type, doing some processing and then re-throwing the exception. However, that was not the case here. She just re-threw it. I recognize that it is technically harmless but this individual was supposedly weeks away from graduating with a CS degree - at that point you should understand exceptions, scope and all sorts of fundamentals that this kind of coding shows she does not.
doja93 wrote:
I recognize that it is technically harmless
Not really... rethrowing the exception with
throw e;
breaks the stack and you never get to see where the original exception was thrown. That said, I'm with what the others said earlier in the thread: It's a good opportunity to mentor someone before they get ruined by some jackaninny who doesn't understand proper exception handling.============================= I'm a developer, he's a developer, she's a developer, Wouldn't ya like to be a developer too?
-
Meh, catching and re-throwing exceptions does have its uses, just not with
throw e
. Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;try
{
// code that may throw exception here
}
catch(Exception ex)
{
// error logging here
// re-thrown the exception
throw;
}Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
// code that may throw exception here
}
catch(SQLException sqlex)
{
//Handle this exception here
//Don't re-throw the exception
}
catch
{
throw; //re-throw all other exceptions that may occur
}What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
Rod Kemp wrote:
try { // code that may throw exception here } catch(SQLException sqlex) { //Handle this exception here //Don't re-throw the exception } catch { throw; //re-throw all other exceptions that may occur }
You really should not have put in the generic catch in this case. While cycles are cheap, this throws many cycles away for no reason. Simply catch those you are interested in and let the others roll up. No need to catch them if you plan to do nothing but throw them.
-
Rod Kemp wrote:
try { // code that may throw exception here } catch(SQLException sqlex) { //Handle this exception here //Don't re-throw the exception } catch { throw; //re-throw all other exceptions that may occur }
You really should not have put in the generic catch in this case. While cycles are cheap, this throws many cycles away for no reason. Simply catch those you are interested in and let the others roll up. No need to catch them if you plan to do nothing but throw them.
I never said this was production code but rather a way to teach an inexperienced developer about exception handling which should go beyond try/catch blocks and also incorporate defensive programming, maybe you should read the rest of my comments. That said, during normal operation having the general catch block has no impact on performance it is only when something goes wrong that it may take a few cycles to propagate the error, but honestly if you're bothering to optimise your code at that point you really need to get out more.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs