> On 8 Oct 2015, at 20:51, Simone Bordet <simone.bor...@gmail.com> wrote: > > What it is still missing is the fact that there is no specification > about the onXXX methods regarding the lifecycle of the parameters > passed in.
There is, actually. I have put it as a top-level javadoc, not as a javadoc to each single method. But that's an editorial problem, not a spec one. 1. Go to: http://cr.openjdk.java.net/~prappo/8087113/webrev.01/raw_files/new/src/java.httpclient/share/classes/java/net/httpclient/WebSocket.java 2. Look for occurrences of "reuse". There are exactly 2 of them :-) > For example, this is going to surprise users (simple echo): > > onBinary((ws, bytes, last) -> { > ws.sendBinary(bytes, last, null, (_, failure) -> {}); > } That would only surprise people who don't read javadoc. Or you want to say it's an inherently bad decision to allow people to reuse implementation's ByteBuffer? > It's not going to work because the send is async, and there is no > specification about who owns the ByteBuffer "bytes". > In my experience, it is bad to force applications to perform a copy > (not to mention that the copy could be really expensive, as WebSocket > frames could be large). I wonder what percentage of use cases this scenario corresponds to? Namely, a huge payload being received, transformed(?) and sent on the WebSocket. I ask this because I have to understand whether the added complexity worth the value provided by the functionality. > This would also lead to applications being forced to block waiting for > the send to complete How come? Send is not blocking. Unless one performs a rather odd thing: will block on handler completion before returning an onXXX call. > StatusCode makes little sense to me: it's a wrapper for int, and > nothing more. I would prefer to see primitive int in the signatures. Type safety, having documentation in a single place, maybe richer string representation (toString)? > Method isClosed() does not convey enough information. > WebSocket has built-in in the protocol half-closes, so it should be > able to report this information, like Socket returns > isInputShutDown(), isOutputShutDown() and isOpen(). > A simple enum would do. What's the use case other than debugging? > I think it's enough for the builder to provide an async build() method only. Makes sense.
