Is there any valid reason to ever ignore a caught

Question

Wow, I just got back a huge project in C# from outsourced developers and while going through my code review my analysis tool revealed bunches of what it considered bad stuff. One of the more discouraging messages was:

Exceptions.DontSwallowErrorsCatchingNonspecificExceptionsRule  : 2106 defects 

The developers assure me they had good reason for all the empty catch blocks, that sometimes the try with empty catch blocks are just there to ignore useless exceptions and keep the application from crashing. I feel this is a cop out and complete BS. Some of the examples I actually looked up were database calls where the record was being saved to the database, and in this case, if an exception was ignored, the user would get back an okay prompt, think everything was okay, and continue on with their work. In reality, their work was never saved. I think this is absolutely the most horrible kind of error. In this case, they are completely wrong in throwing that code in a try with an empty catch block. But my question is, "Is this EVER acceptable in ANY situation?" I think not, but I've been known to be wrong.

Answer 1

I think the original question has been well answered, but one thing I'd like to add is that if you think these outsourced/contracted developers produced poor quality work, you should make sure the right people at your company know about it.

There may be a chance that it can be sent back for rework, that payment can be partially withheld, or that the same firm won't be used again. If your company uses contractors again, they might find a way to build quality requirements into the agreements, assuming that's not alredy there.

If this was in-house work, there would be consequences to the team/individual that produced substandard work. Maybe that would just mean that they have to work nights and weekends to fix it, but they'd be on the hook for it one way or another. The same should apply to contractors, possibly even more so.

Just be careful to explain your position professionally and with a focus on what's best for the company/product. You don't want it to seem like you're just complaining, or that you have some kind of political objection to outsourcing. Don't make it about you. Make it about cost, time to market, customer satisfaction, etc. You know, all those things that management types care about.

Answer 2

We have an application that does a lot of processing on behalf of other applications, where you insert some job (collection of config) into a database and the app will take it and run it at the appropriate time. We tend to swallow a lot of exceptions in that application, because even if Job1 dies in a horrifying fashion with a catastrophic error, we want the app to stay alive to take a stab at processing Job2.

Answer 3

The following only applies to languages that have checked exceptions, e.g. Java:

Sometimes a method throws a checked Exception that you know won't happen, e.g. some java APIs expect an encoding name as a string and throw a UnsupportedEncodingException if the given encoding isn't supported. But usually i pass a literal "UTF-8" that i know is supported so I could theoretically write an empty catch there.

Instead of doing that (empty catch) I usually throw a generic unchecked exception wrapping the "impossible" exception, or I even declare a class ImpossibleException that i throw. Because my theory about that error condition being impossible might be wrong and in that case I wouldn't want the exception to be swallowed.

Answer 4

The other case where you can be excused for catching and ignoring exceptions is when you're unit testing.

public void testSomething(){
    try{
        fooThatThrowsAnException(parameterThatCausesTheException);
        fail("The method didn't throw the exception that we expected it to");
    } catch(SomeException e){
        // do nothing, as we would expect this to happen, if the code works OK.
    }
}

Note that, even though the catch block does nothing, it explains why.

Having said this, more recent testing frameworks (Junit4 & TestNG) allow you to specify the exception that is expected - which leads to somthing like this...

@Test(expected = SomeException.class)
public void testSomething(){
    fooThatThrowsAnException(parameterThatCausesTheException);
    fail("The method didn't throw the exception that we expected it to");
}

Answer 5

It depends on the framework. Badly implemented frameworks might actually require this. I recall a hack in VB6 where there was no way to determine whether a collection contained an element. The only way was to try to retrieve the element and swallow the error.

Answer 6

I wonder about this specific one a lot.

Connection con = DriverManager.getConnection(url, "***", "***");

try {
    PreparedStatement pStmt = con.prepareStatement("your query here");

    ... // query the database and get the results
}
catch(ClassNotFoundException cnfe) {
    // real exception handling goes here
}
catch(SQLException sqle) {
    // real exception handling goes here
}
finally {
    try {
        con.close();
    }
    catch {
        // What do you do here?
    }
}

I never know what to do in that last catch in the finally block. I've never seen close() throw an exception before, and it's so unlikely that I don't worry about it. I just log the exception and move on.