|
You raised very valid points. Thanks.
Understand SOLID! Believe SOLID! Try SOLID! Do implement live SOLID; your Code base becomes Rock SOLID!!!
http://www.codeproject.com/Articles/593751/Code-Review-Checklist-and-Guidelines-for-Csharp-De
|
|
|
|
|
cjb110 wrote: Also is a try/catch block 'free' of any performance/memory penalty?
It should be, certainly is in C++, and I'd imagine most later languages have been sensible enough to copy that implementation. Throwing exceptions has a definite overhead, but probably no more than writing the error code manually.
"If you don't fail at least 90 percent of the time, you're not aiming high enough."
Alan Kay.
|
|
|
|
|
The question is why do you catch exceptions at all... 
Most common answer is - to react to specific errors, for example - AccessDenied exception -> show message to user, SQLException - inform about DB problems or retry/abort and clean up..., etc.
In theory if application is properly writen, nothing unexpected should happen. But what if it happens?
You have two options:
1. Allow you OS/Framework show "unhandled exception" message - this annoys end users and makes troubleshooting harder as most users will close the window and will not report the details
2. Catch all otherwise unhandled exceptions and do something with them - log and rethrow or supress with generic error message and app shutdown/restart
Generally in development - this is not very good, unless you have a breakpoint in catch block or rethrow exception in debug config.
In production - I personally think this is good idea. For sure better than "unhandled exception" or empty catch block. 
|
|
|
|
|
That's a nice explanation Jarek!!! Thanks.
So you mean you support placing try/catch blocks everywhere and catching the main Exception everywhere, right? (But I'm still not supporting this practice)  .
As you said, if it is properly written then no exceptions at all. But yes this is not the case always for some exceptions like MemoryOverflow, AccessDenied, etc. Therefore for only these kind of exceptions might specifically catch them.
Understand SOLID! Believe SOLID! Try SOLID! Do implement live SOLID; your Code base becomes Rock SOLID!!!
http://www.codeproject.com/Articles/593751/Code-Review-Checklist-and-Guidelines-for-Csharp-De
|
|
|
|
|
Mohammed Hameed wrote: So you mean you support placing try/catch blocks everywhere and catching the
main Exception everywhere, right?
Well it depens on your code structure...
Let's say you have the SaveRecord() method. This method is fired from UI and saves the record in DB.
Inside we have code dealing with database surrounded with try/catch(SQLException).
DB code throws SQLException as the DB is down, in catch we log the exception and throw custom RecordNotSavedExecption. UI part catches that and displays message to user.
Or returns false instead true, depending on your pattern.
That's the predictable and desired part.
Now if some other code in SaveRecord() throws OutOfMemoryException then we can:
1. Catch noting - we have OS/Framework unhandled exception. Bad.
2. We can catch all on application level (in one place in code), display generic error message to user, log the error and continue/close the app. Good.
3. We can catch generic Exception in SaveRecord(), log and throw custom RecordNotSavedExecption. Then we have above plus we just cancel the failing operation - not close the whole app. Better?
Mohammed Hameed wrote: But I'm still not supporting this practice
There are many patterns and practices, generally it's good to suport just one.
|
|
|
|
|
I'd tend to have exception-handling code mostly in one place, for example in a "SaveRecord" method - any exceptions raised in methods invoked by this can be caught here.
Putting exception-handling code in every method is overkill and leads to code being harder to read and maintain.
"If you don't fail at least 90 percent of the time, you're not aiming high enough."
Alan Kay.
|
|
|
|
|
Quote: Putting exception-handling code in every method is overkill and leads to code being harder to read and maintain.
My vote for this statement. Thanks.
http://authenticcode.com
|
|
|
|
|
I concur. Further, once the exception has been handled there should (normally) be no need to rethrow it.
This rule leads to the useful guidline: catch exceptions at the lowest level where they can be meaningfully (i.e. completely) handled.
The exception to the no-need-to-rethrow rule is when some cleanup action is needed as the stack is unwound. In that case, catch the specific type of exception, do the local cleanup only, and rethrow (ideally, using throw without a paramter - to preserve the stack trace). Do not duplicate the work that will be done in the can-meaningfully-handle-it catch block (such as logging, notifying the user, clearing context).
Note that the using (...){...} construct implicitly follows this rule: in the event that the contained block of code throws an exception, it will automatically invoke the Dispose() method on the item allocated by the using() statement.
|
|
|
|
|
Yes, I always liked C++'s RAII (Resource Acquisition Is Initialization) approach for exactly this reason. Every scope in C++ acts as an implicit "using" for all names declared within it, and the destructor is implicitly called when the scope exits. This gives deterministic resource management, but does leave more room for developer mistakes than managed languages.
While "using" does allow this (to a degree) in C#, to apply it to every resource would sometimes result in a cascade of nested "using" blocks. In C++, its all automatic.
Personally, I like to think I don't need my hands held by the compiler all the time.
"If you don't fail at least 90 percent of the time, you're not aiming high enough."
Alan Kay.
|
|
|
|
|
Is that to shorten the test period? IMHO one should go for defensive programming:
public double Divide(double term1, double term2){
double division = double.NaN;
if(term2 != 0){
division = term1/term2;
}
else{
division = double.Infinity;
}
return division;
}
instead of try/catch the DivideByZeroException.
|
|
|
|
|
Yes, that's applicable for user exceptions such as one you mentioned (DivideByZero) because developer already knows this fact and should handle it correctly in code like your example.
What about the system exceptions like MemoryOverflow, etc? This is not in control of developer to handle them.
Understand SOLID! Believe SOLID! Try SOLID! Do implement live SOLID; your Code base becomes Rock SOLID!!!
http://www.codeproject.com/Articles/593751/Code-Review-Checklist-and-Guidelines-for-Csharp-De
|
|
|
|
|
But shouldn't the developer *know* if a MemoryOverflow is a likely occurrence in a piece of code?
Otherwise we're saying that the underlying systems and framework are so flakey, that our apps might just randomly bug out...
That can't be a sensible approach to take surely??
For general code in most apps you have to trust the OS and framework will do as it always does.
|
|
|
|
|
Yes, developer does not know when the system resources like memory etc has crossed some threshold or system is short of memory. How do he/she come to know? As it can happen irrespective of whatever the code is being written.
Please, somebody correct me if my perception is not correct.
Understand SOLID! Believe SOLID! Try SOLID! Do implement live SOLID; your Code base becomes Rock SOLID!!!
http://www.codeproject.com/Articles/593751/Code-Review-Checklist-and-Guidelines-for-Csharp-De
|
|
|
|
|
memory exception for about 90% of the time, come from memory leaks caused by not releasing objects.
There are a few occasions were you could anticipate and act accordingly.
Furthermore, I hope there is some testing involved before going into production, any memory issues will come up there and you can act accordingly without the necessary need of exceptions.
Mohammed Hameed wrote: my perception is not correct.
it is not about right or wrong 
|
|
|
|
|
These kind of exceptions still system can throw eventhough resources have been disposed/released.
This might happen if system's overall memory overflows.
Understand SOLID! Believe SOLID! Try SOLID! Do implement live SOLID; your Code base becomes Rock SOLID!!!
http://www.codeproject.com/Articles/593751/Code-Review-Checklist-and-Guidelines-for-Csharp-De
|
|
|
|
|
V. wrote: IMHO one should go for defensive programming:
public double Divide(double term1, double term2){
double division = double.NaN;
if(term2 != 0){
division = term1/term2;
}
else{
division = double.Infinity;
}
return division;
}
instead of try/catch the DivideByZeroException.
If that's C#, you don't need any of that code. If you divide a double by 0, you won't get a DivideByZeroException; you'll get double.Infinity.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
that could very well be, didn´t test that.
|
|
|
|
|
I am catching the top level exception in a large chunk of my current application. I have two reasons for this, but I'm willing to listen to anyone who thinks this is "wrong" and learn a better development practice if I can:
1. I rely heavily on a third party webservice that is very fickle and unpredictable. I have exception handling in here when making calls to this so I can gracefully handle errors from this application for the user. However, even I as I typed this I realized I should be throwing specific exception and not general ones, so I'll be refactoring that code.
2. In general I prefer to capture exceptions and log them so when my users (these are internal applications) call and say they had a problem I can check the log file to help resolve the issue. WIth CustomErrors on the end user normally can't provide me with much in the way of useful information so I rely on my logs for production troubleshooting.
|
|
|
|
|
Very good question.
According to my view, you can catch specific exceptions for the methods, which you are sure what kind of exception(s) can occur. And for the other methods, which you are not probably sure of all possible exceptions, which may occur you can catch the top-level exception.
Obviously, in any case you need to log the exception details.
Note: I would recommend you to go through this entire discussion of this thread you will get more information and also check this link: http://msdn.microsoft.com/en-us/library/vstudio/ms229005(v=vs.100).aspx[^]
http://authenticcode.com
|
|
|
|
|
Just remembered another stunning design stumble in the same area as the multiple database scenario ... they had a SQL table and decided that because the majority of requests required the output in XML format, it would be "best practice" (that weasel word again) to convert the entire table into a blob of XML. They then dropped the original table and recreated it as a table with 1 row and 1 column, into which they inserted the XML blob.
Unfortunately they had not realised that most requests wanted a subset of the table, not the entire table. So for each request, the XML blob had to be converted to a temp table, the appropriate SELECT run on it, and the results sent back to the requester as XML ... Phew!! Inserts, updates and deletes had to go through the same process: XML blob -> temp table -> apply insert/update/delete -> convert back to XML blob, save back in the "table".
|
|
|
|
|
Precious...
Are you still working there?
Careful, it might be contagious... 
|
|
|
|
|
Haha, I'm too lazy a programmer to make work for myself like that
I'm on contract 
|
|
|
|
|
|
Yeah, that basically sums it up
|
|
|
|
|
johnsyd wrote: Inserts, updates and deletes had to go through the same process: XML blob -> temp table -> apply insert/update/delete -> convert back to XML blob, save back in the "table".
Presumably without any locking to ensure that concurrent changes don't get lost? 
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
