Good vs Bad
-
Sample 1: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { continue; } } Sample 2: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { // Left blank } } Question: In sample 1 we have "continue;" in catch block and in Sample 2 we have NOT written anything. Are they doing the same thing? If yes, which one is better practice?
-
Sample 1: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { continue; } } Sample 2: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { // Left blank } } Question: In sample 1 we have "continue;" in catch block and in Sample 2 we have NOT written anything. Are they doing the same thing? If yes, which one is better practice?
They are both various shades of bad practice. You're just catching ANY exception here, without actually bothering to do anything about it. In both cases, the behaviour is the same - the loop keeps on iterating, but the reality is that you have just consumed something that has gone wrong. That is bad practice, so I would urge you to do neither case. Exceptions are thrown for a reason. Only catch them if you intend to do something with them, such as logging or you have logic in there to mitigate the effect, e.g. you might have some retry functionality that you need to use. Ignoring exceptions is the wrong thing to do.
-
They are both various shades of bad practice. You're just catching ANY exception here, without actually bothering to do anything about it. In both cases, the behaviour is the same - the loop keeps on iterating, but the reality is that you have just consumed something that has gone wrong. That is bad practice, so I would urge you to do neither case. Exceptions are thrown for a reason. Only catch them if you intend to do something with them, such as logging or you have logic in there to mitigate the effect, e.g. you might have some retry functionality that you need to use. Ignoring exceptions is the wrong thing to do.
Thanks for your reply. I am catching InvalidOperationException instead of general exception (will update the question). However, we have plans to get logging in catch block instead of continue or blank. The only reason I wanted to ask this, because I wanted to know if writing continue is bad instead of leaving it blank. --> Our aim was to not do anything for the iteration where an exception is thrown.
-
They are both various shades of bad practice. You're just catching ANY exception here, without actually bothering to do anything about it. In both cases, the behaviour is the same - the loop keeps on iterating, but the reality is that you have just consumed something that has gone wrong. That is bad practice, so I would urge you to do neither case. Exceptions are thrown for a reason. Only catch them if you intend to do something with them, such as logging or you have logic in there to mitigate the effect, e.g. you might have some retry functionality that you need to use. Ignoring exceptions is the wrong thing to do.
+5 Small addition: If you catch Exceptions to log them or to do anything else that doesn't include mitigating the error, re-throw the exeption at the end of the catch-Block, either with "throw;" to re-throw the exact same Exception or by throwing a custom Exception and providing the original Exception as an InnerException to the constructor. See here: Anti-Patterns and the Solutions[^] especially the part "Error Hiding Anti-Pattern".
-
Thanks for your reply. I am catching InvalidOperationException instead of general exception (will update the question). However, we have plans to get logging in catch block instead of continue or blank. The only reason I wanted to ask this, because I wanted to know if writing continue is bad instead of leaving it blank. --> Our aim was to not do anything for the iteration where an exception is thrown.
Leaving it blank is bad: it "swallows" the exception, so that nobody ever knows it has happened. So the underlying problem never gets fixed until it has reached massive proportions. If you are going to add logging later, then re-throw the exception (so you know it occurred in development) and add a TODO to the code to remind you to fix it:
catch(InvalidOperationException ex)
{
// TODO Log exception
throw;
}Adding the TODO means it comes up in the Visual Studio Task List[^] automatically.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
-
Leaving it blank is bad: it "swallows" the exception, so that nobody ever knows it has happened. So the underlying problem never gets fixed until it has reached massive proportions. If you are going to add logging later, then re-throw the exception (so you know it occurred in development) and add a TODO to the code to remind you to fix it:
catch(InvalidOperationException ex)
{
// TODO Log exception
throw;
}Adding the TODO means it comes up in the Visual Studio Task List[^] automatically.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
Thanks Everyone for the information.
-
Sample 1: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { continue; } } Sample 2: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { // Left blank } } Question: In sample 1 we have "continue;" in catch block and in Sample 2 we have NOT written anything. Are they doing the same thing? If yes, which one is better practice?
I would think hard before even trying to put a try ... catch block "inside" a foreach loop. Exception handling can be expensive. If you were looping "thousands" of times, that would potentially mean "thousands" of exceptions. An exception "inside" a loop, generally doesn't happen just once because the data / processing requirements are usually similar. (IMO) One generally wraps a loop "inside" a try ... catch block; not the other way around; in which case, to "continue or not" is a non-issue. Repeated exceptions in a loop are a symptom of bigger problems that should be solved; not just "handled". Exceptions are for "exceptions"; not general error handling.
-
Sample 1: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { continue; } } Sample 2: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { // Left blank } } Question: In sample 1 we have "continue;" in catch block and in Sample 2 we have NOT written anything. Are they doing the same thing? If yes, which one is better practice?
In catch, continue will automatically work so no need to add the keyword there.
hi
-
In catch, continue will automatically work so no need to add the keyword there.
hi
-
In catch, continue will automatically work so no need to add the keyword there.
hi
-
Sample 1: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { continue; } } Sample 2: foreach(int i in integerList) { try { // Do something which might throw exception } catch(InvalidOperationException) { // Left blank } } Question: In sample 1 we have "continue;" in catch block and in Sample 2 we have NOT written anything. Are they doing the same thing? If yes, which one is better practice?
Apart from what ever has been answered it is a good practice to catch the general exception also. In both the catch blocks you have to do 2 things log an exception as well as Display the exception to the user. But my suggestion is if you can avoid exceptions that would be even better so have proper checks where ever required like null check etc etc.
