On Sun, Sep 30, 2018, at 13:19, Matthias J. Sax wrote:
> Closeable is part of `java.io` while AutoClosable is part of
> `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> out that `Closable#close()` must be idempotent while
> `AutoClosable#close()` can have side effects.

That's an interesting note.   Looks like the exact JavaDoc text is:

 > Note that unlike the close method of Closeable, this close method is not 
 > required to be idempotent. In other words, calling this close method more 
 > than once may have some visible side effect, unlike Closeable.close which 
 > is required to have no effect if called more than once. However, 
 > implementers of this interface are strongly encouraged to make their close 
 > methods idempotent.

So you can make it non-idempotent, but it's still recommended to make it 
idempotent.

> 
> Thus, I am not sure atm which one suits better.
> 
> However, it's a good hint, that `AutoClosable#close()` declares `throws
> Exception` and thus, it seems to be a backward incompatible change.
> Hence, I am not sure if we can actually move forward easily with this KIP.

I was worried about that too, but actually you can implement the AutoCloseable 
interface without declaring "throws Exception".  In general, you can implement 
an interface while throwing a subset of the possible checked exceptions.

There is one big benefit of AutoCloseable that I haven't seen mentioned here 
yet: the ability to use constructrs like try-with-resources transparently.  So 
you can do things like

> try (MyClass m = new MyClass()) {
>   m.doSomething(...);
> }

best,
Colin

> 
> Nit: `RecordCollectorImpl` is an internal class that implements
> `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
> 
> 
> -Matthias
> 
> 
> On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> >> (Although I am not quite sure
> >> when one is more desirable than the other)
> > 
> > Most kafka's classes implementing Closeable/AutoCloseable doesn't throw 
> > checked exception in close() method. Perhaps we should have a 
> > "KafkaCloseable" interface which has a close() method without throwing any 
> > checked exception...
> > 
> > On 2018/09/27 19:11:20, Yishun Guan <gyis...@gmail.com> wrote: 
> >> Hi All,
> >>
> >> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> >> should be implementing Closeable as well (Although I am not quite sure
> >> when one is more desirable than the other), also I just looked through
> >> your list - these are some great additions, I will add them to the
> >> list.
> >>
> >> Thanks,
> >> Yishun
> >> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <dong...@apache.org> wrote:
> >>>
> >>> Hi Yishun,
> >>>
> >>> Thank you for your great KIP. In fact, I have also encountered the cases
> >>> where Autoclosable is so desired several times! Let me inspect more
> >>> candidate classes as well.
> >>>
> >>> +1. I also refined your KIP a little bit.
> >>>
> >>> Best,
> >>> Dongjin
> >>>
> >>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <chia7...@apache.org> 
> >>> wrote:
> >>>
> >>>> hi Yishun
> >>>>
> >>>> Thanks for nice KIP!
> >>>>
> >>>> Q1)
> >>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> >>>>
> >>>> Q2)
> >>>> I grep project and then noticed there are other close methods but do not
> >>>> implement AutoCloseable.
> >>>> For example:
> >>>> 1) WorkerConnector
> >>>> 2) MemoryRecordsBuilder
> >>>> 3) MetricsReporter
> >>>> 4) ExpiringCredentialRefreshingLogin
> >>>> 5) KafkaChannel
> >>>> 6) ConsumerInterceptor
> >>>> 7) SelectorMetrics
> >>>> 8) HeartbeatThread
> >>>>
> >>>> Cheers,
> >>>> Chia-Ping
> >>>>
> >>>>
> >>>> On 2018/09/26 23:44:31, Yishun Guan <gyis...@gmail.com> wrote:
> >>>>> Hi All,
> >>>>>
> >>>>> Here is a trivial KIP:
> >>>>>
> >>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >>>>>
> >>>>> Suggestions are welcome.
> >>>>>
> >>>>> Thanks,
> >>>>> Yishun
> >>>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> *Dongjin Lee*
> >>>
> >>> *A hitchhiker in the mathematical world.*
> >>>
> >>> *github:  <http://goog_969573159/>github.com/dongjinleekr
> >>> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> >>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> >>> www.slideshare.net/dongjinleekr
> >>> <http://www.slideshare.net/dongjinleekr>*
> >>
> 
> Email had 1 attachment:
> + signature.asc
>   1k (application/pgp-signature)

Reply via email to