Is it okay that I sometimes sink my exceptions?

Question

I have a best practices question. I realize this is subjective but wanted to ask people smarter than me if this is a common programming practice.

If you have a method of something NON-CRITICAL that you don't want to interfere with the important functioning of your application, is it common to use an error sink like this?

Try 
    'do stuff.  not important if it fails.

Catch ex as exception
    'sink.  do nothing.
End Try

If you were thinking of hiring me and you were reading some of my code and saw this...would you?

Seth

EDIT Wow! Thanks for your answers. I think the consensus is that should never be done or it should be very rare.

I thought I would give you context for this question. First, I have a strong familiarity with the Karl Sequin article and have followed that pattern for years.

But today on the project I was working on, I was working through the change list and was faced with the addition of a simple feature. (If you care to know...it is adding context menu support to a Rich Text Box.)

The attached note said, "if it takes longer than 15 mins...drop it."

So I am faced with adding what is a potentially useful feature but the don't really have the time to test that it won't break working features. For the record, our exception handler for this system DOES have a mechanism for handling and sinking or logging these errors. But what if I was working on a system that did not have a robust error handling system. Would it be okay to add this feature and if an error occurs...nothing is really lost.

That was my thinking. But I have taken your message to heart...that basically this is a bad idea.

Seth

Answer 1

My take is that you can do it, but you have to keep your eyes open and know why you're doing it. Think it through. In my shop we require that you have a comment explaining why you're eating the exception

Answer 2

It's almost never a good idea, but depending on how you do exceptions there are cases when it's applicable. For example:

try {
   Cache.refresh(myObject)
}
catch (NotInCacheException) {
   // If the object isn't already in the cache, then we don't have to worry 
   // about refreshing it. It's assumed that this is a common occurrence,
   // so we don't need to log it either; that ends up just spamming
   // the logs. So, just swallow it, silently.
}

(Note: Whether or not Cache.refresh() should be throwing a NotInCacheException at all is not at issue here; I'm just using this as a quick example).

Caveat though: you can only get away with this if you have a very specific exception that you're looking for. In your example, you're catching Exception, which is way too high-level (you'll be swallowing critical errors as others have mentioned).

Also: If you have a flexible logging framework, you might want to log this exception anyway for informational purposes. I'm thinking of Log4Net/Log4J in particular here: you can configure your logging (during runtime even) to select/ignore exceptions based on class and severity. Thus, you could silence these types of exceptions under normal circumstances, but log them if you wanted to do some poking around your application.

If you do decide to swallow the exception, my policy is to always have a code comment explaining why you did so; this marks it as not just another bad dev practice.

Answer 3

I do this only at annoying excpetions like closing a connection:

try {
   conn.close()
}
catch( Exception e ) {
}

because I'm already sure that I don't need it anymore.