mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ashish <paliwalash...@gmail.com>
Subject Re: svn commit: r1142469 - in /mina/branches/3.0/core/src: main/java/org/apache/mina/ main/java/org/apache/mina/filterchain/ test/java/filterchain/
Date Mon, 04 Jul 2011 13:30:49 GMT
On Mon, Jul 4, 2011 at 5:28 PM, Julien Vermillard <jvermillard@gmail.com>wrote:

> Hi,
> comments inline
>
> On Mon, Jul 4, 2011 at 7:05 AM, Ashish <paliwalashish@gmail.com> wrote:
> > Minor comments
> >
> > 1. Do we want to have a check if the filter is already added or we just
> > leave it to user?
>
> You have a use case in mind ? You can add a method for knowing the
> position of a given filter.
>

On 2nd thoughts, its just a chain for us, and logging filter example is a
good example that goes against implementing this.
Moreover, we can just check on some id only, which might not be very useful.
Lets drop this atm, and if needed we can always revisit :)


>
> > 2. Was just trying to use the API public void insertBefore/After(int
> > position, IoFilter ioFilter) as an end user.
> >   a) As a User, how do I find the index of the Filter?
> >       Probably, we need to provide another API which can give the same or
> > alternatively we can pass IoFilter instance instead of index.
>
> Good idea,  but you can actually insert two reference to the same
> filter (ex: the LoggingFilter in different place of the chain).
> Could work, but the filter will be inserted at the position of the
> first filter reference.
>

Agreed. We can rethink on API signature, all I am worried is the ease of
usage. For static chains its easy,
but this shall come in handy while dynamically changing the list :)
Let me think a bit more on this.



>
> >
> > Good to see things are taking shape :)
> >
>
> Yes :) Feel free to hack it by adding your suggestion method, I'll
> concentrate on filter execution, I just needed basic filter chain
> building.
> I would like to start processing events.
>
> Julien
>

Definitely, shall add it and probably add more unit tests if needed.

Let the party begin :)

cheers
ashish

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message