In my opinion ProtocolDecoders should be stateless and a single instance should be used for all sessions. This allows multiple sessions to be handled with a single ProtocolDecoder instance.
It is probably far more common for protocols to be stateless. Note I am talking about the communication PROTOCOL. Clients and servers may be stateful, but the communication protocol even for these systems usually is not. Stateful ProtocolDecoders can be implemented very simply by storing state in the session as an attribute. Even if the protocol must implement a complex state machine with multiple attributes, this is best done by storing a single "state" object in the session and using the State Pattern. I feel implementing stateful ProtocolDecoders by instantiating a new one for each session and storing state within the ProtocolDecoder itself is a very bad idea. At the very least, please make sure that Mina doesn't always instantiate a new ProtocolDecoder instance in support of stateful ProtocolDecoders. This will add unnecessary overhead to servers where the protocol is not stateful. Rob ----- Original Message ---- From: Maarten Bosteels <[EMAIL PROTECTED]> To: [email protected] Sent: Sunday, March 18, 2007 3:44:58 PM Subject: Re: ProtocolDecoderAdapter and session-state. On 3/17/07, Niklas Therning <[EMAIL PROTECTED]> wrote: > Maarten Bosteels wrote: > > Hi Niklas, > > > > I am reading the mina mailing list since about a year, but this is the > > first time I see a recommendation to keep the ProtocolDecoder > > implemantation stateless. > > I prefer to create a new decoder per session and keep the decoder > > state in the decode. > > Sound OO-principle says : store the state in the class that uses it. > > Maybe stateless was the wrong word here. There's nothing wrong with > storing state in your ProtocolDecoder implementation. As long as it > isn't per-session state. I was talking about per-session state: the state of the decoding algorithm for one message. For example: suppose each message starts with a length-header (an integer), once the header is read, I would store the length of the incomplete message as an instance variable of the decoder, because the decoder is the only one that needs to know this data. WDYT ? I guess James is right about the visibility issue, hadn't thought about that yet, but I can solve that without storing the state in the IoSession. > Even if that will work in MINA 1.0 I guess that > could change in future versions. AFAIK there's nothing in the docs that > tells you that MINA 1.0 creates one decoder per session, right? You > would have to look at the ProcolCodecFilter code. You are right, some time ago I already noticed that it wasn't in the docs, and I had plans to raise it as an issue. http://mina.apache.org/report/trunk/apidocs/org/apache/mina/filter/codec/ProtocolCodecFactory.html#getDecoder() says: "Returns a new (or reusable) instance of ProtocolEncoder which encodes message objects into binary or protocol-specific data." I think it should be documented that this method is called once for each new session, no ? And maybe, the docs could also mention that storing per-session state in the decoder isn't a best-practice, although I am not yet sure why not :-) Maarten > > I think that if this is something users do and expect to work we need to > enforce this rule (one decoder instance per session) somehow to prevent > incompatibilities later on. At least document it thoroughly. > > > > > Should I really anticipate that mina will someday no longer support > > statefull decoders ? > > No, you shouldn't. :-) I'm just saying that AFAIK there's really nothing > that says that it works that way and at the moment you cannot expect it > not to change in the future. It's great that you're raising the issue > because if this is what users want we need to make sure it stays that way. > > > -- > Niklas Therning > www.spamdrain.net > > ____________________________________________________________________________________ TV dinner still cooling? Check out "Tonight's Picks" on Yahoo! TV. http://tv.yahoo.com/
