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