ExternalResource: declare after() to throw Exception #1421
Conversation
So that clients don't have to try-catch-and-rethrow explicitly checked exceptions.
| */ | ||
| protected void after() { | ||
| protected void after() throws Exception { |
There was a problem hiding this comment.
Why Exception instead of Throwable like we did for before()?
There was a problem hiding this comment.
Just because AFAIK rarely methods throw Throwable, it's more likely they throw Exception, e.g. AutoCloseable#close.
But ya, we should probably make it more generic and throw Throwable.
Do you want me to?
There was a problem hiding this comment.
While it's rare for methods to throw Throwable, when you call a method using reflection you often handle the InvocationTargetException by throwing the cause (a Throwable). That's why Statement.evaluate() is declared to throw Throwable.
I don't think it harms anything to have the method declared to throw Throwable. The calling code catches Throwable and subclasses of ExternalResource can override the method to throw a different exception.
There was a problem hiding this comment.
I don't think that closing a resource usually involves doing reflection or anything that throws Throwable, but anyway yeah, it's harmless, so I'll do.
|
Merged. @alb-i986 could you please update the release notes? |
|
Release notes updated. Cheers |
This allows clients to call methods that throw checked exceptions without having to catch and wrap checked exceptions.
|
@marcphilipp This breaks code that extends ExternalResource, overrides |
|
Agreed! @kcooney Do you have time to do so? |
|
@marcphilipp I thought I might have time but apparently not 🙁 |
Revert commit cebbf5e. It breaks code that extends ExternalResource, overrides after() and calls super.after() in after().
This allows clients to call methods that throw checked exceptions without having to catch and wrap checked exceptions.
So that clients don't have to try-catch-and-rethrow explicitly checked exceptions.
You may not be happy with changing the signature, though :(