Hi Roger, thanks for looking at this!
> On 1 Sep 2015, at 17:05, Roger Riggs <roger.ri...@oracle.com> wrote:
>
> - The Incoming class combines separate functions that would be easier to
> use if the methods were directly on the Builder.
...
> |WebSocket ws = WebSocket.newBuilder("ws://websocket.example.com")
> .onReceiveText( (ws, text) -> System.out.println(text)) .onError( (ws, t) ->
> t.printStackTrace()) .onClose( (ws, msg) -> System.out.println("closing: " +
> msg)) .create();|
At some stage earlier I had a prototype that looked exactly what you've
described
here.
One thing that caused me to discard it, was not lambda expressions themselves,
but the fact that we're splitting one type into several fine-grained ones. Some
simple examples showed me that non-trivial (useful) handlers would need to hold
and share some state. With split interfaces (an interface per function) we don't
have the kind of convenience and simplicity we would have with a single
interface.
Thus we have to organize this state/data sharing somewhere outside, making the
code a tiny bit more tangled and rigid (subjectively).
With functional interfaces and lambdas I feel it looks cleaner, but it might be
more painful to code against. I understand I might be overestimating the amount
of inconvenience here, so I would appreciate to hear your opinion on this.
> The Builder methods can use generic java.util.function interfaces with the
> appropriate parameter types.
> Each of the callbacks should include the websocket,
Yes, I thought about it. Among other good things, it gives us most of the
required types for free (i.e. no need to create many tiny types).
The thing that I didn't like was that if we choose to go with anonymous classes,
we would be stuck with these overly generic method names: apply, accept, etc.
That might become awkward if we're going to use the same type but from different
perspectives.
For example, both read and write back pressure have essentially the same
interface and the same semantics. It's the same message. The difference is
whether one is receiving or sending. In this case -- which side of the
callback one is on. Either one is calling .suspend(boolean) or one is being
called as suspend(boolean).
(for illustrative purposes only):
/**
* A control used to communicate back-pressure to a peer.
*/
public interface Backpressure {
/**
* Tells peer to suspend/resume data flow.
*
* @param f {@code true} to suspend, {@code false} to resume
*/
void suspend(boolean f);
}
Then WebSocket.Builder could have this:
/**
* Configures the builder with a user-provided back-pressure handler.
*
* <p> {@code WebSocket} will notify the user on whether they should
* suspend sending by calling {@code control.suspend(true)}, and {@code
* control.suspend(false)} when sending can be resumed.
*
* @param handler a back pressure handler
* @return this builder
*/
public Builder sendingBackpressure(Backpressure handler) {
return this;
}
And WebSocket could have this:
/**
* Returns a back pressure control, a user can use to notify a server
* it should suspend/resume sending.
*
* @return a control to communicate an incoming back-pressure.
*/
public abstract Backpressure receivingBackpressure();
This type is already lambda-freindly (it's a functional interface). Now let's
change it to Consumer<Boolean>, just to not create an additional type.
Then in the builder you can easily hide Consumer's "accept" by providing a
method reference:
WebSocket ws = WebSocket.newBuilder("ws://websocket.example.com")
.sendingBackpressure(this::onHold())
or lambda
WebSocket ws = WebSocket.newBuilder("ws://websocket.example.com")
.sendingBackpressure(() -> onHold())
But not in the WebSocket:
ws.receivingBackpressure().consume(true);
Generic functional interface is a good handler (callback), but a terrible domain
object.
> - The design to wrap the Outgoing data creates an extra step that makes the
> API
> a bit harder to use. It would be more natural to add the sendAsync methods
> to WebSocket
> and avoid the creation of extra objects.
>
> |ws.sendAsync("hello"); ws.sendAsync(bytebuffer); // an alternate signature
> could use varargs, ByteBuffer data... |
I considered it some time ago. Here's the obvious disadvantage of not having
this step I've found: it mixes the way data is sent with the way data is
constructed. Previous prototypes had several different "send" methods:
S = {send, sendAsync, queue,...}
We also have several different ways (4 at the moment) to construct data:
D = {ByteBuffer, CharSequence, Iterator<CharSequence>,
Iterator<ByteBuffer>, ...}
So if we put this into a single type we'll have to provide {S x D} methods:
send(ByteBuffer);
send(CharSequence);
send(Iterator<CharSequence>);
send(Iterator<ByteBuffer>);
...
queue(Iterator<ByteBuffer>);
Agreed, at the moment we have only 1 send and just 4 types of data sources. But
size S or D might change in future, and we'd better have a prepared way to adapt
to it.
-Pavel
