#1
  1. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Oct 2006
    Posts
    38
    Rep Power
    8

    C# Try/Catch Best Practice Question


    Hi guys, I sincerely hope you can clear this up.

    First off, I'm a young programmer, relatively fresh off the college boat, finishing up some code for a government project written in C#.

    A number of people do code reviews over stuff I have implemented, and have one major complaint that I just quite frankly, don't understand, and I have argued this issue until I'm blue in the face.

    Take for instance something like this, taken from the BusinessLogic layer of a four tier app:

    Code:
    		
    public void SaveDomainObject(HardTypedDS businessTypedDataSet)
    {
    	try
    	{
    		SomeBL myBL = new SomeBL();
    	        myBL.SaveDomainObject(businessTypedDataSet);
    	}
    	catch (Exception ex)
    	{
    		throw ex;
    	}
    }

    This is their development "pattern" they follow, and the main complaint against my code is that I leave out the try/catch entirely.

    Is it just me, or can anybody explain a logical reason why you'd do this? If you just catch and then immediately throw it, you accomplish nothing since its going to cascade up to the next layer anyway, and in fact this syntax destroys the stack trace, right? I could understand it maybe if you were writing some info to a log, or something useful, but their "pattern" is to do nothing with exceptions until the presentation layer, yet to catch and throw at every layer.

    Just seems to me its a waste to put try/catch around everything at every layer of the app, when the only place that Exceptions are actually handled usefully is at the presentation layer.

    Surely I like to assume that people I work for with years of experience know what their doing, but they can't give me a logical reason why they follow this pattern. Seems like a waste of resources and time writing code.

    Thanks guys, in advance.
  2. #2
  3. ASP.Net MVP
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Aug 2003
    Location
    WI
    Posts
    4,378
    Rep Power
    1511
    I don't think the stack trace is destroyed. It's contained in the caught ex object. And re-throwing it actually adds another link in the chain.

    Even if I'm wrong about that (I suppose I could do a quick test to find out), the more important thing here is that it's part of their coding standards. Every development shop has it's own set of standards and expectations for how things are to be done. They're all different, and just about everything will have something silly in it. Unless you're a very senior developer it's usually not a good idea to go challenging them. At least, not yet. The higher ups will always win that battle, and you'll just hurt your reputation by fighting it.

    [edit]
    Okay, it does reset the trace. I was thinking of using Throw by itself in a VB.Net catch block, which results in the original exception moving up the ladder. C# probably has something similar.

    However, my main point still stands. The powers that be may have decided they don't want stack traces moving from tier to tier. Personally, though, I'd do something more like this:
    throw new Exception(ex.Message, ex);
    which is kind of a hybrid between the two. The original stack trace is hidden in the inner exception.

    And in practice I try to avoid using exceptions as much as possible. Better to do as much as possible to make sure they're never thrown in the first place, though of course this isn't always possible.
    Last edited by f'lar; December 10th, 2007 at 12:37 PM.
    Primary Forum: .Net Development
    Holy cow, I'm now an ASP.Net MVP!

    [Moving to ASP.Net] | [.Net Dos and Don't for VB6 Programmers]

    http://twitter.com/jcoehoorn
  4. #3
  5. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Oct 2006
    Posts
    38
    Rep Power
    8
    I read that the stack trace was cleared from this page:

    http://forums.devshed.com/newreply.php?do=newreply&p=1947141

    towards the bottom, under the section titled: "Don't clear the stack trace when re-throwing an exception"

    Maybe I'm too stubborn, I don't mind doing things according to a standard, it just bothers me to do something without knowing the real reason behind it.
  6. #4
  7. Arcane Scribbler
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2005
    Location
    Indianapolis, IN
    Posts
    1,907
    Rep Power
    585
    HAHAHAHAHAH. You are very right, Anthony, that this is retarded.

    Calling throw ex; replaces the stack trace with the current function/line. The only acceptable ways of rethrowing an exception are: A) If you're throwing a new exception to add more helpful information and include the original exception as the inner exception to preserve its information; B) If you just call throw;.

    It is silly for them to catch and just throw, but the only actually destructive part of their practice is calling throw ex; (which replaces the stack trace). If you just call throw;, the stack trace will remain intact.

    Edit:
    @f'lar: The place where the exceptions are actually handled probably isn't geared towards recursing through inner exceptions to get the original stack trace, even if it is logging them. That this anti-pattern is used so prevalently throughout the program, I'm betting that it was born of ignorance of proper exception-handling (and exception-throwing), rather than the desire that the stack trace should not bubble up to the UI.
    Last edited by LyonHaert; December 10th, 2007 at 12:46 PM.
    Joel B Fant
    "An element of conflict in any discussion is a very good thing. Shows everybody's taking part, nobody left out. I like that."

    .NET Must-Haves
    Tools: Reflector
    References: Threading in .NET
  8. #5
  9. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Oct 2006
    Posts
    38
    Rep Power
    8
    Yeah, I know if they at least change it to "throw;" instead of "throw Ex;" it won't be damaging anything, but it just seems like such a huge waste of time.

    From what I understand, isn't try/catches usually expensive on resources as well, slowing down the app unecessarily with so many pointless try/catch blocks?
  10. #6
  11. Arcane Scribbler
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2005
    Location
    Indianapolis, IN
    Posts
    1,907
    Rep Power
    585
    The slow-down is barely noticeable unless you throw many exceptions in a loop or something. It's not the try-block that slows things down; it's the exception.

    You can test it. There's a StopWatch class in System.Diagnostics. You can try a loop on a method DoStuff() (that does nothing) without any exceptions or try-catch. Then a loop with a try-catch inside the loop, with the call to DoStuff() in the try block (do nothing in the catch block). Finally, change DoStuff() to do one thing: throw a new exception.

    At 1000 iterations each: 332, 449, 54798475. These times are in ticks (10000 ticks per millisecond, I think). Edit:Wrong kind of ticks earlier. The new ones are correct. In milliseconds: 0, 0, 5479.
    Last edited by LyonHaert; December 10th, 2007 at 01:04 PM.
    Joel B Fant
    "An element of conflict in any discussion is a very good thing. Shows everybody's taking part, nobody left out. I like that."

    .NET Must-Haves
    Tools: Reflector
    References: Threading in .NET
  12. #7
  13. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Oct 2006
    Posts
    38
    Rep Power
    8
    I figured with sufficiently decent machine it shouldn't matter, and its probably being overly meticulous, the whole thing just grates me to have 50 different classes with a dozen or more methods in each, with every method having a perfectly useless try/catch block in it. Just seems like a waste of time to have this as a standard
  14. #8
  15. Arcane Scribbler
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2005
    Location
    Indianapolis, IN
    Posts
    1,907
    Rep Power
    585
    Well, maybe you can get away with 'throw;' instead of 'throw ex;'. Maybe they won't notice.

    Sometimes all you can do is that little bit in the tug-of-war against such things. I have to constantly fight my boss's whims to maintain integrity in my project. I can't have everything the way I'd prefer it, but at least it's better than if I didn't fight.

    Edit:
    I figured with sufficiently decent machine it shouldn't matter, and its probably being overly meticulous
    Well, their anti-pattern isn't so much a performance issue as it is a design issue, though I bet repeatedly catching and rethrowing the same exception does actually compound the performance hit.
    Last edited by LyonHaert; December 10th, 2007 at 01:41 PM.
    Joel B Fant
    "An element of conflict in any discussion is a very good thing. Shows everybody's taking part, nobody left out. I like that."

    .NET Must-Haves
    Tools: Reflector
    References: Threading in .NET
  16. #9
  17. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2005
    Location
    O-H-I-O
    Posts
    314
    Rep Power
    157
    A really weak argument for maintaining the practice:

    It creates the structure for throwing an appropriate exception. I don't remember exactly, but I think I read in the Microsoft Guidance Explorer (a long time ago) that you should take some of the following actions when error handling:

    1. Try to avoid throwing an exception in the first place.
    2. Try to avoid throwing generic exceptions.

    Code:
    try
         blah blah blah
    catch argEx as ArgumentException
         do stuff
    catch argNullEx as ArgumentNullException
         do something else
    end try
    3. If you are going to throw a generic exception include a detailed message.
    4. Use custom exceptions.
  18. #10
  19. Arcane Scribbler
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2005
    Location
    Indianapolis, IN
    Posts
    1,907
    Rep Power
    585
    CodingHorror.com is also a good read.

    Comments on this post

    • amhaskar agrees : Great article
    Joel B Fant
    "An element of conflict in any discussion is a very good thing. Shows everybody's taking part, nobody left out. I like that."

    .NET Must-Haves
    Tools: Reflector
    References: Threading in .NET
  20. #11
  21. I <3 ASCII
    Devshed Regular (2000 - 2499 posts)

    Join Date
    Aug 2003
    Posts
    2,400
    Rep Power
    1233
    How, 'colorful' are the comments and variable names where you work?

    My former boss was in a presentation where code blew up, the stack traced was displayed, and very colorful language was being displayed in foot tall letters on the projector in front of a crowd.

    Killing the stack trace might be their way of avoiding such things. Even if you don't use such comments third party components might...

    -MBirchmeier

    Comments on this post

    • amhaskar agrees : classy
  22. #12
  23. I lov C in AIX/Linux, hate C++
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jul 2003
    Location
    Jacksonville, Florida
    Posts
    1,655
    Rep Power
    34
    I found having lots of try-catch statement to come in handy. So far, I haven't noticed the slow-down in the application either if it is done properly by what LyonHaert said.

    Here at work, some of the applications were written by a former employee who rarely use the try-catch statement and I was given the assignment to make it work properly. I found that some codes within the function if it run into error wasn't alway passed on to the script which called it and I thought that was interesting. It also explained why we don't noticed the data corruption or broken features lot of times. That become a living hell if the customer discovered it cuz it take so much time to repair the data corruption or application update. So, I included the try-catch statements in all functions and display the error to the pop-up message. That way, anyone can find the errors better and fix it shortly whenever possible. I also noticed that once the source code get so big, it become more common for other scripts (not related to the current code we would like to think of) to run into error without our knowledge.

    A few times I use the try-catch-finally statement so I can restore the control of whatever to the end-users, like the database or freeing the resources.

    It would be awesome if the try-catch statement can include the line # of where the error occur. I wouldn't want to use the stack trace in a production environment though. :-)

    Those are thoughs that might be helpful one day if anyone is interested.

    Scott
  24. #13
  25. Arcane Scribbler
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2005
    Location
    Indianapolis, IN
    Posts
    1,907
    Rep Power
    585
    MBirchmeier: I don't let unhandled exceptions bubble up to the UI. I handle a couple different unhandled-exception events and log it, displaying my own message. And colorful variables are ever only local variables (and are replaced a while later).

    fletchsod: When you compile a WinForms app, beside appname.exe is appname.pdb. This is a symbol file. If you run the exe alone and have it throw an exception and display the stack trace, you won't see the line numbers, but if the pdb file is there when you run the exe, you'll have line numbers. I don't remember if this is only with Debug builds or also works with Release builds, though. (Edit: Works with both.)

    If you keep your functions small and mostly keep to relying on return values (rather than side effects), not having a line number in your stack trace won't be that bad. Unit testing helps a lot, too.
    Last edited by LyonHaert; December 12th, 2007 at 10:07 AM.
    Joel B Fant
    "An element of conflict in any discussion is a very good thing. Shows everybody's taking part, nobody left out. I like that."

    .NET Must-Haves
    Tools: Reflector
    References: Threading in .NET
  26. #14
  27. I lov C in AIX/Linux, hate C++
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jul 2003
    Location
    Jacksonville, Florida
    Posts
    1,655
    Rep Power
    34
    I agree that unit testing helps alot! Oohh! I haven't thought about the pdb with the Release build. I'll have to check it out.

    I forgot to mentioned that I have a log file, so it log all errors from the try-catch statements and a function's name, so the error message doesn't have to be displayed as much. If a customer have a problem, I just grab a copy of the log file if the problem can't be reproduced on every try and do the best I can on the problem. It does help.

IMN logo majestic logo threadwatch logo seochat tools logo