Hi Greg,
Thanks for the feedback - some thoughts inline...
On 8/7/12 10:34 PM, Greg Wilkins wrote:
>
>
> Danny,
>
> thanks for the update and here is some initial feedback
>
> I like the break down of all the different message handling interfaces
> - makes it clear all the different options available. However it is
> not clear if the annotation WebSocketMessage is able to represent all
> those different signatures - ideally anything that can be done with an
> interface can also be done with an annotation. Perhaps the
> annotation should just say that it supports any method signatures of
> the MessageHandler subtypes? For example I'd like to be able to do:
>
> @WebSocketEndPoint
> public class MyPojoWebSocket
> {
> @WebSocketMessage
> public void myAsyncTextHandler(String part, boolean last)
> { ... }
> }
When I first was thinking about the annotation, I'd only been thinking
about it mapping for the simple String or byte case. But I don't see why
it couldn't work for the other message handlers too as you suggest. We
could certainly declare that the annotation processor deal with
@WebSocketMessage by handling any of the parameter types in any of the
WebSocketHandler onXX methods and mapping it to the appropriate handler
type. byte[], String, InputStream, Reader, byte[]-boolean,
String-boolean I think is the list.
I can't think of a reason to add more information to the annotation to
distinguish the cases, I *think* its unambiguous.
>
>
> Also for ease of use, I'd really like to see the API offer message
> aggregation as part of the API. With annotations this could be:
>
> @WebSocketEndPoint(maxMessage=8192)
> public class MyPojoWebSocket
> {
> @WebSocketMessage
> public void myAsyncTextHandler(String part)
> { ... }
> }
Does this mean that the 'part' variable passed in would be the size of
the message if less than 8192 bytes in length, otherwise the first 8192
bytes of the message ?
Interesting. What kind of use case are you thinking of ?
>
> You might need and extra interface for this and a setter in
> EndpointConfiguration
>
>
>
>
> For the send side of things, I firstly think SendHandler#onResult
> should really be called SendCallback#onResult, or at least
> SendHandler#onResult
Yes I agree (onResult > setResult). It does look like *Handler is more
common than *Callback in the API's I've come across.
>
> Also I'm a bit dubious about the send methods that take both a
> completion handler AND return a future.
> Typically one or the other style is used, so if you are using
> callbacks, it is a bit wasteful to have to always create a future even
> if it was never used. Currently you have the pair of methods:
>
> |void||*sendBytes*(byte[] data)|
> |java.util.concurrent.Future<SendResult>||*sendBytes*(byte[] data,
> SendHandler completion)|
>
> with the first being blocking. I think it would be better to have:
>
> |||java.util.concurrent.Future<SendResult>||*sendBytes*(byte[] data)|
> |||void ||||*sendBytes*(byte[] data, SendHandler completion)|
>
> with neither being blocking. The user simply needs to call
> sendBytes(data).get() to achieve blocking.
>
> Alternately if you want to keep the blocking call, then don't have a
> future in the API at all. Instead provide a utility implementation
> of SendHandler that is a Future and can be used like:
>
> SendHandlerFuture future=new SendHandlerFuture();
> wsEndPoint.sendBytes(data,future);
> future.get(10,TimeUnit.SECONDS);
>
> This mantra could easily be wrapped up in a static method to make it a
> one liner:
>
> SendHandlerFuture.sendBytes(wsEndPoint,data).get(10,TimeUnit.SECONDS);
>
Yeah, I agree combining the byFuture and the byCompletion version into
one method is neither typical nor efficient if you don't use the future,
even if its compact. Probably should separate them out.
I do think that the blocking version is going to be the easy/70% kind of
style that developers would use. So I think it ought to be as simple as
possible - i.e. only one method call.
Maybe we should just have 3 calls for the three styles: blocking,
byFuture, byCompletion, nice and explicit and no extra steps needed ?
- d
>
>
>
> cheers
>
>
>
> --
> Greg Wilkins <gregw_at_intalio.com <mailto:gregw_at_intalio.com>>
> http://www.webtide.com
> Developer advice and support from the Jetty & CometD experts.
--
<http://www.oracle.com> *Danny Coward *
Java EE
Oracle Corporation