RFR JDK-8087113: Websocket API and implementation

[Pavel Rappo]

> On 4 Apr 2016, at 18:16, Anthony Vanelverdinghe <anthony.vanelverdinghe at 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!