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-de...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel






--
Regards,
Nuno Oliveira
==
GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information.
==

Nuno Miguel Carvalho Oliveira
@nmcoliveira
Software Engineer

GeoSolutions S.A.S.
Via di Montramito 3/A
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:      +39 0584 1660272

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Con riferimento alla normativa sul trattamento dei dati
personali (Reg. UE 2016/679 - Regolamento generale sulla
protezione dei dati “GDPR”), si precisa che ogni
circostanza inerente alla presente email (il suo contenuto,
gli eventuali allegati, etc.) è un dato la cui conoscenza
è riservata al/i solo/i destinatario/i indicati dallo
scrivente. Se il messaggio Le è giunto per errore, è
tenuta/o a cancellarlo, ogni altra operazione è illecita.
Le sarei comunque grato se potesse darmene notizia.

This email is intended only for the person or entity to
which it is addressed and may contain information that
is privileged, confidential or otherwise protected from
disclosure. We remind that - as provided by European
Regulation 2016/679 “GDPR” - copying, dissemination or
use of this e-mail or the information herein by anyone
other than the intended recipient is prohibited. If you
have received this email by mistake, please notify
us immediately by telephone or e-mail.


------------------------------------------------------------------------------
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

Reply via email to