I know it's evil, but I've seen swallowed exceptions in code written by a good programmer. So I'm wondering if this bad practice could have at least one positive point.
In other words, it is bad but why do good programmers, on rare occasions, use it?
try
{
//Some code
}
catch(Exception){}
Because sometime even good programmers make mistakes.
Either that or your opinion of a good programmer isn't the same as some (because a good programmer would have left some reasoning behind as to why the Exception was swallowed rather than something being done with the information).
Looking in my own code, I found a place in my logging code where, after failing to write to a file and failing to write to the event log, it swallows the error as there is no place left to report it. That's one example: there aren't many others.
Maybe because it won't catch but needs to be able to anyway. I can't think of any reason why to avoid some sort of handler, though.
I'd argue that a good programmer wouldn't do that without explaining it.
try
{
//Some code
}
catch(Exception e) {
// Explanation of why the exception is being swallowed.
}
I'm very unlikely to just silently swallow the base Exception class though, for any reason. I'd at least try to log the error.
Here's a concrete example that I just happened to run across today in my project. This is part of a JavaScript function that gets an XMLHttpRequest supported by the browser.
var XMLHttp = null;
if( window.XMLHttpRequest ) {
try {
XMLHttp = new XMLHttpRequest();
} catch(e) {} // we've already checked that this is safe.
}
else if( window.ActiveXObject ) {
...
}
Since the try is wrapped in an if...else if that checks the type of object to be created, it's safe to swallow the exception.
Yes, I use them quite often in exception cleanup code so that I don't mask the original exception.
For example, if your catch handler is trying to rollback a transaction and close a connection before rethowing the exception, there could be several reasons that the rollback/close itself could fail (since you're already in some busted up as a result of the original exception). So, may want to wrap the clean in a try block with an empty catch handler, basically saying "if something goes wrong in the cleanup, that's sort-of-OK, since we have bigger problems to report"
That's like asking why good people sometimes lie or oogle women?
No one is perfect.
I do it when an exception can't be handled effectively, it shouldn't impact the normal operation of the application, and/or it might represent a transient condition that is known but has little overall impact.
A simple example is logging. You don't need the app to blow up just because some logging information won't be saved to your backing store.
Another example might be where you have alternative means of handling the situations, like this:
private Boolean SomeMethod() {
Boolean isSuccessful = false;
try {
// call API function that throws one of several exceptions on failure.
isSuccessful = true;
} catch(Exception) { }
return isSuccessful;
}
In the above example, you may not have a fall back position, but you don't want it to filter up. This is ok for SHORT methods where the return value is only set to a successful status as the last step in the try block.
Unfortunately there are many calls that will throw exceptions instead of simply returning an error code or false condition in the result. I've also seen calls that throw more exceptions than what their documentation suggests which is one of the reasons why I'll put a catch all around them.
Even if it's just because nothing can be done with it, it'd still be nice to have comments. However, in ongoing custom/vertical apps with a stable team, sometimes those niceties go away if the reasons are at all obvious in the preceding code.
I won't say that I'm a good programmer but I have a good example why sometimes it is ok to swallow an exception, in my case it is part of the behaviour of the program.
example:
public void Load()
{
try
{
using (System.Xml.XmlReader stream = System.Xml.XmlReader.Create(fileName))
{
XmlSerializer xs =
new XmlSerializer(typeof(List<Contact.Contact>), types.ToArray());
List<Contact.Contact> data =
(List<Contact.Contact>)xs.Deserialize(stream);
this.Clear();
this.AddRange(data);
}
}
catch (System.IO.FileNotFoundException)
{
// do nothing; no file, new database
}
}
If XmlReader doesn't find a file it's ok, we just start over.
Without claiming to be a good programmer, I can at least explain why I sometimes catch and ignore. There are times when what I'm working on has to deal with an exception to compile, but is not the appropriate place to actually handle the exception. For instance, I was recently working in a program which parses a good deal of XML as one of its tasks. The actual XML parser opened a file, which generated an exception. The XML parser was not the best place to generate a dialog informing the user of bad file choice, though. Rather than get sidetracked with throwing the exception to the right place and handling it there, I silently caught the exception, leaving a comment and TODO (which my IDE tracks for me). This allowed me to compile and test without actually handling the exception. Sometimes, I will forget to take care of the TODO immediately, and the swallowed exception sticks around for a while.
I know this is not the best programming practice, and I certainly am not an expert programmer, but I think this is an example of why someone would want to do this.
Because sometimes the programmer has more knowledge than the compiler.
E.g. (in some fictional language where division by zero is regarded as an error):
try {
x = 1 / (a*a + 1);
} catch (DivisionByZeroException ex) {
// Cannot happen as a*a+1 is always positive
}
As some languages (e.g. Java) require to catch some/many/all exceptions, the compiler might complain while the programmer knows it is not possible. Sometimes the only way to get the compiler to shut up, is to explicitely swallow the exception.
In languages where the compiler doesn't complain, the empty catch usually is not written at all.
Edit: In practice, I would add an assert(false) in the catch block. If theoretically something cannot happen, it is a very serious problem when it does happen in practice ;)
Edit2: Java only requires to catch checked exceptions. Reworded my statement a bit.
at the very least write it to the output window
try
{ }
catch (exception ex)
{
System.Diagnostics.Debug.Writeline(" exception in method so and so : " ex.message);
}
I needed to do this once, because an old framework threw senseless exceptions in rare cases, which I didn't care about. I just needed it to run in the cases that worked.
I've done this when there some is some piece of code where regardless of it failing I want the program to continue. For example I've opened an optional log file and regardless of why it failed to work I want to continue the execution of the program.
Even so though, it's not good code as you can get exceptions like out of memory exceptions that ignoring makes no sense at all so even though i've done it, it's still not the best idea and I wouldn't admit to doing it in public. (oops...)
I've done it on programs that are intended to be run once to convert some data or something just to shut the compiler up. That's an entirely different situation to any form of production code though.
Basically there is no good reason for doing this other than lazyness.
How about when performing a progress-control update? I've updated the state variables used by a control's drawing routine, so the next time the control is drawn it will show the proper state. I then perform:
Try
MyControl.BeginInvoke(MyControlUpdateDelegate);
Catch Ex as Exception
End Try
If the control still exists, it will update. If it gets disposed, I won't care whether it gets updated or not. If the control isn't disposed, but can't be updated for some other reason, I still won't really care whether it gets updated or not because I'm not apt to be able to do anything sensible about it anyway.
Note that I could add "If Not MyControl.IsDisposed Then..." but that would just add work to the common case, and wouldn't prevent the exception from occurring if the control gets disposed during the BeginInvoke.
Due to the nature of the applications I write, virtually every exception I catch needs to be logged to a file.
Exceptions should never be eaten without any kind of rethrow, logging, or even a comment explaining why the exception was silently eaten. In my earlier years I would often eat exceptions if I knew that I was the only person working on the code. Of course when it came time to revisit the code I would forget what that exception really meant, and I'd have to retest everything to figure it out again.
I find silent exception handling works great for AJAX routines when alerting the user or handling the exception might bomb the code. Javascript can be so moody, its sometimes best to let it run its course knowing what it will do differently on errors.
try
{
var result1=ajax_loop(field1,type1,field2,user_id,xsrf);
}
finally
{
if (result1==0)
{
setTimeout( "redirect();", 3*1000 );
}
}
Notice there is no catch in this example. This is done purposely as any exception simply retries the AJAX when the result is zero.
I can only think of two kinds of cases where I've swallowed exceptions.
When closing files or database connections. If I get an error on the close, what am I going to do about it? I suppose I really should write out some sort of error message, but if I've already successfully read the data, it seems superfluous. It also seems like a very unlike event to happen.
More justified: Inside error handling. If I fail trying to write to the log file, where am I going to write the error that says that I couldn't write an error? In some contexts you could try to write to the screen, but depending on the app, that might be impossible, e.g. a background job with no screen; or just confusing to the user who won't be able to do anything about the error anyway.
Another poster mentioned cases where you know the exception is impossible, or at least, that for it to happen there would have to be major problems with your compiler or operating environment, like it did not add two numbers together correctly, in which case who says the exception is meaningful?
I heartily agree that in these rare cases where it is legitimate, you should include a comment to explain why the exception is irrelevant or impossible. But then, I'd say that anytime you write code that is potentially cryptic, you should include an explanatory comment.
Side note: Why is it that programmers routinely include comments like "x=x+1; // add 1 to x", like duh, I never would have digured that out without the comment, but then throw in really cryptic code with no explanation?
Because something has to be finished for a presentation the boss spontaneously does tomorrow and nothing is worse than the program crashing or showing a nasty error message, while when something is just not working he can always "talk around" that.
After the presentation, no one will improve those "quick fixed" parts of course. ;-)
I believe it's because of Java's checked exceptions. I personally hate them because of this because people tend to think they need exception-handling code everywhere. IMO in 99% of the code you should just be throwing your exception up the stack and not handling it. I'd be happy if the default signature for any new method created had 'throws Exception' in it so that I don't have to deal with exceptions unless I want to.
IMO you only need exception-handling in two places: 1. For resource cleanup like closing an input stream, database connection or deleting a file. 2. At the very top of the stack where you catch the exception and either log it or show it to the user.
There are places where you really don't need to do anything in the catch such as handling the InterruptedException from Thread.sleep() and there you should at least have a comment to make it clear that you really don't want anything to happen there.