Re: [MINA 3.0] SelectorListener implementation

[Emmanuel Lécharny]

Le 11/17/12 7:42 AM, Julien Vermillard a écrit :
 well I strongly disagreeing here :)

 The selector listener and the selector loop should be seen as a wrapper on
 top of nio to make things easier.
I don't disagree here.

 The event can happen at the same moment so we need to provide them as is
 and not to choose the order to propagate them.
You *do* chose the order in which the event are handled in the code of
the ready method ! The question is more about where we express the order
in which they are expressed... I see your point here : it's up to the
listener to deal with that choice, not to the selector loop.

 It's up to the listener to choose how to react if more than one flag is
 raised.

 IMHO I'm a bit bike-shedding here but what would be coding without tiny
 detail and taste discussion :)

Ok, I have no problem with your vision. However, there is something I
dislike, if we use the ready() method you are proposing, it's the way we
pass the events : passing many booleans in a specific order make it
difficult to know what event is to be processed. Something like :

ready( true, false, readBuffer, true )

does not tell anything about which event is true and which one is false,
unless you look at the interface : not very convenient... I'd rather
have something like :

reay( ByteBuffer readBuffer, SelectionKey.OP_WRITE, SelectionKey.OP_ACCEPT )

using this interface :

void ready( ByteBuffer, int... event )

This way, you don't have to question yoruself what this boolean stands
for when you look at the code,

 Hi guys,

 I'm wondering if it would not be more convenient to have 3 methods
 (well, 4 would probably be better) :

 void readyRead(ByteBuffer readBuffer);
 void readyWrite();
 void readyAccept();
 void readyConnect(); // This is for the client

 With an intermediate abstract class, we will just have to implement the
 method we need in the classes (for instance, the readyConnect() will
 only be implemented by the client, not by the server).

So I move those methods in the listener, and they will be called by the
listener's generic ready() method, not by the SelectorLoop.

Is that ok ?


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com 

Re: [MINA 3.0] SelectorListener implementation

[Julien Vermillard]

well I strongly disagreeing here :)

The selector listener and the selector loop should be seen as a wrapper on
top of nio to make things easier.

The event can happen at the same moment so we need to provide them as is
and not to choose the order to propagate them.

It's up to the listener to choose how to react if more than one flag is
raised.

IMHO I'm a bit bike-shedding here but what would be coding without tiny
detail and taste discussion :)

and by the way 3 method calls cost a lot more than 3 ifs :)


On Fri, Nov 16, 2012 at 1:54 PM, Emmanuel Lécharny elecha...@gmail.comwrote:

 Hi guys,

 I looked at the SelectorListner interface and its implementation.

 Here is the hierarchy of classes implementing this interface :

 (SelectorListener)
 o
 |
 +-- [NioTcpServer]
 |
 +-- [NioUdpServer]
 |
 +-- [NioTcpSesession]

 The interface itself expose one single method :
 void ready(boolean accept, boolean read, ByteBuffer readBuffer,
 boolean write);

 Considering that the implementers code is like that :

 NioTcpServer

 public void ready(final boolean accept, final boolean read, final
 ByteBuffer readBuffer, final boolean write) {
 if (accept) {
 createSession(getServerSocketChannel().accept());
 }

 if (read || write) {
 throw new IllegalStateException(should not receive read or
 write events);
 }
 }

 NioUdpServer

 public void ready(final boolean accept, final boolean read, final
 ByteBuffer readBuffer, final boolean write) {
 if (read) {
 readBuffer.clear();

 final SocketAddress source =
 datagramChannel.receive(readBuffer);
 readBuffer.flip();

 NioUdpSession session = sessions.get(source);

 if (session == null) {
 session = new NioUdpSession(this, idleChecker,
 address, source);
 }

 session.receivedDatagram(readBuffer);
 }

 if (write) {
 // TODO : flush session
 }
 }

 NioTcpSession

 public void ready(final boolean accept, final boolean read, final
 ByteBuffer readBuffer, final boolean write) {
 if (read) {
 processRead(readBuffer);
 }

 if (write) {
 processWrite();
 }
 if (accept) {
 throw new IllegalStateException(accept event should never
 occur on NioTcpSession);
 }
 }


 I'm wondering if it would not be more convenient to have 3 methods
 (well, 4 would probably be better) :

 void readyRead(ByteBuffer readBuffer);
 void readyWrite();
 void readyAccept();
 void readyConnect(); // This is for the client

 With an intermediate abstract class, we will just have to implement the
 method we need in the classes (for instance, the readyConnect() will
 only be implemented by the client, not by the server).

 wdyt ?

 --
 Regards,
 Cordialement,
 Emmanuel Lécharny
 www.iktek.com