In Java, when should I create a checked exception, and when should it be a runtime exception? [duplicate]

Possible Duplicate:
When to choose checked and unchecked exceptions

When should I create a checked exception, and when should I make a runtime exception?

For example, suppose I created the following class:

public class Account {
    private float balance;

    /* ... constructor, getter, and other fields and methods */

    public void transferTo(Account other, float amount) {
        if (amount > balance)
            throw new NotEnoughBalanceException();
        /* ... */
    }
}

How should I create my NotEnoughBalanceException? Should it extend Exception or RuntimeException? Or should I just use IllegalArgumentException instead?

Solution 1:

There's a LOT of disagreement on this topic. At my last job, we ran into some real issues with Runtime exceptions being forgotten until they showed up in production (on agedwards.com), so we resolved to use checked exceptions exclusively.

At my current job, I find that there are many who are for Runtime exceptions in many or all cases.

Here's what I think: Using CheckedExceptions, I am forced at compile time to at least acknowledge the exception in the caller. With Runtime exceptions, I am not forced to by the compiler, but can write a unit test that makes me deal with it. Since I still believe that the earlier a bug is caught the cheaper it is to fix it, I prefer CheckedExceptions for this reason.

From a philosophical point of view, a method call is a contract to some degree between the caller and the called. Since the compiler enforces the types of parameters that are passed in, it seems symmetrical to let it enforce the types on the way out. That is, return values or exceptions.

My experience tells me that I get higher quality, that is, code that JUST WORKS, when I'm using checked exceptions. Checked exceptions may clutter code, but there are techniques to deal with this. I like to translate exceptions when passing a layer boundary. For example, if I'm passing up from my persistence layer, I would like to convert an SQL exception to a persistence exception, since the next layer up shouldn't care that I'm persisting to a SQL database, but will want to know if something could not be persisted. Another technique I use is to create a simple hierarchy of exceptions. This lets me write cleaner code one layer up, since I can catch the superclass, and only deal with the individual subclasses when it really matters.

Solution 2:

In general, I think the advice by Joshua Bloch in Effective Java best summarises the answer to your question: Use checked expections for recoverable conditions and runtime exceptions for programming errors (Item 58 in 2nd edition).

So in this case, if you really want to use exceptions, it should be a checked one. (Unless the documentation of transferTo() made it very clear that the method must not be called without checking for sufficient balance first by using some other Account method - but this would seem a bit awkward.)

But also note Items 59: Avoid unnecessary use of checked exceptions and 57: Use exceptions only for exceptional conditions. As others have pointed out, this case may not warrant an exception at all. Consider returning false (or perhaps a status object with details about what happened) if there is not enough credit.

Solution 3:

When to use checked exceptions? Honestly? In my humble opinion... never. I think it's been about 6 years since I last created a checked exception.

You can't force someone to deal with an error. Arguably it makes code worse not better. I can't tell you the number of times I've come across code like this:

try {
  ...
} catch (IOException e) {
  // do nothing
}

Whereas I have countless times written code like this:

try {
  ...
} catch (IOException e) {
  throw new RuntimeExceptione(e);
}

Why? Because a condition (not necessarily IOException; that's just an example) wasn't recoverable but was forced down my throat anyway and I am often forced to make the choice between doing the above and polluting my API just to propagate a checked exception all the way to the top where it's (rightlfully) fatal and will be logged.

There's a reason Spring's DAO helper classes translate the checked SQLException into the unchecked DataAccessException.

If you have things like lack of write permissions to a disk, lack of disk space or other fatal conditions you want to be making as much noise as possible and the way to do this is with... unchecked exceptions (or even Errors).

Additionally, checked exceptions break encapsulation.

This idea that checked exceptions should be used for "recoverable" errors is really pie-in-the-sky wishful thinking.

Checked exceptions in Java were an experiment... a failed experiment. We should just cut our losses, admit we made a mistake and move on. IMHO .Net got it right by only having unchecked exceptions. Then again it had the second-adopter advantage of learning from Java's mistakes.

Solution 4:

IMHO, it shouldn't be an exception at all. An exception, in my mind, should be used when exceptional things happen, and not as flow controls.

In your case, it isn't at all an exceptional status that someone tries to transfer more money than the balance allows. I figure these things happen very often in the real world. So you should program against these situations. An exception might be that your if-statement evaluates the balance good, but when the money is actually being subtracted from the account, the balance isn't good anymore, for some strange reason.

An exception might be that, just before calling transferTo(), you checked that the line was open to the bank. But inside the transferTo(), the code notices that the line isn't open any more, although, by all logic, it should be. THAT is an exception. If the line can't be opened, that's not an exception, that's a plausible situation.

IMHO recap: Exceptions == weird black magic.

being-constructive-edit:

So, not to be all too contradictive, the method itself might very well throw an exception. But the use of the method should be controlled: You first check the balance (outside of the transferTo() method), and if the balance is good, only then call transferTo(). If transferTo() notices that the balance, for some odd reason, isn't good anymore, you throw the exception, which you diligently catch.

In that case, you have all your ducks in a row, and know that there's nothing more you can do (because what was true became false, as if by itself), other than log the exception, send a notification to someone, and tell the customer politely that someone didn't sacrifice their virgins properly during the last full moon, and the problem will be fixed at the first possible moment.

less-enterprisey-suggestion-edit:

If you are doing this for your own pleasure (and the case seems to be this, see comments), I'd suggest returning a boolean instead. The usage would be something like this:

// ...
boolean success = transferTo(otherAccount, ONE_MILLION_DOLLARS_EXCLAMATION);

if (!success) {
  UI.showMessage("Aww shucks. You're not that rich");
  return; // or something...
} else {
  profit();
}
// ...

Solution 5:

I recently had a problem with exceptions, code threw NullPointerException and I had no idea why, after some investigation it turned out that real exception was swallowed(it was in new code, so its still being done) and method just returned null. If you do checked exceptions you must understand that bad programmers will just try catch it and ignore exception.