Skip to main content

[jsr356-experts] Re: V003 API for your review

  • From: Greg Wilkins < >
  • To:
  • Subject: [jsr356-experts] Re: V003 API for your review
  • Date: Wed, 8 Aug 2012 15:34:54 +1000


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:

public class MyPojoWebSocket
  public void myAsyncTextHandler(String part, boolean last)
  { ... }

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:

public class MyPojoWebSocket
  public void myAsyncTextHandler(String part)
  { ... }

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

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();

This mantra could easily be wrapped up in a static method to make it a one liner:



Greg Wilkins < " target="_blank"> >
Developer advice and support from the Jetty & CometD experts.

[jsr356-experts] V003 API for your review

Danny Coward 08/03/2012

[jsr356-experts] Re: V003 API for your review

Greg Wilkins 08/08/2012

[jsr356-experts] Re: [jsr356-users] Re: V003 API for your review

Danny Coward 08/09/2012

[jsr356-experts] Re: [jsr356-users] Re: V003 API for your review

Greg Wilkins 08/11/2012
Please Confirm