> On 4 Apr 2016, at 18:16, Anthony Vanelverdinghe > <anthony.vanelverdin...@gmail.com> wrote: > >>> - CompletableFuture<Void> sendClose(CloseCode code, CharSequence reason) >>> change the type of "reason" to String >> What's the rationale behind this? > Unlike with the sendText methods, I don't see the added value here. For > example, compare this with the method "header(String name, String value)" in > the Builder interface. To me this is the same: it might be useful for callers > if the parameters of that method were declared as a CharSequence, but they > aren't, and it would not be "Java-like" (according to my totally subjective > definition of that term) for them to be either. Anyway, just my 2 cents.
Okay. >>> java.net.http.WebSocket.Builder >>> - Builder connectTimeout(long timeout, TimeUnit unit) >>> use a single java.time.Duration argument instead >> I'm not too familiar with the new java.time API, but maybe you could explain >> what would some advantages be over the java.util.concurrent one? >> >> The one I can think of immediately is the encapsulation and consistency check >> would be done by java.time rather than by java.net.http.WebSocket. On the >> other >> hand java.util.concurrent.TimeUnit is more familiar. > I'd argue that, since java.net.http will be a new package (& separate > module), its API should make as good use as possible of the available types > in the JDK. And in that regard, I believe a single Duration parameter is > clearer. And, though it doesn't apply in this case: an advantage of Duration > is that you can cleanly return it: Duration getTimeout(), in which case it > would be natural to have the setter declared as void setTimeout(Duration t). > While j.u.c.TimeUnit is indeed more familiar, I don't see why this would > speak in its advantage: it's just a natural consequence from the fact that > it's been around 10 years longer than Duration already. I think I'm convinced. Thanks! >>> java.net.http.WebSocket.Listener >>> - onText/onBinary/onPing/onPong >>> return an immediately-complete CompletionStage (i.e. >>> CompletableFuture.completedStage(null)) instead of null >> We've been through this. It's a matter of both performance AND convenience. A >> user gets both convenience of returning null and reduction of allocation. >> Moreover if we go with disallowing null, people might be tempted to use a >> single >> shared instance >> >> CompletableFuture.completedStage(null) >> >> as a return value. I'd better stay away from it (though it seems technically >> possible, it still feels hacky). > Sorry, I hadn't gone through previous discussions. I assume the option of > returning Optional<CompletionStage<?>> has already been considered as well > then? Isn't it a leaning in the opposite direction? I mean it will surely work with an java.util.Optional.empty() in case of null, but for the non-null case we'll have to pay twice. Constructing CompletionStage AND its enclosing Optional. I agree with you, null smells, but think about the user. And the implementation doesn't care -- it's not the worst of its problems :-) Thanks for your time!