On Wed, Oct 24, 2012 at 9:15 PM, Danny Coward <danny.coward_at_oracle.com>wrote:
> Hello experts,
>
> Here's a list of API suggestions from the EDR to get things going, from
> larger to smaller. Let me know what you think - I've put my own thoughts on
> each.
>
> 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:
>
> MessageHandler<T>
> void onMessage(T message)
>
> with allowable types T of String, ByteBuffer, InputStream, Reader, Object,
> and a new interface PongMessage, and
>
> AsyncMessageHandler<T>
> 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.
>
>
> B) Add getURI() to ClientConfiguration
>
> IMO: This I think is an oversight. Implementations need this, otherwise
> they don't know where to connect the client endpoint to.
>
> C) Add checked exceptions to ClientContainer.connectToServer and
> ServerContainer.publishServer methods.
>
> For example, a new ConnectException, thrown in the case when the connect
> or publish fails.
>
> IMO: This makes sense to me.
>
> D) API fixes - as reported by Mark during implementation.
>
> Here's a long list of smaller API nits,
>
> 1. DecodeException / EncodeException
> Parameter ordering is inconsistent
> (ByteBuffer, String) vs. (String, Object)
>
> IMO: Should be consistent - will change ordering on EncodeException to
> match pattern on DecodeException unless I hear otherwise.
>
> 2. DefaultClientConfiguration
> getExtensions() returns null
> setExtensions() has a parameter of preferredExtensions
> (should probably be extensions)
>
> IMO: will fix unless I hear otherwise.
>
> 3. CloseReason
> No accessor for closeCode
> No accessor for reasonPhrase
>
> IMO: An oversight, will add unless I hear otherwise.
>
> 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.
5. Endpoint
> 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.
>
> 6. Generics
> API uses raw types in a few places.
>
> IMO: Will fix unless I hear otherwise.
>
> 7. Session
> s/getRemoteL/getRemote/
>
> IMO: oops, will fix unless I hear otherwise.
>
> Thanks,
>
> - Danny
> --
> <http://www.oracle.com> * Danny Coward *
> Java EE
> Oracle Corporation
>
--
You can find me on the net at:
http://antwerkz.com http://antwerkz.com/+
http://antwerkz.com/twitter http://antwerkz.com/github