debugjavaMinor
Idiomatic catching of unchecked exceptions in ExecutorService.submit
Viewed 0 times
exceptionsexecutorservicecatchingidiomaticsubmitunchecked
Problem
OK, here's the problem space:
Lwjgl, a graphical library, has the ability to throw an unchecked
My solution? I used the strategy pattern, like so.
Using dependency injection, I inject an instance of
The reason behind my choices here is that if a
Lwjgl, a graphical library, has the ability to throw an unchecked
LWJGLException on certain methods - like Display.create(). This becomes a problem when you have to off-load all work done by Display onto a single thread using an ExecutorService - that's what I'm doing, unfortunately, due to how Lwjgl works. ExecutorService.submit(Runnable) does not allow you to throw unchecked exceptions. ExecutorService.submit(Callable) does, however, I still have to handle that exception.My solution? I used the strategy pattern, like so.
private final ExecutorService executor;
private final ExceptionHandler exceptionHandler;
@Inject
public LwjglWindow(@Lwjgl ExecutorService executor, @Lwjgl ExceptionHandler exceptionHandler) {
this.executor = executor;
this.exceptionHandler = exceptionHandler;
}
public void start() {
executor.submit(() -> {
try {
Display.create();
} catch (LWJGLException ex) {
exceptionHandler.onException(ex);
}
});
}Using dependency injection, I inject an instance of
ExceptionHandler to deal specifically with exceptions originating from Lwjgl. Should an exception occur from Lwjgl, well, then, this is called (from LwjglExceptionHandler):@Override
public void onException(Exception exception) {
logger.trace("lwjgl encountered an error", exception);
systemShutdownStrategy.shutdown();
}systemShutdownStrategy is an interface that is nothing more than a strategy pattern. This is injected into the instance of LwjglExceptionHandler, so it could be a stub and do nothing or it could shut down your program (which is, as the name of the field suggests, the intent). There's always room to implement some user-friendly error notification, rather than closing quietly, but this is just here for demonstration purposes.The reason behind my choices here is that if a
LwjglException occurs, the program is pretty much guaranteed to be uSolution
Exception handling in Java has certain best practices. But not all situations are covered equally.
In this case, what you have, is a combination of logging, handling, and shutdown handling.
In this case, your handling appears to be sane. The centralization of this logging/shutdown is convenient because it essentially extracts the same functionality from multiple places in to a single place.
There are two issues that I can see:
-
Logging in Java often is tied to the class that the log message is called from. For example, the Logger is created with a name of the class as a constructor/key.
This means that the logging of the exception may/will be logged with the name of the ErrorHandler class rather than the class that threw the exception.
Depending on your circumstances, this may be a problem.
-
Logic flow in situations like this are often complicated by the return types of the class. For example, your code may look like:
but, that code will not compile because you need to return an int value after handling the exception.
You know that there will never be an exit from the onException, because it shuts down. The compiler does not know that, so the compiler makes the wrong assessment of the exit path of the method.
I recommend you change the signature of the onException class, to have a return type of
That will keep the compiler happy too.
In this case, what you have, is a combination of logging, handling, and shutdown handling.
In this case, your handling appears to be sane. The centralization of this logging/shutdown is convenient because it essentially extracts the same functionality from multiple places in to a single place.
There are two issues that I can see:
-
Logging in Java often is tied to the class that the log message is called from. For example, the Logger is created with a name of the class as a constructor/key.
This means that the logging of the exception may/will be logged with the name of the ErrorHandler class rather than the class that threw the exception.
Depending on your circumstances, this may be a problem.
-
Logic flow in situations like this are often complicated by the return types of the class. For example, your code may look like:
public int myMethod() {
try {
// .... do something ....
return 1;
} catch (LWJGLException ex) {
exceptionHandler.onException(ex);
}
}but, that code will not compile because you need to return an int value after handling the exception.
You know that there will never be an exit from the onException, because it shuts down. The compiler does not know that, so the compiler makes the wrong assessment of the exit path of the method.
I recommend you change the signature of the onException class, to have a return type of
RuntimeException. With that, you can have:} catch (LWJGLException ex) {
throw exceptionHandler.onException(ex);
}That will keep the compiler happy too.
Code Snippets
public int myMethod() {
try {
// .... do something ....
return 1;
} catch (LWJGLException ex) {
exceptionHandler.onException(ex);
}
}} catch (LWJGLException ex) {
throw exceptionHandler.onException(ex);
}Context
StackExchange Code Review Q#61567, answer score: 2
Revisions (0)
No revisions yet.