Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Ben Caradoc-Davies]

Agreed, and Java generics are erased at compile time and offer no run 
time benefits.


Kind regards,
Ben.

On 07/06/18 08:06, Nuno Oliveira wrote:

Hi Ben,

+1 for the NON generics option, my felling is that usually generics 
bring more harm than good in the long run :(



On 06/06/2018 03:44 AM, Ben Caradoc-Davies wrote:

Or with generics:


public interface ThrowingDisposable extends 
AutoCloseable {

    /**
 * @see java.lang.AutoCloseable#close()
 */
    @Deprecated
    @Override
    default void close() throws T {
    dispose();
    }
    void dispose() throws T;
}
public interface Disposable extends ThrowingDisposable {
    /**
 * @see java.lang.AutoCloseable#close()
 */
    @Deprecated
    @Override
    default void close() {
    dispose();
    }
    @Override
    void dispose();
}


Or just keep it simple and have dispose throw Exception like 
java.lang.AutoCloseable#close() and let implementers narrow the return 
type to no exception if they like. Impact will be minimal as client 
code will likely use an implementer not a bare Disposable:



public interface Disposable extends AutoCloseable{
    /**
 * @throws Exception
 * @see java.lang.AutoCloseable#close()
 */
    @Deprecated
    @Override
    default void close() throws Exception{
    dispose();
    }
    void dispose() throws Exception;
}


Preference?

Kind regards,





--
Ben Caradoc-Davies 
Director
Transient Software Limited 
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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Nuno Oliveira]

Hi Ben,

+1 for the NON generics option, my felling is that usually generics bring more 
harm than good in the long run :(



On 06/06/2018 03:44 AM, Ben Caradoc-Davies wrote:

Or with generics:


public interface ThrowingDisposable extends AutoCloseable {
    /**
 * @see java.lang.AutoCloseable#close()
 */
    @Deprecated
    @Override
    default void close() throws T {
    dispose();
    }
    void dispose() throws T;
}
public interface Disposable extends ThrowingDisposable {
    /**
 * @see java.lang.AutoCloseable#close()
 */
    @Deprecated
    @Override
    default void close() {
    dispose();
    }
    @Override
    void dispose();
}


Or just keep it simple and have dispose throw Exception like 
java.lang.AutoCloseable#close() and let implementers narrow the return type to 
no exception if they like. Impact will be minimal as client code will likely 
use an implementer not a bare Disposable:



public interface Disposable extends AutoCloseable{
    /**
 * @throws Exception
 * @see java.lang.AutoCloseable#close()
 */
    @Deprecated
    @Override
    default void close() throws Exception{
    dispose();
    }
    void dispose() throws Exception;
}


Preference?

Kind regards,



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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Ben Caradoc-Davies]

Or with generics:


public interface ThrowingDisposable extends 
AutoCloseable {

/**
 * @see java.lang.AutoCloseable#close()
 */
@Deprecated
@Override
default void close() throws T {
dispose();
}
void dispose() throws T;
}
public interface Disposable extends ThrowingDisposable {
/**
 * @see java.lang.AutoCloseable#close()
 */
@Deprecated
@Override
default void close() {
dispose();
}
@Override
void dispose();
}


Or just keep it simple and have dispose throw Exception like 
java.lang.AutoCloseable#close() and let implementers narrow the return 
type to no exception if they like. Impact will be minimal as client code 
will likely use an implementer not a bare Disposable:



public interface Disposable extends AutoCloseable{
/**
 * @throws Exception
 * @see java.lang.AutoCloseable#close()
 */
@Deprecated
@Override
default void close() throws Exception{
dispose();
}
void dispose() throws Exception;
}


Preference?

Kind regards,

--
Ben Caradoc-Davies 
Director
Transient Software Limited 
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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Ben Caradoc-Davies]

On 06/06/18 13:48, Ben Caradoc-Davies wrote:
I would also remove the exception specification because it can never be 
thrown, and for consistency with dispose().


Although I should note that there are several classes with a dispose() 
that throws. While this is ugly, it can be a necessary guard required 
for data integrity. We might need two interfaces ThrowingDisposable and 
Disposable:


public interface ThrowingDisposable extends AutoCloseable {
/**
 * @see java.lang.AutoCloseable#close()
 */
@Deprecated
@Override
default void close() throws Exception {
dispose();
}
void dispose() throws Exception;
}


public interface Disposable extends ThrowingDisposable {
/**
 * @see java.lang.AutoCloseable#close()
 */
@Deprecated
@Override
default void close() {
dispose();
}
@Override
void dispose();
}


Kind regards,

--
Ben Caradoc-Davies 
Director
Transient Software Limited 
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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Ben Caradoc-Davies]

Nuno,

I think you are right. The problem with deprecating dispose is that the 
default method uses it and we will never be rid of it. What we really 
want is for implementers to use dispose() and for only 
try-with-resources to use close(). Java does not permit final on default 
methods 
 
but we can add a stern warning to treat close() as final. We could even 
deprecate close(), which is an ugly misuse, but might be the best way of 
warning implementers.


I would also remove the exception specification because it can never be 
thrown, and for consistency with dispose().


To make intent clear and make retrofitting even easier, I propose that 
we add a Disposable interface that we can conveniently add to any 
interface or class with an existing dispose() method. How about:


public interface Disposable extends AutoCloseable {
/**
 * Stern warning to treat close as final */
 * @see java.lang.AutoCloseable#close()
 */
@Deprecated
@Override
default void close() {
dispose();
}
void dispose();
}

I would put this in org.geotools.util in gt-metadata so it can be used 
by the classes in gt-referencing that have dispose().


The one class that cannot be fixed by this is the one that motivated me 
to start: ImageReader, which is in javax.imageio, and thus outside our 
grasp. This will require some ugly workarounds including delegates and 
static factory methods, but this should not deter us from the above fix. 
I am calling ImageReader out of scope for this change proposal.


Kind regards,
Ben.

On 05/06/18 20: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



On Mon, Jun 4, 2018 at 7:49 PM, 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



On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies 
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 

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Nuno Oliveira]

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



On Mon, Jun 4, 2018 at 7:49 PM, 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



On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies 
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 
Director
Transient Software Limited 
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 

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Ben Caradoc-Davies]

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



On Mon, Jun 4, 2018 at 7:49 PM, 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



On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies 
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 
Director
Transient Software Limited 
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 
Director
Transient Software Limited 
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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Ben Caradoc-Davies]

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



On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies  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 
Director
Transient Software Limited 
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 
Director
Transient Software Limited 
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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Chris Snider]

Ben,

Something to potentially consider regarding interfaces; While changing the 
signatures of existing interfaces will break the API, you could do one of two 
things:
1. copy the current interface wholesale and change the name to include a digit 
i.e. MyInterface => MyInterface1
Where the new interface name has the updated signature/implements.  
Implementations of the interface can then change to implement the newly name 
one, while existing implementations continue to work against the original 
interface.  The original interface can be marked as "@Deprecated use interface 
" and eventually removed altogether.

2. create a new interface that extends the original interface adding the 
additional changes.  This also uses the same process for implementation 
classes, but has the downside of NOT being able to deprecate/remove the 
original names if desired.

Anyway, something to consider.

Chris Snider
Senior Software Engineer


-Original Message-
From: Ben Caradoc-Davies [mailto:b...@transient.nz] 
Sent: Monday, June 04, 2018 5:25 PM
To: GeoTools Devel ; Geoserver-devel 

Subject: [Geoserver-devel] API changes to add AutoCloseable for 
try-with-resources

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 
Director
Transient Software Limited 
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

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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Erik Merkle]

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



On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies  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 
> Director
> Transient Software Limited 
> 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
>
--
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

Re: [Geotools-devel] [Geoserver-devel] API changes to add AutoCloseable for try-with-resources

[Erik Merkle]

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



On Mon, Jun 4, 2018 at 7:49 PM, 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
>
> 
>
> On Mon, Jun 4, 2018 at 6:24 PM, Ben Caradoc-Davies 
> 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 
>> Director
>> Transient Software Limited 
>> 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
>>
>
>
--
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