No voting rights here, but just wanted to express my support for this!
On 05-06-18 10:02, Nuno Oliveira wrote:
I would add the AutoClose interface to all interfaces that have the
dispose method or a similar one. Then I would provide a default
implementation for the close method that invokes the dispose method:
default void close() throws Exception {
dispose();
}
This would make the interfaces fully backward compatible and would
allow us to use the resource try catch pattern. I don't see any
possible resource leakage with this approach, the new code that will
start using the auto close approach will delegate on the existing
dispose method and old code will still use the dispose method.
The only drawback I see is that the dispose method would still around,
the only thing we could do is mark it as deprecated ... but I can
leave with that.
On 06/05/2018 02:16 AM, Ben Caradoc-Davies wrote:
Erik,
we require Java 8 for all supported branches. Interface default
methods are on the table.
Kind regards,
Ben.
On 05/06/18 12:54, Erik Merkle wrote:
A small caveat to my suggestion about default methods. Apparently
default
methods on interfaces is a Java 8 thing. So it is not a viable
option if
running with an older version.
Erik Merkle
Software Engineer | Boundless
<http://d32wfbeasdaw38.cloudfront.net/img/Boundless_Logo.png>
On Mon, Jun 4, 2018 at 7:49 PM, Erik Merkle <emer...@boundlessgeo.com>
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-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
------------------------------------------------------------------------------
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-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
