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

Remove Releasable in favor of Closeable #5423

Closed
s1monw opened this issue Mar 13, 2014 · 8 comments
Closed

Remove Releasable in favor of Closeable #5423

s1monw opened this issue Mar 13, 2014 · 8 comments

Comments

@s1monw
Copy link
Contributor

s1monw commented Mar 13, 2014

given that we are moving to Java 1.7 when Lucene 4.8 lands (#5421) we should take advantage of try-with statements and use Closeable instead since it's bascially the same thing!

@uboness
Copy link
Contributor

uboness commented Mar 13, 2014

+1

@jpountz
Copy link
Contributor

jpountz commented Mar 13, 2014

Or should it be just AutoCloseable instead of Closeable? Javadocs suggest that Closeable.close needs to be idempotent while AutoCloseable.close doesn't and I think it makes sense to throw errors upon double-release (as it quite likely suggests a programming error)?

@uboness
Copy link
Contributor

uboness commented Mar 13, 2014

Not sure I agree... Sometimes you just want to make sure something is closed even if there's a chance it's already closed... We should be lenient there

@uboness
Copy link
Contributor

uboness commented Mar 13, 2014

We can use assertion for ensuring things are closed once

@s1monw
Copy link
Contributor Author

s1monw commented Mar 13, 2014

I think it's good to use AutoCloseable since we can then throw already closed exceptiosn? I like the idea - I guess we should have a base class that makes sure the actual releaseing is only happening once?

@s1monw
Copy link
Contributor Author

s1monw commented Apr 14, 2014

@jpountz I think we can lose this now right?

@jpountz
Copy link
Contributor

jpountz commented Apr 14, 2014

Closed via e589301. Releasable now extends AutoCloseable and can be used in try-with-resources.

@jpountz jpountz closed this as completed Apr 14, 2014
@s1monw
Copy link
Contributor Author

s1monw commented Apr 14, 2014

thx

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

4 participants