if we don't care (there where people complaining about double detach calls..
(i think martijn itself was one))
then we can drop the if(ATTACHED) completely and just do everything in one
method
johan
On 3/26/07, Matej Knopp <[EMAIL PROTECTED]> wrote:
I don't see as a problem that onDetach would be called multiple times.
Yes, detachModels() is not final, but the goal is to merge it with
detach() thus simplifying the detaching a bit.
-Matej
On 3/26/07, Johan Compagner <[EMAIL PROTECTED]> wrote:
> detachModels is not final.
> So if you want to detach a model that be done in detachmodels...
>
> The only other solution is that onDetach is always called (but then also
> twice if there are multiply request targets on the stack with the same
page)
>
> But The biggest problem is attach itself. That should really be called
> earlier (not only for rendered components)
> But we can't really change that i am afraid.
>
> johan
>
> On 3/26/07, Matej Knopp <[EMAIL PROTECTED]> wrote:
> >
> > It only detachs the "default" component model, i.e. the one you set
> > with setModel. if you have something like this
> >
> > class MyComponent ... {
> > IModel myModel = ...
> >
> > }
> >
> > and you clean myModel in onDetach, that won't work. I'm not sure how
> > rare this is and if you assign mymodel to a component it will be
> > detached anyway, but there is the possibility.
> >
> > -Matej
> >
> > On 3/26/07, Johan Compagner <[EMAIL PROTECTED]> wrote:
> > > as far as i see it does already what i expect it todo:
> > >
> > >
> > > if (getFlag(FLAG_ATTACHED))
> > > {
> > > // if the component has been previously attached via
> > attach()
> > > // detach it now
> > > setFlag(FLAG_DETACHING, true);
> > > onDetach();
> > > if (getFlag(FLAG_DETACHING))
> > > {
> > > throw new IllegalStateException(
Component.class.getName
> > ()
> > > + " has not been properly detached.
Something in
> > the
> > > hierarchy of "
> > > + getClass().getName()
> > > + " has not called super.onDetach() in the
> > override
> > > of onDetach() method");
> > > }
> > > setFlag(FLAG_ATTACHED, false);
> > > }
> > >
> > > // always detach models because they can be attached without
the
> > > // component. eg component has a compoundpropertymodel and
one
> > of
> > > its
> > > // children component's getmodelobject is called
> > > detachModels();
> > >
> > > // always detach children because components can be attached
> > > // independently of their parents
> > > detachChildren();
> > > }
> > >
> > > So only if attached the onDetached is called
> > > But detachModels and detachChildren are always called.
> > >
> > > isn't that correct? Where does it go wrong now?
> > >
> > > johan
> > >
> > >
> > >
> > >
> > > On 3/26/07, Matej Knopp <[EMAIL PROTECTED]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > i think we should finally define onDetach() contract and decide
what
> > > > to do with detachModels().
> > > >
> > > > Right now, after the detach attach/refactor the situation is that
> > > > onDetach is called only for components which were attached and
> > > > detachModels is called for every component.
> > > >
> > > > While this kinda does make sense, there are two possible problems:
> > > > - the semantics are not very well defined and when which method is
> > > > called can be confusing
> > > > - when there is model in component that is not in the default
"slot"
> > > > (get/setModel), it will not be detached properly unless the whole
> > > > component was attached.
> > > >
> > > > The goal of attach/detach refactor was to simplify the things and
I
> > > > think we can go a little bit further. I propose merging
detachModels
> > > > with detach. And I also propose that onDetach should be called for
> > > > every component. IMHO it would be safer and simpler than what we
have
> > > > right now.
> > > >
> > > > The onDetach sematics would be defined something like this:
> > > > The method is called on the end of request, even if the component
was
> > > > not attached.
> > > >
> > > > The detach methods are usually rather simple, so I really don't
think
> > > > there would be a big performance penalty, as we already need to
> > > > traverse the component tree.
> > > >
> > > > WDYT?
> > > >
> > > > -Matej
> > > >
> > >
> >
>
