|
I think this deserves a post in the Weird and the Wonderful forum-
|
|
|
|
|
Yea, I realized that afterwards! ^_^
|
|
|
|
|
I'm afraid to look at some of my early code because I'm afraid I'd see something like this.
|
|
|
|
|
Ugh, I've seen code like that many times!
try {
DoX();
}
catch (Exception ex) {
throw new Exception("Having problem Doing X.", ex);
} Fixed it.
That exclamation mark is just screamy and unprofessional
|
|
|
|
|
Glad you understand!
|
|
|
|
|
I always think that putting an exclamation mark at the end of an error message is the equivalent of adding "you idiot!".
"Value must be numeric, you idiot!"
"Date must be in d-ddd:mmm/yyyyy format, you idiot!"
"Credit card number must not include spaces, you idiot!" (world's most annoying message: why can't you remove the spaces yourself?)
Or in "success" messages:
"File has been saved!" (like it's some sort of special achievement, and the computer is being extra kind to you in doing it)
|
|
|
|
|
Member 14885955 wrote: "File has been saved!" (like it's some sort of special achievement, and the computer is being extra kind to you in doing it)
Maybe the software is so bad that successfully saving a file is the exceptional case...
|
|
|
|
|
I actually know someone who puts an exclamation behind pretty much every message box!
Or three in case of an error!!!
Really childish and unprofessional and this is a smart guy with good business instincts (but no programming instincts whatsoever ).
|
|
|
|
|
ditching original exception seems like a good idea (no)
|
|
|
|
|
<JOKE>
Well, obviously it should be more like:
try
{
DoX();
}
catch (Exception ex)
{
throw ex;
}
</JOKE>
Personally, I wish the Exception class had been declared abstract. If you need to throw a new exception, it should always be something more specific than the base Exception class.
Especially prior to .NET 4 and the HandleProcessCorruptedStateExceptions attribute[^] , when a catch block for the base Exception class would also catch "corrupted process state" exceptions, which should pretty much never be caught.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
another tragic mistake! You should have just
throw;
That preserve the calllstack ^_^
Hey, I didn't know this attribute!
|
|
|
|
|
That was my whole point - I've lost track of the number of times I've seen catch (Exception ex) { throw ex; } used in QA questions.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Haha, got me!
|
|
|
|
|
While the message reported here is rather useless, I could see it containing useful details, and the expectation is that somebody among the callers will log it.
|
|
|
|
|
It would be better to add info at each step up the call stack (for debugging info) into the exception's Data collection, then just a throw; statement. Once at the top of the call stack, handle the exception and log the contents of the exception and its data collection.
|
|
|
|
|
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...
|
|
|
|