Let's say I am designing an API for storing passwords. According to Effective Java it is a good idea to use throw clauses to show checked exceptions, however by having a throw clause that throws a SQLException, which is a checked exception, then I am revealing the underlying implementation details of my API and thus I will be unable to change the implementation details of my API at a later stage. One of the pros to throwing a checked exception is that the programmer who uses the API should be able to handle the exception in a manner of their choosing. Which of these two methods should I choose to do, add a throw clause which reveals the implementation or hide it or use a different approach?
5 Answers
Your motivation is correct for not "leaking" SQLException to the users of your class.
The fact that you're using SQL could be considered an implementation detail. You may even swap SQL persistence for say, an in-memory one at a later time, and this change shouldn't impact the users of your class.
If you are inclined to use checked exceptions, I would define your own exception type (say, PasswordStoreException -- just an example name). You can use it to wrap the original exception that was thrown, e.g.:
try { // do whatever } catch (SQLException ex) { throw new PasswordStoreException(ex); } 
Comments
- It is today considered bad design for an API to declare checked exceptions. If you have ever used such an API, you should already know why.
- In any case your API should never throw (let alone declare) exceptions belonging to other APIs. If you do that, you hang a completely unrelated dependency on your client's back. The only "exception" to this rule are JDK's standard exceptions like NPE, ISE and the like.
1 Comment
akuhn
+1 for mentioning that it’s bad style to declare checked exceptions!
Catch the SQLException, and wrap it into your own exception:
try { // JDBC code } catch (SQLException e) { throw new MyException("Error persisting the secret", e); // choose a better name, of course } Whether this exception should be a checked exception or a runtime exception depends on what the caller can do about it. If it's recoverable (which, I think, is not the case here), it should be a checked exception. If it's not recoverable (the caller can just display an error message), then it should be a runtime exception.
If it's a checked exception, you have no choice; the exception MUST be declared in the throws clause of the method.
5 Comments
Marko Topolnik
(not your dv) JB, I think we should stop reiterating that same old story with "recoverable" exceptions. Everyone who has used Java APIs for more than a week has come to realize that it is not up to the API designer to decide up front whether an exception is recoverable. Take reflection: in 95% of use cases an exception is a sign of a programming error, non-recoverable. Take IO: a
FileNotFoundException is at least 50% of time non-recoverable: we simply need that file to be there for our app to work. It goes on for every single exception out there.
JB Nizet
The problem is: if it could be recoverable (50%), should we throw a checked or a runtime exception? Java has often chosen to use checked exceptions in this case. It starts doing the inverse choice in some APIs (JPA for example), and I often prefer runtime exceptions in this case. However, there are situations where exceptions should be handled by the caller, especially if the caller and the callee are part of the same application. As I said in my answer, in this specific case, the exception should be a runtime exception.
Marko Topolnik
You said that well: the only legitimate case for checked exceptions is not public API, but within-application logic -- where you know for sure the character of that exception. Older APIs in Java reflect the original hope of language designers that checked exceptions will make safer code; they have been proven wrong by Java practice and checked exceptions are generally considered a failed experiment. Newer APis invariably choose runtime exceptions; there was a transitional period when they were actually bragging about it :)
JB Nizet
I'm not sure I've completely made up my mind about that. One of the advantages of checked exceptions, even in an API, is that it forces me to think about situations that I could deal with, and that I could have missed, even after reading the documentation. The main drawback of checked exceptions, IMO, is that newbies invariably deal with them by swallowing them, which makes the situation worse than with runtime exceptions.
Marko Topolnik
Agreed on that main drawback :) However, for me personally the main drawback is that they point a gun at me with "Handle or declare!" and I want neither. Every time you need to write
catch (Exception e) { throw new RuntimeException(e); the checked exceptions failed for you. They are just not the mechanism that will force you to write better code. Maybe sometime, somewhere they'll help you not miss an exc the first time you write that code (they are incredibly easy to find and fix through testing), but they'll shoot you 100 times in between.As is, it is always a good idea to throw your own exception checked/unchecked. But before that, try to fix the underlying exception if possible. I always prefer the below way,
try { // JDBC code } catch (SQLException e) { // try to solve the exception at API level bollean solvable = trySolveException(e); if (!solvable) { // Alert the admin, or log the error statement for further debugging. mailClient.sendMailToDBAdmin("Some issue storing password", e); // or LOG.severe("some issue in storing password " + e.toString); throw MyException("A request/logstatement is logged on your behalf regarding the exception", e); } LOG.info("The exception is solved"); } finally { // don't forget to free your resources - to avoid garbage and memory leaks, incase you have solved the issue in trySolveException(e). } So,
1) You don't expose the SRQException directly, but you throw your own version of the exception.
2) You tried to solve the exception once and if not you alerted somehow - through a mail or a log statement.
3) Finally, you ve released all the resources if you succeed in solving the exception.
The finally clause can be avoided if you use the new Java7's try with resource close option.
For whether to throw checked or unchecked exception, I will give you an example
1) Say an exceptions like NPE - they are programmatic errors and the developer should be more responsible to have not created a NullPointer. You don't expect your code to account for such careless errors and put a try(NPE), catch(NPE). So throw a unchecked exceptions.
2) On the other hand the exceptions like SQL exceptions are at the rare cases, account for some external dependency. So, better throw a user defined checked exceptions. And the user can determine if he can connect to the backup SQL server if any.
3) There are another clause of exceptions, where the program cannot continue furhter. Say a Memory Out of Bounds. They should be thrown as Errors.
2 Comments
Jason Zhu
Thank you for the thorough coverage on good practices to do in this situation. The only issue that I have with logging is that there can be A LOT to log in large programs so log files can grow to become very large.
Mohan
yes, that might happen. Try rolling the log. Also while logging set different log level for different error statements. So unless you set the log level to debug, your log file size won't grow. The SQLException is not supposed to happen and a rare event. So set them as warning. Whereas a incorrect username - set as debug. And make your log level info by default. So this won't clutter your log unless you set the log level as debug.
Try this..
fillInStackTrace() method is called to re-initialize the stack trace data in the newly created throwable. Will be helpful in masking the info about the exception when tries to access the API.
Exception), and throw that. Just make sure that you generate your Javadocs appropriately...