Hi,

So Tim, you was also ok with optin 2 and finally Norman reverted to option 1 ? (not sure to be in line :) ?)

I've looked at revision 987821:
- SubscriptionManager is not used anymore in MailboxManager hirerachy (StoreMailboxManager,...) - The 3 delegate methods present in DelegatingMailboxManager are removed (so, yes, DelegatingMailboxManager is not delegating anymore, and we could rename it :), or merge with StoreMailboxManager
- The ImapProcess is now responsible for the SubscriptionManager

I suppose the spring-beans.xml is not committed according to these changes (imapProcessor and mailboxmanager constructors should be updated)

I like the idea to leave an interface thats clearly describe the Subscription methods. Is there a particular reason to have now the DefaultImapProcessorFactory responsible for the SubscriptionManager ?
I can see it's // with the MailboxManager.
NioImapServer finally uses the imapProcessor bean.

btw, if we talk Gof patterns, we went from a "Strategy" pattern to a "Chain of responsibility" pattern. So the question I'm wondering is "is it the responsibility of the the NioImapServer to process the mailbox, then the subscription" (it does it via the DefaultProcessorChain).
I remain in favour of "strategy or chain", rather then inheritance.

Tim, are the mapper considerations your're talking here related to IMAP-206 ? (not sure to understand the relations, maybe a separate thread would help)

Tks,

Eric


On 21/08/2010 22:53, Tim-Christian Mundt wrote:
Norman,

I have not yet reviewed your recent commit. However, I'd like to remark
that I was in favor of your last attempt. I just wanted to
_additionally_:

I think we should also merge them which
would also simplify the package structure because we wouldn't need
the .mail and .user packages anymore.
Which means, not only merge the SubscriptionManager into the
MailboxManager like you did, but also the SubscriptionMapper into the
MailboxMapper. That would make the separation between .user and .mail
superfluous.

Does that make sense? Maybe your current attempt is still better, I
haven't checked yet.

Best
Tim

Am Samstag, den 21.08.2010, 22:08 +0200 schrieb Norman Maurer:
Ok another attempt was made.. please review changes made in revision 987821

Thx,
Norman

2010/8/21 Norman Maurer<nor...@apache.org>:
Well I need to revert it ;) I will do so then..

Bye,
Norman


2010/8/21 Norman Maurer<nor...@apache.org>:
Hi Tim,

comments inside..

2010/8/21 Tim-Christian Mundt<d...@tim-erwin.de>:
Norman,

you are right in that it was kinda double, so there should be some
cleanup. My first attempt would have been to remove the subscription
stuff from the MailboxManager (your option #1). The reason is that we
always have a manager and its respective mapper. Now we have the
MailboxManager with two Mappers. I think we should also merge them which
would also simplify the package structure because we wouldn't need
the .mail and .user packages anymore.
Thats true I just thought it would be more easy to have not to many
interfaces to implement. Anyway I would also be happy to move the
subscripe stuff to any extra interface. I just don't like to have it
duplicated so feel free to revert...

One more thing concerning naming and stuff: Now the
DelegatingMailboxManager is not really delegating anymore. Is there any
good reason we should keep it separate from StoreMailboxManager? If not
I'd rather have a little bigger class but fewer hierarchy levels.
I need to review..

Any thoughts?

Tim

Am Samstag, den 21.08.2010, 10:23 +0200 schrieb Norman Maurer:
I just committed the changes.. If anyone thinks its a bad idea we can
revert it anyway..

https://issues.apache.org/jira/browse/IMAP-205

Bye,
Norman

2010/8/21 Norman Maurer<nor...@apache.org>:
Hi there,

after looking again at the IMAP api I'm in favor of removing the
org.apache.james.imap.store.Subscriper interface and merge the
implementations with the MailboxManager implementations. Thats because
the Subsciper interface has 3 methods, all of the methods are already
in MailboxManager. So the MailboxManager just wraps the Subscriper
implementation and delegate the call to it.

So there are two solutions to this:

1) Remove the methods from MailboxManager and move the Subscriper
interface to the mailbox api
2) Remove the Subscriper interface from store api and merge the implementations

As I stated before I would prefer 2).

WDYT ?

Bye,
Norman

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org


Bye,
Norman

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org

Reply via email to