Empty Catch Clause

	try {
		someObject.something();
	} catch(Exception e) {
		// should never happen
	}

Oh, the one designing the interface of the something() method screwed up? Well, try to get it fixed in the method first. If this is not possible, just let the exception pass through (ThrowDontCatch). If this is not possible (why shouldn't it?), subclass the class of someObject, and provide a wrapper method, which contains the dreaded EmptyCatchClause OnceAndOnlyOnce.

Although it would be nice if every exception were valid, that's not always the case. Sometimes a piece of code throws a checked exception that you know can't occur. Perhaps it's unique to your situation, or you're wrapping a bit of poorly-designed code. In any case, you don't want to propagate the exception (because then someone else will have to spend cycles dealing with it) and there's no need to handle the exception. In these cases, I like to use an assertion. For example, in a single-threaded app, InterruptedException can't occur:

 public void sleep(long delayInMilliseconds) {
	try {
	Thread.sleep(delayInMillisecond);
	}
	catch (InterruptedException e) {
	Assert.impossibleException(e);
	}
 }

What about this instead?

 catch ( InterruptedException iE ) {
	// somebody interrupted the thread so I could tidy up and stop, maybe I should 
	// have allowed the exception to propagate, or stop and reset the interrupted flag
	// rather than asserting the impossible.
 }

Contrast it to this:

	try {
		someObject.something();
	} catch(Exception e) {
		// don't care if it happens
	}

Note, it's been pointed out to me that expressing this as

	try {
		someObject.something();
	} catch(Exception unimportant) {}

I would say definitely don't have an empty catch() for the base classes like Throwable or Exception. The someObject.something() call may call many other methods and if one of those throws an Exception, it will be silently caught and it is very frustrating trying to track it down. In these cases I catch an more specific exception (like InterruptedException) or at least add an e.printStackTrace() as a temporary measure. -- RobNielsen?

I say it is valid to have an empty catch clause: The unchangeable 3rd party package you're using throws an exception for a condition you expect to happen during the normal course of execution of your program, and they didn't think to provide a way to give you a return code instead.

For example, I've worked on several applications that would INSERT a record and watch for a "duplicate index entry" error to handle the 2% of the cases where there was a conflict. Now one would rather check for that particular error and handle it as a return code, so that alternate business logic can be executed, but I've yet to see a 3rd party database interface that uses exceptions and also gives you that level of control. (On one project I wrote such a package - in a layer of C++ wrapper classes over ODBC.) So, if you want the main line of your business logic to be rational, you'll want to catch the errant exception and turn it into a special return code. The catch block can still end up empty because you might also be using the defensive coding style of setting the return code to "fail" before the try block, and only setting it to "success" at the end of the successful operation. (IE: at the end of the try block.) -- JeffGrigg

Can somebody elaborate on "defensive coding", perhaps with an example?

Example:

  successFlag = FALSE;
  try {
	if (doSomethingAndReturnResult() == resultWeLike)
	successFlag = TRUE;
  } catch(ExceptionTypeWeExpect e) {
	// 'successFlag' is already FALSE.
  }
  return successFlag;

I think it's the "Can't Ever Happen" comment that represents the smell here. If something "can't ever happen", about five minutes after it gets into production you'll have a long and exhausting debugging session when it does. In such a case, you should throw a runtime exception; after all, one of the fundamental assumptions in the system has just been proven wrong.

In the case where there's an empty catch block because you've recognized it can happen, and your way of handling it is to do nothing (as in Jeff's defensive coding technique), it's not a smell. The possibility has been recognized, and the exceptional condition has been dealt with correctly (by ignoring it). -- RobertWatkins

Ooh, ooh, I have one of these. Consider the case of some session management code that periodically expires sessions.

	try
	{
		for (Iterator i = iterator(); i.hasNext();)
		closeSession( (Session)i.next() );
	}
	catch (ConcurrentModificationException cme) {}

	try
	{
		for (Iterator i = iterator(); i.hasNext();)
		closeSession( (Session)i.next() );
	}
	catch (ConcurrentModificationException ignored) {}

I just love this one - we used to have a programmer working with us who did this so much, we now use his name to describe this "technique".

Another variation (which seems to be caused by programmers inserting print statements everywhere during frantic debugging sessions) is wrapping every line of code with

	try {
		... something ...
	} catch (Exception e) {
		e.printStackTrace (System.err);
		throw e;
	}

That's just silly. -- rw

One solution I prefer is to use RuntimeException. It makes for cleaner code and bugs show up very prominently when you hit an uncaught exception. -- TimBurns

I'm disappointed that RuntimeException doesn't have a constructor taking an exception. It would be nice to promote a checked exception to an unchecked one for cases like these. Fortunately, there are a number of implementations of NestedRuntimeExceptions available. -- MarkAddleman

No big deal, just write your own WrappedRuntimeException that can hold the real exception inside. -- KyleCordes

Sun has recognized this as a flaw - in Merlin (Java 1.4), there will be a new method on the Exception class allowing you to chain them up. I don't know if they'll extend the constructors, but it does make sense -- rw

Yes, Sun did add "cause" arguments to some constructors.

I actually vacillate about this. OnceAndOnlyOnce drives me to want to use Java's checked exceptions as a poor man's extended message syntax.

Without using exceptions, I would write something like

 Row row = retrieveDbRow(key);
 if (row == null) {
	do something
 }

 try {
	Row row = retrieveDbRow(key);
 } catch (NotFoundException e) {
	do something
 }

The upshot of this style is that there will be cases with EmptyCatchClauses:

 try {
	Row row = retrieveDbRow(key);
 } catch (NotFoundException e) {
 }

-- MarkAddleman

I think this is a misunderstanding of the concept of a CodeSmell. From CodeSmells: A perfectly good idiom may be considered a CodeSmell because it's often misused, or because there's a simpler alternative that works in most cases. Calling something a CodeSmell is not an attack; it's simply a sign that a closer look is warranted. -- AlexChurchill

With the pending release of Java 1.4, and the addition of an assert() facility built into the language (as opposed to those hacks which all of us have been using so long), I would say that the correct coding of the initial example is as follows:

	try {
		someObject.something();
	} catch(Exception e) {
		// should never happen
		assert(false);
	}

Remember that assertions can be switched off and then we would never get this exception. It would be better to log so that program does not crash but the administrator/user/developer can check the logs for errors. -- VhIndukumar

If the code is not already tied to Java 1.4, it might be excessive to make it so just for the benefit of something that can never happen.

	try
	{
		for (Iterator i = iterator(); i.hasNext();)
		closeSession( (Session)i.next() );
	}
	catch (ConcurrentModificationException cme) {}

	for (Iterator i = iterator(); i.hasNext();)
	{
		try
		{
		closeSession( (Session)i.next() );
		}
		catch (ConcurrentModificationException cme) {}
	}

Maybe I misunderstand, but I'm not sure why one would want to stop closing ALL sessions because one happened to throw an expired exception. Of course, if this exception is normal and expected, I don't think I would rely on an exception to control program flow - instead, I would probably do something like this:

	...
		void closeSession( Session ctx )
		{
		if ( ! ctx.isExpired() )
			ctx.close();
		}
	...

	for (Iterator i = iterator(); i.hasNext();)
	{
		try
		{
		closeSession( (Session)i.next() );
		}
		catch ( Throwable e )
		{
		Logger.printException( e ); // print error and continue
		}
	}

I guess I was not being very clear. The ConcurrentModificationException will only happen if someone is trying to add or delete a session while you are going through the list checking for expired sessions. Handling that case seems useless because another aging pass will be made shortly. Also, the iterator is invalid after the exception, so moving the try inside the loop would not work.

The first example was snipped too much. Here is the full method:

	private void ageSessions()
	{
	try
	{
		for (Iterator i = iterator(); i.hasNext();)
		{
		Session session = (Session)i.next();
		if (!session.isActive())
			session.close();
		}
	}
	catch (ConcurrentModificationException cme)
	{
		The iterator may bail out if someone tries to add or delete
		a session while we are iterating (fail-fast). When this
		happens, we don't need to do anything. If we missed any
		sessions, they will get expired next time around.
	}
	}

Hmmm... in that case, have you considered using an internal iterator and synchronizing on that so that a ConcurrentModificationException? is not possible?

	sessions.enum( new SessionBlock() {
	public void run( Session each ) {
		if ( each.isActive() )
		each.close(); } } );

I just read the excellent explanation of InternalIterator and BlocksInJava. This is a very nice way to avoid the empty catch clause. The only remaining concern is that synchronizing the iterator holds off any opens or closes of new sessions for the duration of the iteration. -- MichaelBanks

I'd think the lock is exactly the problem, at least from one point of view. The first example would be low-priority, pre-emptible cleanup. The locking one might still be low-priority, but it's pre-emptive. Which one you'd choose would depend on whether accumulating garbage has enough consequences that you'd be willing to compromise the main path in order to clean it up. In the case of session management, I'd probably choose the former, but there might be resource leak problem as you scale (cleanup never happens because sessions come in faster than you can expire them), so you might need to move to the second or a hybrid model. --ChrisMellon

  FileInputStream fis;
  try {
	fis = new FileInputStream("foo.txt");
	//  do stuff
  } catch (IOException e) {
	// warn user
  } finally {
	if (fis != null) {
	try {
	fis.close();
	} catch (IOException e) {}  // EmptyCatchClause?
	}
  }

Well, you could create an InputSteamAdapter that hides the try...catch around the #close call. I still wouldn't keep it empty. I would probably write a log entry just in case. -- RobertDiFalco

I'd write a log entry because I want to know under what circumstances closing a file can fail...

	try {
		someObject.something();
	} catch(Exception canNeverHappen) {
		throw new ImpossibleRuntimeException(canNeverHappen);
	}

-- NeilSwingler

I like this a lot because I have a strong feeling that, as something() changes, these will start popping up for reasons we couldn't have imagined at first writing. -- DustinAleksiuk

It is even worse in C++:

	try {
		someObject.something();
	} 
	catch ( ... ) {}

Note that PMD - http://pmd.sf.net/ - can find this problem.

Whenever its been said "it can never happen", I have suggested putting a System.exit(1) - people aren't so certain then ;) -- ChanningWalton

 try {
     a.mustThrowMyException();
     fail("My exception should be thrown");
 } catch (MyException e) {
     // This SHOULD happen
 }

Thank you for coming up with the only sane example of an empty Java catch block on this page. Sorry folks, if you catch one of these other examples you could at least log it. What? You say you don't have one... 'nother smell I suppose.

Eh, how about:

 try {
    a.mustThrowMyException();
    fail("My exception should be thrown");
 } catch (MyException expected) {
 }

This page mirrored in JavaIdioms as of April 29, 2006