> Overall, it seems that `AutoClosable` might be the better interface to
> use though because it's more generic.
This sounds good to me. I don't know whether or not it's worth actually
transitioning any existing classes from Closeable to AutoCloseable.
I don't think it would affect any invocations, but there's the off-chance
that someone has assigned an instance to a Closeable variable, which would
break.
Overalll, it seems like maybe we should leave any Closeable implementations
alone and just add AutoCloseable where there's no existing closeable
interface.
-John
On Wed, Oct 3, 2018 at 12:06 PM Matthias J. Sax <matth...@confluent.io>
wrote:
> Thanks for clarifying. I thought that if we inherit `close() throws
> Exception` we need to declare the same exception -- this would have been
> an issue. Thus, my backward compatibility concerns are resolved.
>
> About try-with-resources: I think, allowing to use try-with-resources is
> the core motivation of this KIP to begin with. Also note, that `Closable
> extends AutoClosable`. Thus, both interfaces work with try-with-resource.
>
> Overall, it seems that `AutoClosable` might be the better interface to
> use though because it's more generic.
>
> -Matthias
>
>
> On 10/3/18 9:48 AM, Colin McCabe wrote:
> > 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)
>
>
