What is the guideline for methods declaring runtime exceptions?

Let's say I call a 3rd party routine that throws SQLException. Is it permissible/standard/acceptable for that routine to be able to throw RuntimeExceptions without declaring that it does so?

As always, I am surprised by the confusion my questions cause :-D This is probably because I am confused.

In the following, the callable is a lambda that issues a commit, and this throws SQLException. callable.call throws Exception.

private doThis(Callable<T> callable) throws SQLException {
    try {
        return callable.call();
    } catch (SQLException e) {
        // do stuff
        throw e;
    } catch (Exception e) {
        break; // Eats any exception from call() which makes me scream internally.
    }
}

What I surmise from this is that the coder wanted doThis to throw a SQLException. However, the nature of using the Callable meant that the routine had to throw an Exception unless the coder did something. So he caught Exception and swallowed it. But Exception is the parent is RuntimeException, so we're eating those too.

What am I to do? Making doThis throw Exception seems clumsy and random. Wrapping any exception being thrown in a RuntimeException and raising that preserves the coder's intent but seems suspect.

EDIT -

ok, I have learned, thank you. Now the question is, what to do about it. Clearly, eating the exception is a bad idea. Diluting the SQLException as declared seems not great.

How does the collective wisdom of SO feel about wrapping the Exception in a RuntimeException?

    ...
    } catch (Exception e) {
        throw new RuntimeException(e);
    }

CodePudding user response：

What am I to do? Making doThis throw Exception seems clumsy and random. Wrapping any exception being thrown in a RuntimeException and raising that preserves the coder's intent but seems suspect.

This method's sole reason for existence appears to be to perform special handling (// do stuff) in case of a SQLException. To handle that error correctly, some assumptions about what the Callable is doing must hold. For the sake of argument, perhaps the "stuff" is marking a particular database connection for rollback, assuming that the Callable used it do some work that may have partially failed.

Yet, this method accepts Callable. That doesn't impose any limitation on what the called function might do, why it might throw a SQLException, or what a SQLException might mean if it does arise. This code feels wrong, because it is wrong to make these very specific assumptions about an open-ended, black-box function like Callable. And, it's probably unnecessary. This is a private method, so you know everything about the methods that call it, and you can modify them to make it clear how things work together.

Instead, define your own (functional) interface. Declare the specific exceptions your helper method can handle, and document any other requirements for implementations. If, in fact, the "stuff" done in your helper method involves a database connection that is a member of the object, you might define the interface like a Consumer or Function, but with SQLException or other errors.

Here's a hypothetical example:

private final Connection db;

@FunctionalInterface
private interface DatabaseOperation<T> {
    T apply(Connection db) throws SQLException;
}

private <T> T withRollback(DatabaseOperation<T> op) throws SQLException {
    try {
        return op.apply(db);
    } catch (SQLException ex) {
        db.rollback();
        throw ex;
    }
}

None of this is to say that Callable or other core functional interfaces are bad. You shouldn't create new, non-standard functional interfaces when the pre-defined interfaces would meet your needs. It's just that they don't in this case.

CodePudding user response：

Not sure what you mean.

Is it okay to stick exception types that extend RuntimeException in the throws clause of a method?

Java implies certain things and usually you can't undo these things.

For example, all java files act like they have import java.lang.*; at the top even if don't write that. All classes that don't have an extends clause act like they have extends java.lang.Object...

and all methods act like you have throws RuntimeException, Error on them whether you write it or not.

It's legal to write import java.lang.*;, although it would be completely pointless to do this. It is similarly legal to write:

void foo() throws RuntimeException {}

even though this is rather pointless. However, unlike import java.lang.*;, adding some specific runtime exception can be useful in the sense that it documents (It has absolutely no effect whatsoever on what the code does). Some linting tools (I would disagree with the style they are espousing, though!) flag it down as 'useless code' but I wouldn't be so hasty. For example, this method:

public static int parseInt(String in) throws NumberFormatException {
  ..
}

is declaring the NFEx for no programmatic purpose (NFEx is a RuntimeException and all methods trivially can throw those without declaring them), but it is documentary: It highlights that that's an exception you should be taking into consideration when invoking this method / it's the obvious "thing you are looking for" (in the sense that 'soooo... what if I pass a string that does not contain a number to this method, what happens then' - is the obvious question to ask).

So, do it, if that's your intent. Your javadoc should probably have a:

 * @throws NumberFormatException If the provided {@code input} is not parseable as an integer number.

or similar, for each explicit RuntimeException you care to state.

Is it okay to wrap SQLException to RuntimeException

Usually: No. The notion of declared checked exceptions is that they are part of your signature. A signature is all about what a method does. It is very much not at all about how it does so.

Thus, the crucial question to ask is: Is the SQLException aspect of it fundamental to what your method does, or fundamental to how it does it?

A trivial example:

saveGame() may simply 'save your game to some persistent storage mechanism'. That's what it does. Perhaps the how involves a database today. Perhaps tomorrow it'll involve the file system. Perhaps next year it's a cloud save system and it involves network access. If the point of this method is merely to force a save, and the method's signature intentionally does not explain how (because you want the future flexibility of changing this, and thus any callers should not assume it's disk, or DB, or network, or anything else), then and only then: SQLException would purely be about the how - and thus it has no place being thrown from the saveGame method. You should wrap it. I highly doubt it's a good idea to wrap that in RuntimeException, I'd invent your own exception (SaveGameException perhaps). Should that exception be checked or unchecked? That depends on too many factors to get into for this answer.

However, something like Files.newOutputStream is different. The what of this method includes the notion that its file based. Fundamentally: It would be nuts if that method doesn't interact with a file system at all and instead opens a database connection. The very name of that method spells it out: It is, after all, in a class named Files - the fact that it interacts with files is a fundamental, unremovable aspect of it. Hence, that method should most absolutely be declared to throws IOException. Similarly, if you have a method called saveToDb, that method should definitely be declared as throws SQLException - and thus, you should not be wrapping your SQLException in anything else.

In rare cases, convenience and intended usage strongly dictates you do not want the exception to be checked. In that case, make an unchecked variant and wrap that. Java itself has UncheckedIOException already - reuse that if it's about IOExceptions. Write the same principle if its about SQLException. However, usually if this comes up you've badly designed your classes - if there's an aspect of your system that is fundamentally tied closely to the notion of interacting with DBs, that entire chain of methods should all be declared to throws SQLException, at which point the need to make that unchecked no longer exists.

Can RuntimeExceptions happen?

Yes, by their nature, RuntimeExceptions can happen at any time. Looking at the greater java ecosystem, RuntimeExceptions cover 3 mostly separate use cases (this isn't getting into what the feature was ever intended for, this is solely about what common java libraries and code actually use them for):

• Programming errors (bugs)

The quintessential example is NullPointerException. There is a bug, the bug should cause some kind of RuntimeException to be thrown. We do not want to make such exceptions checked (because nobody likes a catch block that literally is impossible to trigger unless you actively write bugs to do so - what could you possibly stick in the catch block?), but we DO want to throw something: Breaking off the method and 'unrolling' the entire call stack until some general handler deals with the problem (and gets lots of details about where that problem occurred and why) is precisely what we want. Hence, exception good, checked bad - these should be runtime exceptions and generally are.

Similar examples: Many methods that have rules about what you can pass to them which cannot be expressed in the type system (say, a method that requires you pass a non-negative integer) will throw IllegalArgumentException if you pass a negative number to them, for example. Some methods cannot be called when the object is in a certain state, and will throw IllegalStateException or UnsupportedOperationException, both unchecked (subtypes of RuntimeException) - for example, if you attempt to invoke .add() on an immutable list, you'd get that.

• Couldn't be bothered

Sometimes checked exceptions can occur but you just don't care, either because you're a lazy programmer who wants to just go home (that'd be quite bad, of course), or because you cannot fathom why a checked exception could occur. For example, an IOException when writing to an OutputStream that is memory based.

IDEs tend to suggest this incredibly stupid 'fix':

try {
  code();
} catch (IOException e) {
  e.printStackTrace();
}

I do not know why. Mass delusion? At any rate, all IDEs are very wrong, that is bad code, the correct 'I do not want to think about this checked exception right now' handle block is throw new RuntimeException("uncaught", e); - and smart folks have updated their IDEs to auto-generate this. It's wrapping a checked exception in an unchecked one simply for convenience, or because the coder did not think it could happen.

• For API/programming model reasons

See UncheckedIOException - usually because the code is designed to be used in stream pipelines which doesn't allow checked exceptions. Somewhat rare.

• Unexpected situations

It could be reasonable for the programmer of this library to fail to account for some exotic situation which nevertheless is also reasonable for your specific usage of some library to run into. For example, a third party library could be saving data to whatever OutputStream you provide for it, and that OutputStream might throw a documented unchecked exception for some reason - in which case your library is likely to also throw it. This is rare - you'd presumably know about it, and it comes up only if some library is 'intermediary' - you pass it some object that it, in turn, acts upon, and in that interaction, the runtime exceptions end up getting passed through. Or, the object you pass in doesn't adhere to certain rules. For example, if I hand a comparator that doesn't adhere to the spec (for example, is inconsistent: both a.compareTo(b) and b.compareTo(a) return a negative number - the Comparator spec says you should not do that) to some library, and that library ends up throwing some undocumented unchecked exception for this, I wouldn't blame the library author.

---

The 'first' case (code bugs) should be documented. For example, Integer.parseInt indeed documents that a NumberFormatException is thrown if you pass a string that doesn't contain something parseable to an int. List's add documents that it might not be supported in which case UnsupportedOperationException will be thrown, etcetera. If your 3rd party routine's docs don't state anything, then it does not apply (or it is badly documented).

The 'second' case (laziness) is hopefully not the case either - that would mean the programmer of it did a bad job.

The third case should also be documented.

That leaves the 4th case, which could happen and won't be documented. You have some control over this, though.

CONCLUSION: Well written libraries will not be throwing undocumented RuntimeExceptions that you didn't expect (i.e. where you aren't the direct cause of it by handing weird objects or things that don't fit clearly documented specs)