API changes to add AutoCloseable for try-with-resources

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

API changes to add AutoCloseable for try-with-resources

Ben Caradoc-Davies-2
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 <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

Re: 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 XXXX" 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:[hidden email]]
Sent: Monday, June 04, 2018 5:25 PM
To: GeoTools Devel <[hidden email]>; Geoserver-devel <[hidden email]>
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 <[hidden email]>
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
[hidden email]
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

Re: API changes to add AutoCloseable for try-with-resources

Erik Merkle
In reply to this post by Ben Caradoc-Davies-2
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 <[hidden email]> 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 <[hidden email]>
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
[hidden email]
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

Re: 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
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
[hidden email]
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

Re: API changes to add AutoCloseable for try-with-resources

Ben Caradoc-Davies-2
In reply to this post by Erik Merkle
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 <[hidden email]> 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 <[hidden email]>
>> 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
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>
>

--
Ben Caradoc-Davies <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

Re: API changes to add AutoCloseable for try-with-resources

Ben Caradoc-Davies-2
In reply to this post by Erik Merkle
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 <[hidden email]>
> 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 <[hidden email]>
>> 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 <[hidden email]>
>>> 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
>>> [hidden email]
>>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>>
>>
>>
>

--
Ben Caradoc-Davies <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Nuno Oliveira-3
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 <[hidden email]>
>> 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 <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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
>>>> [hidden email]
>>>> 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
_______________________________________________
Geoserver-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Ian Turton
In reply to this post by Ben Caradoc-Davies-2


On Tue, 5 Jun 2018 at 00:25, Ben Caradoc-Davies <[hidden email]> 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.

+1 for this anything that improves clean up and saves me writing code is good! 

- Should we add AutoCloseable to interfaces, and if so which ones? We
could make a list.

This is an obvious first step, is there an easy way to do it? grep for dispose?
   

- Do we make the change one interface at a time or try to do them all at
once?

 
I would go for one at a time, working from a list. My feeling is that doing one is something I could work on while waiting for something else to complete, whereas doing all of them is going to be a weekend or more and harder to share the work out.  
 
- Should we rename dispose() to close() in implementers and add a
deprecated dispose() that wraps close(), or just add a close() that
wraps dispose()?

I'd favour deprecating a dispose() that wraps close() - makes it clearer what we intend.
 

- As we are breaking the API anyway, should we get rid of dispose()
entirely by renaming it to close() without adding a deprecated wrapper?


That is harder for downstream users to handle IMHO.
 
- 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?


I would prefer to fight the debt as we go and find all the deprecated disposes in our code as we go. 
 
- Who is interested in participating in this work?


I'd be up to do some of it.

Ian



------------------------------------------------------------------------------
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

geowolf
On Tue, Jun 5, 2018 at 10:05 AM, Ian Turton <[hidden email]> wrote:

- Who is interested in participating in this work?


I'd be up to do some of it.

Same here. As spare time contribution, if we are a group, we do the deprecation, and attack one interface at a time.
As a code sprint if we want to do all toghether (though, in a week long code sprint, we might want to look into more 
"urgent yet too big" stuff, like running the whole project stack on Java 10-11).

Cheers
Andrea

== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 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
_______________________________________________
Geoserver-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Niels Charlier
In reply to this post by Nuno Oliveira-3
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 <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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 <[hidden email]>
>>>>> 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
>>>>> [hidden email]
>>>>> 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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Christian Mueller-3
A big +1 one  from my side,  the auto closable feature increases code quality.

Cheers

On Tue, Jun 5, 2018 at 10:47 AM, Niels Charlier <[hidden email]> wrote:
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 <[hidden email]>
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 <[hidden email]>
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 <[hidden email]>
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
[hidden email]
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
_______________________________________________



--
DI Christian Mueller MSc (GIS), MSc (IT-Security)
OSS Open Source Solutions GmbH


------------------------------------------------------------------------------
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Ben Caradoc-Davies-2
In reply to this post by Ian Turton
On 05/06/18 20:05, Ian Turton wrote:
> On Tue, 5 Jun 2018 at 00:25, Ben Caradoc-Davies <[hidden email]> wrote:
>> - Should we add AutoCloseable to interfaces, and if so which ones? We
>> could make a list.
> This is an obvious first step, is there an easy way to do it? grep for
> dispose?

I found many interfaces and classes with:
find . -name "*.java" -exec grep -H 'void dispose()' {} \;

With context:
find . -name "*.java" -exec grep -H -B2 'void dispose()' {} \;

Names only:
find . -name "*.java" -exec grep -l 'void dispose()' {} \; | sort

Nuno also made a list.

>> - Should we rename dispose() to close() in implementers and add a
>> deprecated dispose() that wraps close(), or just add a close() that
>> wraps dispose()?
> I'd favour deprecating a dispose() that wraps close() - makes it clearer
> what we intend.

This was my first thought, but now I prefer Nuno and Erik's suggestion
of a default method. I am contemplating adding a Disposable interface.

Kind regards,

--
Ben Caradoc-Davies <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Ben Caradoc-Davies-2
In reply to this post by Nuno Oliveira-3
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
<https://stackoverflow.com/questions/23453287/why-is-final-not-allowed-in-java-8-interface-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
>>>
>>> <http://d32wfbeasdaw38.cloudfront.net/img/Boundless_Logo.png>
>>>
>>> On Mon, Jun 4, 2018 at 7:49 PM, Erik Merkle <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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 <[hidden email]>
>>>>> 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
>>>>> [hidden email]
>>>>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>>>>
>>>>
>>>>
>>>
>>
>

--
Ben Caradoc-Davies <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Ben Caradoc-Davies-2
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 <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Ben Caradoc-Davies-2
Or with generics:


public interface ThrowingDisposable<T extends Exception> extends
AutoCloseable {
     /**
      * @see java.lang.AutoCloseable#close()
      */
     @Deprecated
     @Override
     default void close() throws T {
         dispose();
     }
     void dispose() throws T;
}
public interface Disposable extends ThrowingDisposable<Exception> {
     /**
      * @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 <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Nuno Oliveira-3
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<T extends Exception> extends AutoCloseable {
>     /**
>      * @see java.lang.AutoCloseable#close()
>      */
>     @Deprecated
>     @Override
>     default void close() throws T {
>         dispose();
>     }
>     void dispose() throws T;
> }
> public interface Disposable extends ThrowingDisposable<Exception> {
>     /**
>      * @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
_______________________________________________
Geoserver-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Reply | Threaded
Open this post in threaded view
|

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

Ben Caradoc-Davies-2
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<T extends Exception> extends
>> AutoCloseable {
>>     /**
>>      * @see java.lang.AutoCloseable#close()
>>      */
>>     @Deprecated
>>     @Override
>>     default void close() throws T {
>>         dispose();
>>     }
>>     void dispose() throws T;
>> }
>> public interface Disposable extends ThrowingDisposable<Exception> {
>>     /**
>>      * @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 <[hidden email]>
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel