Eric,
that is an interesting idea but I suspect it would introduce silent
resource leaks. Unless we have a default implementation that calls the
deprecated dispose() method? In any case, just like constructors and
virtual destructors in C++, there in an intimate relationship between a
close() method and the implementing class. Avoiding a default
implementation avoids the risk that an implementer might forget to
override it.
Kind regards,
Ben.
On 05/06/18 12:49, Erik Merkle wrote:
I don't believe I have a vote here, but I wanted to add that you could
provide a default implementation on any GeoServer Interfaces to which you
want to add AutoCloseable. That would get around the compile issue, and I
believe the backward compatibility compile issue is exactly why Java added
the default keyword for interface methods. The default implementation could
simply be a no-op for things like close().
While it would alleviate compile issues, it might not have the most
reliable runtime effects.
For what it's worth,
Erik Merkle
Software Engineer | Boundless
<http://d32wfbeasdaw38.cloudfront.net/img/Boundless_Logo.png>
On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies <b...@transient.nz> wrote:
Many interfaces in GeoTools and GeoServer use the Dispose pattern, often
with a dispose() method, but do not implement AutoCloseable, preventing
their use in a try-with-resources statement. Examples range from
ImageReader to DataStore/DataAccess. Some interfaces like FeatureReader
already implement Closeable and thus AutoCloseable, but many do not.
Java 7 try-with-resources improves code quality because it simplifies code
by automating common boilerplate:
https://docs.oracle.com/javase/tutorial/essential/exceptions
/tryResourceClose.html
https://docs.oracle.com/javase/8/docs/api/java/lang/AutoCloseable.html
Adding AutoCloseable to an interface is an API-breaking change because
third-party subclasses that do not implement a close() method will no
longer compile. Any change would be applied only to master and would target
GeoTools 20.0 and GeoServer 2.14.0.
- Should we add AutoCloseable to interfaces, and if so which ones? We
could make a list.
- Do we make the change one interface at a time or try to do them all at
once?
- Should we rename dispose() to close() in implementers and add a
deprecated dispose() that wraps close(), or just add a close() that wraps
dispose()?
- As we are breaking the API anyway, should we get rid of dispose()
entirely by renaming it to close() without adding a deprecated wrapper?
- I thought of updating only interfaces and overrides. A more ambitious
scope would find every deprecated dispose() and refactor to use
try-with-resources. The alternative is to refactor incrementally over time.
How do we wish to pay off our technical debt?
- Who is interested in participating in this work?
Kind regards,
--
Ben Caradoc-Davies <b...@transient.nz>
Director
Transient Software Limited <https://transient.nz/>
New Zealand
------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Geoserver-devel mailing list
geoserver-de...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
--
Ben Caradoc-Davies <b...@transient.nz>
Director
Transient Software Limited <https://transient.nz/>
New Zealand
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
