Month: December 2016

Something I wrote seven years ago and I’m publishing now to see if learned anything since then.

This is part of a series where I review common wisdom about Java error handling. The series is in no particular order, but the first installment explains my categorization method.

Clean up with finally
Truth: high
Importance: high

Joshua Bloch explains the reasoning behind this in Effective Java as “strive for failure atomicity.” Whatever happens, clean up after yourself. Checked exceptions give us one of their rare benefits by just maybe reminding us to write finally blocks.

One way not to write a finally block, however, is like this:

try {
	connection = DriverManager.getConnection("stooges");
	// Snip other database code
} catch (SQLException e) {
	throw new RuntimeException(e);
} finally {
	try {
		// Bad. Do not do this.
		connection.close();
	} catch (SQLException e) {
		throw new RuntimeException(e);
	}
}

If opening the connection throws SQLException, closing the connection throws NullPointer and obscures the original cause. Usually, people suggest wrapping a conditional around the connection.close() call to check for null, but that gets ugly fast and is easy to forget. Instead, follow the next rule.

Make try blocks as small as possible
Truth: high
Importance: medium

Consider this code that obscures which file could not open:

try {
	// Smelly. Don't do this.
	curly = new FileReader("Curly");
	shemp = new FileReader("Shemp");
} catch (FileNotFoundException e) {
	throw new RuntimeException("Whoopwoopwoopwoop", e);
} finally {
	// The close method translates IOException
	// to RuntimeException
	if (curly != null) {
		close(curly);
	}
	if (shemp != null) {
		close(shemp);
	}
}

We programmers wrap multiple lines in the same handler because we are lazy, but like the hare napping during the race, that laziness hurts us in the end; it forces us to reason much more carefully about the application state during cleanup.

Instead, handle the exception as close to its cause as possible:

try {
	curly = new FileReader("Curly");
} catch (FileNotFoundException e) {
	throw new RuntimeException("Missing: Curly", e);
}
try {
	shemp = new FileReader("Shemp");
} catch (FileNotFoundException e) {
	throw new RuntimeException("Missing: Shemp", e);
} finally {
	close(curly);
}
close(shemp);

The benefits of the shrunken try block may not be obvious from this tiny example, but consider how you can now factor out the file opening blocks to a method that simply throws an unchecked exception for missing files. Once done, this code simplifies down to:

curly = open("Curly");
try {
	shemp = open("Shemp");
} finally {
	close(curly);
}
close(shemp);

Note that all these examples lose the original exception when the close method throws an exception. Most applications can afford that minimal risk, but if you believe it likely that your cleanup code will throw further exceptions, a log and re-throw might be appropriate.

Do not rely on getCause()
Truth: high
Importance: high

Peeking at an exception’s cause is equivalent to using something’s privates or parsing the string representation to find a field value. It makes code brittle; moreover, you can only test it by forcing errors in your collaborators.

If you find that some library forces you to do this, consider avoiding that function completely; if you still have no way around it, adorn your code liberally with “XXX” comments, and test as best you can.

Do not catch top-level exceptions
Truth: low
Importance: low

Top-level exception classes like Exception live close to the root of the exception hierarchy. The argument against catching these says that you should avoid it because the specific lower type, such as FileNotFound, traditionally conveys information necessary to handling the exception, so by catching the top-level exception, your handler is dealing with an unknown error, which it probably knows little about.

Actually, the advice should say “do not try to recover from top-level exceptions.” Catching top-level exceptions is not fundamentally wrong or even bad, but because you do not really know how severe the exception was, you should usually do no more than report and organize a crash.

Something I wrote seven years ago. Did I learn anything in that time?

This is part of a series where I review common wisdom about Java error handling. The series is in no particular order, but the first installment explains my categorization method.

Do not use empty catch blocks
Truth: high
Importance: high

This is the most obvious of the category of exception handling rules that address how to avoid losing error information. The usual example for when you might justifiably ignore an exception goes like so:

static void closeQuietly(Closeable closeable) {
  // Smelly. Do not do this.
  try {
    closeable.close();
  } catch (IOException e) {
    // I've already done what I needed, so I don't care
  }
}

Yes, the comment makes this better than the completely empty catch block, but that is like saying that heroin is fine because you wear long sleeves.

Very rarely, suppressing an exception actually is the right thing to do, but never unless you absolutely know why it happened. Do you know when close() throws IOException? I thought not.

Do not catch and return null
Truth: medium
Importance: medium

Catching and returning null is a minor variation on the exception-suppression theme. Consider the Stooges class, which contains this method:

public String poke(String stooge)
              throws StoogeNotFoundException {
  if (stooges.contains(stooge)) {
    return "Woopwoopwoopwoop";
  } else {
    throw new StoogeNotFoundException("Wise guy, eh");
  }
}

Suppose you want to write another method that checks a Stooge’s reaction to a poke, but Stooges gives you no isStooge method. Instead, it forces you to write this:

static String getStoogeReaction(Stooges stooges, String name) {
  try {
    return stooges.poke(name);
  } catch (StoogeNotFoundException e) {
    return null;
  }
}

If you have to use an API that uses exceptions for flow control, something like this might be your best option, but never write an API that makes your clients do it.

Log only once
Truth: medium
Importance: low

You can also state this rule as “Log or re-throw, not both.” Redundant logging is certainly impolite to those maintaining your application, but hardly the worst you could do. You might log and re-throw for legitimate reasons:

• Your framework swallows exceptions you throw at it
• Your application logs in a specific location, different from your container
• This is part of a larger application, and you worry that your clients might ignore the exception

Do not log unless you need to, but if in doubt, log it.

Always keep the cause when chaining exceptions
Truth: high
Importance: high

Only the very naive intentionally do this, but it is easy to do accidentally, and a very easy way to lose the information about what went wrong.

In 2016, I still think exception chaining is a very important feature and I’ve been surprised by how many mainstream languages lack exception chaining.