Everywhere a single Catch block used catching top Exception all times.
-
Let me start this discussion something like this: Obviously, we need to catch only specific exceptions and that too if it is required. But unfortunately this is not the case in my company. Here for each and every method (regardless it is required or not) you will find a single catch catching only the top Exception. Error logging is being done here and then sometimes it is being rethrown. I dont understand this scheme/practice and hence I discussed regarding it with my Developers, peers, and Manager(s) but most of them say something like these: --> 'It does not make any difference' --> 'We are doing in this same fashion everywhere from beginning' --> 'My TL asked me to follow like this' --> 'It is a client's requirement to log all top exceptions all times and it should not be missed anytime' Is any point from above is applicable? Let's have some discussion please so that I get a broad and clear picture. 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
-
Let me start this discussion something like this: Obviously, we need to catch only specific exceptions and that too if it is required. But unfortunately this is not the case in my company. Here for each and every method (regardless it is required or not) you will find a single catch catching only the top Exception. Error logging is being done here and then sometimes it is being rethrown. I dont understand this scheme/practice and hence I discussed regarding it with my Developers, peers, and Manager(s) but most of them say something like these: --> 'It does not make any difference' --> 'We are doing in this same fashion everywhere from beginning' --> 'My TL asked me to follow like this' --> 'It is a client's requirement to log all top exceptions all times and it should not be missed anytime' Is any point from above is applicable? Let's have some discussion please so that I get a broad and clear picture. 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
Doesn't that go against the whole point of exception handling? try/catch is not equivalent of VB's On Error Goto. Also if the code breaks all the time then its hardly an 'exception' is it? And debugging must be a pain, as VS will break in catch block, not where the error occurred wont it? How many of these catch blocks are entered on regular operation? Exceptions aren't cheap to throw after all. Also is a try/catch block 'free' of any performance/memory penalty?
-
Doesn't that go against the whole point of exception handling? try/catch is not equivalent of VB's On Error Goto. Also if the code breaks all the time then its hardly an 'exception' is it? And debugging must be a pain, as VS will break in catch block, not where the error occurred wont it? How many of these catch blocks are entered on regular operation? Exceptions aren't cheap to throw after all. Also is a try/catch block 'free' of any performance/memory penalty?
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
-
Let me start this discussion something like this: Obviously, we need to catch only specific exceptions and that too if it is required. But unfortunately this is not the case in my company. Here for each and every method (regardless it is required or not) you will find a single catch catching only the top Exception. Error logging is being done here and then sometimes it is being rethrown. I dont understand this scheme/practice and hence I discussed regarding it with my Developers, peers, and Manager(s) but most of them say something like these: --> 'It does not make any difference' --> 'We are doing in this same fashion everywhere from beginning' --> 'My TL asked me to follow like this' --> 'It is a client's requirement to log all top exceptions all times and it should not be missed anytime' Is any point from above is applicable? Let's have some discussion please so that I get a broad and clear picture. 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
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. :)
-
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) :confused:. 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
-
Let me start this discussion something like this: Obviously, we need to catch only specific exceptions and that too if it is required. But unfortunately this is not the case in my company. Here for each and every method (regardless it is required or not) you will find a single catch catching only the top Exception. Error logging is being done here and then sometimes it is being rethrown. I dont understand this scheme/practice and hence I discussed regarding it with my Developers, peers, and Manager(s) but most of them say something like these: --> 'It does not make any difference' --> 'We are doing in this same fashion everywhere from beginning' --> 'My TL asked me to follow like this' --> 'It is a client's requirement to log all top exceptions all times and it should not be missed anytime' Is any point from above is applicable? Let's have some discussion please so that I get a broad and clear picture. 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
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.
-
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
-
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.
-
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
-
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 :-D
-
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 :-D
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
-
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) :confused:. 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. ;)
-
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.
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
doubleby0, you won't get aDivideByZeroException; you'll getdouble.Infinity.
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
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
doubleby0, you won't get aDivideByZeroException; you'll getdouble.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.
-
Doesn't that go against the whole point of exception handling? try/catch is not equivalent of VB's On Error Goto. Also if the code breaks all the time then its hardly an 'exception' is it? And debugging must be a pain, as VS will break in catch block, not where the error occurred wont it? How many of these catch blocks are entered on regular operation? Exceptions aren't cheap to throw after all. Also is a try/catch block 'free' of any performance/memory penalty?
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.
-
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) :confused:. 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
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.
-
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.
-
Let me start this discussion something like this: Obviously, we need to catch only specific exceptions and that too if it is required. But unfortunately this is not the case in my company. Here for each and every method (regardless it is required or not) you will find a single catch catching only the top Exception. Error logging is being done here and then sometimes it is being rethrown. I dont understand this scheme/practice and hence I discussed regarding it with my Developers, peers, and Manager(s) but most of them say something like these: --> 'It does not make any difference' --> 'We are doing in this same fashion everywhere from beginning' --> 'My TL asked me to follow like this' --> 'It is a client's requirement to log all top exceptions all times and it should not be missed anytime' Is any point from above is applicable? Let's have some discussion please so that I get a broad and clear picture. 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
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.
-
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.
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
throwwithout 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 theusing (...){...}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. -
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[^]
