Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network.close() implicitly declared as throws Exception #1389

Closed
KantarBenedictAdamson opened this issue Apr 11, 2019 · 4 comments
Closed

Network.close() implicitly declared as throws Exception #1389

KantarBenedictAdamson opened this issue Apr 11, 2019 · 4 comments

Comments

@KantarBenedictAdamson
Copy link

The Network interface simply declares itself as extends AutoCloseable. It therefore inherits the throws clause from AutoCloseable, which is throws Exception. That is, it is implicitly declared as throwing a checked exception, which is inconvenient (but sometimes necessary). But as the actual implementation in Network.NetworkImpl does not throw any checked exceptions (Network.NetworkImpl.close() has no throws clause), this is an unnecessary inconvenience. Leaving the method as implicitly throws Exception is also against the advice of the AutoCloseable.close() specification:

While this interface method is declared to throw Exception, implementers are strongly encouraged to declare concrete implementations of the close method to throw more specific exceptions, or to throw no exception at all if the close operation cannot fail.

(A minor annoyance)

@KantarBenedictAdamson
Copy link
Author

The fix is, of course, simply:

public interface Network extends AutoCloseable, TestRule {
    @Override
    void close();

...

@bsideup
Copy link
Member

bsideup commented Apr 11, 2019

Hi @KantarBenedictAdamson,

Would you like to contribute it?

@KantarBenedictAdamson
Copy link
Author

I'm not sure I can contribute yet; I'm looking into using Testcontainers at my workplace. If the demonstration is OK, I'll then talk to my boss about making OSS contributions. Sorry I can't do more now. :-(

mumukiller pushed a commit to mumukiller/testcontainers-java that referenced this issue Apr 11, 2019
@rnorth rnorth added this to the next milestone Apr 15, 2019
@rnorth
Copy link
Member

rnorth commented Apr 16, 2019

Released in 1.11.2!

@rnorth rnorth closed this as completed Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants