Exceptions are complicated, so it’s no surprise that they can be misused. A “smell” describes potential problems; whether it’s really a problem is your judgment. Let’s look at smells you may find with exceptions.
Empty Exception Handlers
Java has the notion of “checked” exceptions, that must either be handled by a method or explicitly declared by it. Programmers sometimes use empty handlers just to shut up the compiler complaints.
try {
some code
} catch (SomeException e) {
}
There – no more complaints!
The problem, of course, is that many exceptions tell you about a real problem. An empty handler papers over the problem, but doesn’t solve it.
There are cases where an empty handler is correct; you might want a comment on those.
Unreleased Resources
Many resources come in limited supplies; if you don’t release them when you’re done with them, you risk running out. Typical examples include file handles, database connections, and network connections.
If possible, I like to have the same routine both grab and release the resource. (Otherwise, you have to go find the routine that will release, and then make sure every flow of control guarantees it gets called.)
The finally clause is designed for this: if you take a resource, you can use the finally clause to make sure it gets released.
Not this:
file = open(“something.txt”) // do the work file.close()
More like this:
try {
file = open(“something.txt”)
// do the work (might throw)
} catch (IOException e) {
// handle it
} finally {
// close the file
}
Depending on your language, exceptions aren’t the only way to ensure this: C++ has the “Resource Acquisition is Initialization” pattern; C# has the using statement; Swift has the defer statement. (See References.)
Ignoring Exceptions Thrown By Handlers
The code in your handler also might throw an exception, and you need to handle that too. For example, Java’s reader classes can throw an exception on the close() statement. You need to handle that exception too. In this case, there’s pretty much nothing you can do, so you might have an intentionally empty handler for it.
In other cases, you need a handler that does some actual work.
Not this:
try {
// …
} catch () {
…
} finally {
if (file != nil) { file.close()}
}
But this:
try {
// …
} catch() {
…
} finally {
try {
if file != nil try {
file.close()
} catch (IOException e) {
// can’t do anything, so catch & ignore
}
}
(I wouldn’t bother with that comment in real code.)
Improperly Nested Resources
If you have two or more resources, you have to be extra careful. Remember, your goal is to allocate each resource, use it, and make sure it’s released when you’re done – no matter what (as long as your program keeps running:)
I’ve seen an example like this any number of times:
:
try {
fileReader = new FileReader(filename)
reader = new BufferedReader(fileReader)
:
} catch (IOException e) {
// whatever
} finally {
try {
if reader != null { reader.close(); }
if fileReader != null { fileReader.close(); }
} catch (IOException e) {
// ignored
}
}
Do you see the flaw? If closing reader throws an exception, then fileReader is never closed.
This would be a better finally clause:
:
} finally {
try { if reader != null {reader.close();} }
catch (IOException ignored) {}
try { if fileReader != null { fileReader.close(); }
catch (IOException ignored) {}
}
Undifferentiated Exceptions
If your handler code looks as below, you have an undifferentiated exception: you treat it as an ancestor type to catch it, but you peel it back to the lower type to handle it.
catch (Exception e) {
if (e is Subtype1) {
let e1 = (Subtype1) e
// handle subtype1
} else if (e is Subtype2) {
let e2 = (Subtype2) e
// handle subtype2
} else …
// …
} else {
// handle other Exceptions
}
}
Replace that with separate catch clauses:
catch (Subtype e1) {
// handle subtype1
} catch (Subtype e2) {
// handle subtype2
} catch // etc.
} catch (Exception e) {
// handle other Exceptions
}
Why does this happen?
It’s typically just caused by sloppy coding (especially if it’s with the builtin Exception types) – we just weren’t thinking carefully.
Single Application-Specific Exception
In some systems, every exception that gets caught is immediately wrapped in a standard exception, call it MyException(). That is, the original exception becomes its an inner exception.
To figure out how to handle it, you usually have to peel the exception apart and look at the inner exception (or even its inner exception). Or worse, you create an error message and look for certain substrings to know which case applies.
This is a missed opportunity. The original catch location has information that it either buries in the inner exception, or loses entirely.
Instead, have a family of application-specific exceptions (with a common ancestor if that’s helpful). When you catch an exception, translate it to application terms. For example, a FileNotFoundException is important. But knowing that it’s the configuration file that’s not found can be even more helpful. You can have a MyConfigFileNotFound() exception, with information specific to that.
So, rather than a single application exception, have a set of them to differentiate key situations.
Conclusion
We’ve looked at several exception code smells:
- Empty exception handlers
- Unreleased resources
- Ignoring exceptions thrown by handlers
- Improperly nested resources
- Undifferentiated exceptions
- Single application-specific exception
None of these are hard to recognize, once you start paying attention, and most of them have simple solutions.
References
“The defer keyword in Swift: try/finally done right“, by Paul Hudson. Retrieved 2022-01-17.
“Resource acquisition is initialization”, Wikipedia. Retrieved 2022-01-17.
“Spooky Action at a Distance: Exceptions“, by Bill Wake. Retrieved 2022-01-17.
“using statement (C# Reference)“, Microsoft. Retrieved 2022-01-17.