|
try catch statements should only be used in the UI/client layer, not in business layer or data layer - usually.
you should ALWAYS log your exceptions to a database or file log without exception.
you should not use try/catch/exceptions to control logic flow, whenever possible.
my 2 cents.
the code you referenced is crap IMHO. instead of re-throwing the exception, it should be logged, and handled gracefully and informatively for the end user.
|
|
|
|
|
*clapping*
Finally, another dev gets it!
If you are not going to take care of the exception then don't catch it.
And by taking care of the exception, I don't mean to log it and forget it. Actually, do something about it.
|
|
|
|
|
It's perfectly legal. There's nothing wrong with that.
I do that all the time.
|
|
|
|
|
"And speaking of logging, I have seen such try/catch/logging being nested zillion of time resulting in zillion of log entry for one single exception... yuk..."
That's the Java Spring way, and 128 other exceptions ...
|
|
|
|
|
The code I support came with hundreds of
try
{
... do stuff ...
}
catch (Exception ex)
{
throw ex;
}
I spoke to the original developer, but they are still doing the same thing even now.
|
|
|
|
|
Oooo... this is doubly terrible 
- useless try/catch
- hiding the original stack!
 
|
|
|
|
|
Agree.
I did like Sanders fix though.
However the sub or function that they are calling is defined as a despicable act.
Maybe you should have complained about that as well. 
"Rock journalism is people who can't write interviewing people who can't talk for people who can't read." Frank Zappa 1980
|
|
|
|
|
Your code is not the worst sample of "logic"!
try {
DoX();
}
catch (Exception ex) {
MessageBox.Show("Something happen!");
}
THIS code is a poison of modern apps!! NOBODY knows what happen, where happen, just "close app" and say goodbye to all your work. Even monkeys from MS (I remember - monkeys, SELECTED by HR!) do such things. And when you report to MS "you have Error 0x45454582354", they (like imbeciles) advice you to reboot computer. HEY!! It's your program, get off my cookies, cache, operating system and codecs - fix YOUR failures! damn...
|
|
|
|
|
If you don't trap the error, well... it will just splat on the user screen.
Catching, and deciding what to do, log, or show... will help you <i>build new code,</i> because you will easily be able to fix your old code Having said all that, the code block frustrates me too. Its a bit ugly, and tedious.
A while back they made properties "easier" with all that get / setter code.
I would like to see an auto Trap block, or at least a simpler model for it.
Keep It Simple, keep it moving.
|
|
|
|
|
Just so you know, the code does throw an error, the catching is just to add the useless information "I am here" and rethrow...
So, your comment, as far as I understand, is irrelevant to the situation...
|
|
|
|
|
OK I think I see your point now -
someone codes a trap and really doesn't do anything with it but comment? Is that it?
Keep It Simple, keep it moving.
|
|
|
|
|
Indeed....
It goes further, as I explained tersely... This is recursively done in multiple method that catch and rethrow each other, making the code more verbose and slower and less debugable without any benefit...
|
|
|
|
|
I have seen too. Sometimes adding context is helpful (most of the times it doesn't). If we need to add context, you can use Exception.Data to add context if really really needed.
https://docs.microsoft.com/en-us/dotnet/api/system.exception.data?view=net-5.0
Maybe I, Maybe U, can make a change to the world!
|
|
|
|
|
Yes, but in that case it's not happening! 
|
|
|
|
|
More times than I can count, I have seen debugging and fix time for a production issue cut from days to hours, or hours to minutes, because the appropriate runtime variables were added as name-value pairs to the exception's Data collection. That reduced time is gold to a software project during the full SDLC.
|
|
|
|
|
Thank you for your post. It got me thinking. Maybe I should place a try at each method's entry and a catch at each method's exit so as to add the method's signature and argument list to the exception message. A one line try at entry and a one line catch at exit doesn't offend my sense of aesthetics. Unfortunately as far as I know it would require knowing the types of exceptions thrown by each called method and I don't know how to do that w/o it being a pain in the rear. Perhaps this could be automated. I don't see why not. Something to cogitate on. For now though I have some hopefully exceptional code to write. - Cheerio
|
|
|
|
|
I have seen too. Sometimes adding context is helpful (most of the times it doesn't). If we need to add context, you can use Exception.Data to add context if really really needed.
https://docs.microsoft.com/en-us/dotnet/api/system.exception.data?view=net-5.0
Maybe I, Maybe U, can make a change to the world!
|
|
|
|
|
If they logged, or added something useful to the new Exception (let's say there were parameters to this method, and the exception method was more like ["Having trouble doing X for " + param.ToString()]), then maybe I could see an excuse for a catch and throw.
As you show it, you're right to burn it to the ground.
|
|
|
|
|
Pete McNamee
"True knowledge exists in knowing that you know nothing."
|
|
|
|
|
But you have to do it that way for the super catch block to work. Excuse the syntax due to phone!
Catch (Exception ex){
If (ex.getMessage() == “Having problem doing X”)
Else If (ex.getMessage() == “Having problem doing Y”)
Else If (ex.getMessage() == “Having problem doing Z”)
}
Oh no, I forgot the exclamation mark and a default “catch” all.
|
|
|
|
|
Hahah! 
|
|
|
|
|
Been programming C/C++ for 26 years...never once used a try/catch (I think it's more humane to let the abortive program die and analyze the carcass...I mean the core dump).
And letting videogame consoles die in the middle of a game is more exciting than praying it's not having exceptions.
|
|
|
|
|
Wow.. metagaming here! 
|
|
|
|
|
IMHO, the correct answer to "How should I handle exceptions?" is "Do Not." ("Unless you know how to recover from a specific exception." (e.g. FileNotFoundExceptions are easy to deal with. However, if you try to handle things like DivideByZeroException, OutOfMemoryException, or StackOverflowException, you're pretty much attached by an inclined plane wrapped helically around an axis.)
Also (I might catch (pun intended) a lot of flak for this):
- re-throwing exceptions should be avoided like the plague, as doing so can potentially cause a single exception to be logged multiple times. Unfortunately, of the
async/await (i.e. things in the System.Threading.Tasks namespace) re-throw a lot, which leads to a lot of noise and distractions while debugging). - a
catch { ... } block is the wrong place to log exceptions.
A better alternative is use [Exception Filters] for logging. For example:
try {
Foo();
}
catch(SpecificException ex) when (ex.Log()) {
}
(Where .Log is an extension method that always returns false).
An even better alternative is to avoid logging exceptions in catch blocks or exception filters altogether. I am glad that it was decided not to remove the System.AppDomain class in .NET Core and .NET 5+ for this very reason: you can use the [AppDomain.FirstChanceException] and [AppDomain.UnhandledException] events as the one (erm... OK, two) place(s) to log all exceptions. You can even get fancy and add something to the Exception.Data dictionary in those events to avoid logging the same exception multiple times. However, in order for this to be useful, exceptions should not be re-thrown in order to ensure the logging of complete stack traces.
Eagles my fly, but weasels don't get sucked into jet engines.
|
|
|
|
|
The one variant I think can be useful involves an opaque abstraction, which defines a set of its own exceptions, framed solely in terms of that abstraction. Any opaque abstraction is violated when an unanticipated implementation failure occurs, as it cannot be framed in terms of the abstraction. The best the abstraction can offer is a "weasel words" exception that chains the original cause, shamefully "exposing its call stack to all the world". Recovery code can log the cause for the support ticket.
The posted code, of course, is simply gross. It adds nothing, hides nothing, and loses everything that might be of support value, while working hard at looking lazy. Doing nothing would be far lazier and far more effective.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
