client: add AutoCloseable #244
Conversation
|
|
||
| /** | ||
| * closes this watcher and all its resources. | ||
| **/ | ||
| @Override | ||
| void close(); |
There was a problem hiding this comment.
AutoCloseable::close() throws an exception, I do not think we should change the signature
There was a problem hiding this comment.
I think the point is whether to throw out exception on Watch#close, so far it’s prohibited.
We uses AutoCloseable just for try-with-resource, it should not be a constraint.
| /** | ||
| * close the client and release its resources. | ||
| */ | ||
| @Override | ||
| default void close() { | ||
| // noop |
| import org.junit.After; | ||
| import org.junit.Before; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.junit.rules.Timeout; | ||
|
|
||
| import java.util.Arrays; |
| @@ -26,6 +24,8 @@ | |||
| import org.junit.Test; | |||
| import org.junit.rules.Timeout; | |||
|
|
|||
| import static org.assertj.core.api.Assertions.assertThat; | |||
There was a problem hiding this comment.
prefer not move imports order
| @@ -48,11 +48,12 @@ | |||
| /** | |||
| * Interface of Watcher. | |||
| */ | |||
| interface Watcher { | |||
| interface Watcher extends AutoCloseable { | |||
There was a problem hiding this comment.
why extends not implement the AutoCloseable?
if we @Override the close() from AutoCloseable, how come our close() doesn't throw Exception?
There was a problem hiding this comment.
-
Interface can only
extendsanother interface. notimplement. -
The overriding method can throw those checked exceptions, which have less scope than the exception(s) declared in the overridden method.
There was a problem hiding this comment.
now i understand what you were doing. pretty cool java trick.
ScienJus
force-pushed
the
feature/autocloseable
branch
from
bb44f5f
to
6c45182
Compare
|
lgtm |
for #239
I think
Client,CloseableClientandWatchershould extendsAutoCloseable.but i can't make sure it's necessary for
KeepAliveandKeepAliveListener, because i don't know how to use them.