Thanks Justin - see inlineI don't think any information is getting lost in the change: If they implement MessageHandler<ByteBuffer> we know they want a ByteBuffer, if they implement MessageHandler<String> we know they want a string, if they implement MessageHandler<InputStream> etc. If they implement just MessageHandler<Object> we know we need to find an appropriate encoder.
A) Consolidate MessageHandlers (Issue
Currently we have MessageHandler.Text, MessageHandler.Binary, MessageHandler.AsyncText, MessageHandler.AsyncBinary, MessageHandler.BinaryStream, MessageHandler.CharStream, MessageHandler.DecodedObject and MessageHandler.Pong.
The same functionality could be consolidated into two handler interfaces instead, for example:
void onMessage(T message)
with allowable types T of String, ByteBuffer, InputStream, Reader, Object, and a new interface PongMessage, and
void onMessage(T partialMessage, boolean isLast)
with allowable types String, ByteBuffer.
IMO: Some type of consolidation seems cleaner to me, and also a little simpler to extend to other types in the future.
Internally, how will implementations be able to tell which callback to call when a frame arrives if instanceof is no longer an option? I'm sure I'm missing something here with this change but it feels like we're losing something with this change.My preference stands corrected ! I propose to leave them off.
4. Methods in public interfaces do not themselves need to be declared
public. (style issue / choice)
IMO: Will defer to majority here, but I prefer public declaration.
Oracle (née Sun) style guides recommend leaving off the modifiers as they're redundant.
Parameter ordering is inconsistent
onClose(Session, CloseReason) vs. onError(Throwable, Session)
Parameter naming is inconsistent s vs. session
IMO: Will make consistent, Session as last parameter unless I hear otherwise.
API uses raw types in a few places.
IMO: Will fix unless I hear otherwise.
IMO: oops, will fix unless I hear otherwise.
You can find me on the net at:
[jsr356-experts] Re: Some API suggestions from EDR