Replace Empty Catch With Test

You have an empty catch clause.

Remove the try-catch clause and replace it with a conditional that tests for the exceptional circumstance.

For instance:

 void SendEmailNotification?() throws Exception
 {
     ...
 }

 void aMethod()
 {
     ...
     try{
         SendEmailNotification?();
     }catch(Exception e){
     }
     ...
 }

Becomes:

 bool canSendEmailNotification() // never throws
 {
     ... // Test for the ability to send email
 }

 void sendEmailNotification() throws Exception
 {
     ...
 }

 void aMethod() throws Exception
 {
     ...
     if(canSendEmailNotification())
         sendEmailNotification();
     ...
 }

Motivation An empty catch causes all exceptions of a particular type to be ignored regardless of what actually caused them. Exceptions by their very nature signal the occurrence of the unexpected. When we create an empty catch clause we are saying that we know that sometimes the operation can fail and if it does we don't care. But we can only safely make that statement about circumstances that we can anticipate. An empty catch clause doesn't discriminate between anticipated failure modes and unanticipated failure modes -- it ignores them all.

We can almost always write a method that tests whether some other method will throw if it is called. Such a method enumerates the failure modes that we understand and can safely ignore -- assuming that we can do without the services of the other method. Such a testing method gives us a way to clearly define the pre-conditions for the method that does the work. It also allows the computer to skip all the work of trying an operation, getting part way through, failing, and then unwinding the stack. Simply by checking to see whether the operation is expected to succeed in the first place.

Contraindications Don't do this when all of the following hold.

• You really do want to ignore all failures.
• Whether or not it succeeds can change between the call to the canDoSomething() method and the doSomething() method.

Under these circumstances, ReplaceEmptyCatchWithTest will introduce a race condition.

Mechanics:

• Create a new method. Name it after the method to be protected and prefix it with the word "can".
• Examine the existing method and devise a sequence of tests that define its pre-conditions.
• Code these tests into the new method. Put the ones that are expected to fail most frequently first and use shortcut logic to cause the method to return false as soon as one of the tests fail.
• Compile after each test is added.
• Wrap the body of the try block with a conditional that calls the new testing method.
• Compile and test.
• Remove the empty catch clause. If no catch or finally clauses remain remove the try construct (leaving its contents) as well. Repair indentation.
• Compile and test.

---

---

One place where this Eiffel-style of calling can break down in Java is when you have a bunch of threads running around: something important can change between your test(s) and the actual invocation. Still quite OK for some cases though. I think there's also a case to be made for the "assume success" style of programming where exceptions are just tossed off to where they can be handled and the "doing" code ignores them. -- LukeGorrie

Right, that's what ThrowDontCatch is all about I think, this is only for those cases when you are causing a side-effect that is secondary to the main thrust of what you are trying to do and you don't want exceptions that you consider to be unimportant to prevent your main-line code from executing. If the proper way to handle an exception is to just ignore it then you ought to find a way to prevent it from happening in the first place -- even if that means not calling the code that throws it at all. We know that there aren't any bad side-effects to not calling the code because we were already prepared to ignore it if it failed. -- PhilGoodwin

In Java, on most file I/O objects #close can throw, so is there an alternative to the following, or is it just a LibrarySmell??

  final FileInputStream in = new FileInputStream(foo);

  try {
    slurp(in);
  } finally {
    try {
      in.close();
    } catch(IOException smelly) {
      // And I'm supposed to do what with this exception?
      log.warn("close failed", smelly);
    }
  }

---

[CategoryRefactoring][CategoryException]