I think that violates the spirit, if not the letter, of the addHandler() contract. I add a handler and I get back a HandlerRegistration. I can use that registration to remove the handler later, if I choose to do so. If I expose that registration to other code, then the other code can also remove the handler. But if I guard my registration jealously, I expect that nobody else will have the ability to remove my handler. It seems to me that canceling all of the old handlers when the manager changes is a recipe for hard to find bugs, especially if changing HandlerManagers is not a common use case. You're going to have handlers suddenly stop receiving events and not be able to find an explicit removal.
On Thu, Feb 11, 2010 at 11:36 AM, Arthur Kalmenson <[email protected]> wrote: > But what if you have the case where you're setting the HandlerManager > specifically because you want to stop listening to all the events from > the previous HandlerManager. The idea of a stack of HandlerManagers > makes some sense if you want to have different HandlerManagers > handling events for different parts of a system. > > I understand the idea of making your own HandlerManager, but from a > design perspective, doesn't it make sense to just have a singleton > HandlerManager? Are there performance implications of having > everything go through one HandlerManager? > > All the best, > -- > Arthur Kalmenson > > > > On Wed, Feb 10, 2010 at 11:34 AM, Isaac Truett <[email protected]> wrote: >> The argument seems to revolve around changing HandlerManagers >> resulting in the loss of registered handlers. Is it possible that the >> HandlerManager and the set of registered handlers aren't really the >> same thing and need to be separated? Would everyone be happy if you >> could call setHandlerManager() and the new manager would still service >> the same set of handlers that existed previously? >> >> >> On Wed, Feb 10, 2010 at 10:59 AM, Ray Ryan <[email protected]> wrote: >>> I forgot about that nuance of your proposal. I like. >>> @jat: the stack idea is nifty, but expanding the scope of the patch even >>> worse than I am. And JL's proposal would let one implement that. >>> >>> On Wed, Feb 10, 2010 at 7:56 AM, John LaBanca <[email protected]> wrote: >>>> >>>> I still think my proposal solves this for both cases. We add a protected >>>> createHandlerManager, which is more restrictive because it is only called >>>> once per widget. >>>> We also make getHandlerManager() protected, which allows users to replace >>>> the HandlerManager if they really want to by overriding getHandlerManager >>>> to >>>> return one of many. It would be up to the developer to add a >>>> setHandlerManager() method in their own widgets that would change the >>>> return >>>> value of getHandlerManager(), so its intentionally more difficult and >>>> therefore forces the developer to think about it a little more. >>>> Thanks, >>>> John LaBanca >>>> [email protected] >>>> >>>> >>>> On Wed, Feb 10, 2010 at 10:44 AM, Sven Brunken >>>> <[email protected]> wrote: >>>>> >>>>> >>>>> On 10 Feb., 16:28, Ray Ryan <[email protected]> wrote: >>>>> > Sven, you're arguing both sides here. You want things to be more >>>>> > customizable in general, but with your specific patch you're trying to >>>>> > be as >>>>> > restrictive as you can to achieve your personal goal. >>>>> >>>>> Both patches have the same restriction, once a handlermanager is set >>>>> or the default one created, there is no way back to change it. I am >>>>> also ok with a setHandlerManager without restrictions. But than people >>>>> will have the problem that they will loose handlerregistrations, if >>>>> they dont know what they are doing (and yes, there are these people). >>>>> I wanted to make this patch as much fool proof as possible. It should >>>>> be customaziable, but in a way, that you cannot brake it. >>>>> >>>>> I dont think that you really need to change the handlermanager during >>>>> runtime after you set the first one. You also cannot change the >>>>> element of a widget once you set the first one. >>>>> >>>>> > >>>>> > I'm assuming that if we provide an unrestricted setHandlerManager we >>>>> > would >>>>> > also provide a getHandlerManager. It doesn't seem like rocket science >>>>> > to >>>>> > expect someone to check for an existing one if before clobbering it. >>>>> > I'm >>>>> > also assuming that these methods are protected, not part of every >>>>> > widget's >>>>> > public api. >>>>> Yes sure, protected. They should not be public. >>>>> >>>>> > >>>>> > I also would argue against providing both createHM and setHM. Redundant >>>>> > API >>>>> > is confusing API, another unfortunate trait of our widget set. Lazy >>>>> > creation >>>>> > can happen in the default implementation of getHM. A custom widget >>>>> > author >>>>> > could override that method to maintain laziness, or call setHM from >>>>> > their >>>>> > constructor to keep ours from ever seeing the light of day. >>>>> >>>>> I dont want to add boths. There is no need for it. One approach is >>>>> more than sufficient. With both ways you would be able to change the >>>>> default handlermanager (which is the goal in the end). >>>>> > >>>>> > On Tue, Feb 9, 2010 at 9:11 AM, Sven Brunken >>>>> > <[email protected]>wrote: >>>>> > >>>>> > >>>>> > >>>>> > > The danger is that if you add handlers, and than later in your code >>>>> > > call setHandlerManager with a new HandlerManager. If we dont check >>>>> > > for >>>>> > > a prior existing HandlerManager, all old HandlerRegistration's will >>>>> > > be >>>>> > > lost. If you dont know what happened, you will search forever why >>>>> > > your >>>>> > > old handlers are not working. >>>>> > >>>>> > > You wont have the problem with a createHandlerManager method. >>>>> > >>>>> > > On 9 Feb., 17:55, Ray Ryan <[email protected]> wrote: >>>>> > > > If you're right that swapping HMs midstream is a bad idea, I think >>>>> > > > you >>>>> > > need >>>>> > > > a better argument than "it's a bad idea." What's the actual danger? >>>>> > > > If >>>>> > > I'm >>>>> > > > making my own HM I'm already pretty savvy. Why tie my hands? >>>>> > >>>>> > > > But yes, if you win that point, I agree with the change to replace >>>>> > > setHM(HM) >>>>> > > > with HM createHM() >>>>> > >>>>> > > -- >>>>> > >http://groups.google.com/group/Google-Web-Toolkit-Contributors >>>>> > >>>>> > -- >>>>> > I wish this were a Wave >>>>> >>>>> -- >>>>> http://groups.google.com/group/Google-Web-Toolkit-Contributors >>>> >>>> -- >>>> http://groups.google.com/group/Google-Web-Toolkit-Contributors >>> >>> >>> -- >>> I wish this were a Wave >>> >>> -- >>> http://groups.google.com/group/Google-Web-Toolkit-Contributors >> >> -- >> http://groups.google.com/group/Google-Web-Toolkit-Contributors > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
