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
