You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Johan Compagner <jc...@gmail.com> on 2007/11/13 23:24:52 UTC
Should we expect that IWrapModel.detach() does always detach the model it wraps?
Hi,
now in Component.detachModel we do this:
// also detach the wrapped model of a component assigned wrap (not
// inherited)
*if* (model *instanceof* IWrapModel && !getFlag(*FLAG_INHERITABLE_MODEL*))
{
((IWrapModel)model).getWrappedModel().detach();
}
should we really do that there? Because there are quite al lot of places
where we have models stored in properties that we call this.myModel =
wrap(model) on.
and all or most of those places dont do what the normal model detach will do
and that is calling also detach on the wrapped model.
Now the question is should we do that anywhere so remove the code in
detachModel() and just expect that users will also implement detach
correctly?
We dont even do that in the current code base so i need to fix that. Also
then AbstractWrapModel can't have then an empty detach implementation
because
then people could easily miss it..
What we also can do is introduce a protected final detachModel(IModel) so
the "opposite" of protected final wrap(IModel) that doest the above check
and call that one on all the places we now have internal IModel fields.
johan
Re: Should we expect that IWrapModel.detach() does always detach the model it wraps?
Posted by Johan Compagner <jc...@gmail.com>.
how do you check that?
i already did some fixes so that internal
things do detach and also our AbstractWrapModel doesn't have an empty
detach any more
On 11/14/07, igor.vaynberg@gmail.com <ig...@gmail.com> wrote:
> so instead of detaching make it a check and throw an exception
>
> -igor
>
> On 11/13/07, Johan Compagner <jc...@gmail.com> wrote:
> > yeah thats nice to say, but thats not really up to us.
> > So we can make sure that it is really detached by calling it.
> > As i said even all our own interal wrapmodels don't do that currently, so
> it
> > is that easy to forget.
> >
> > johan
> >
> >
> >
> > On Nov 14, 2007 12:16 AM, Eelco Hillenius <ee...@gmail.com>
> wrote:
> >
> > > On Nov 13, 2007 3:03 PM, Igor Vaynberg <ig...@gmail.com> wrote:
> > > > i should always be able to simply call .detach() on any model and
> > > > expect it to be detached no matter how it was wrapped, etc
> > >
> > > +1
> > >
> > > Eelco
> > >
> >
>
Re: Should we expect that IWrapModel.detach() does always detach the model it wraps?
Posted by ig...@gmail.com.
so instead of detaching make it a check and throw an exception
-igor
On 11/13/07, Johan Compagner <jc...@gmail.com> wrote:
> yeah thats nice to say, but thats not really up to us.
> So we can make sure that it is really detached by calling it.
> As i said even all our own interal wrapmodels don't do that currently, so it
> is that easy to forget.
>
> johan
>
>
>
> On Nov 14, 2007 12:16 AM, Eelco Hillenius <ee...@gmail.com> wrote:
>
> > On Nov 13, 2007 3:03 PM, Igor Vaynberg <ig...@gmail.com> wrote:
> > > i should always be able to simply call .detach() on any model and
> > > expect it to be detached no matter how it was wrapped, etc
> >
> > +1
> >
> > Eelco
> >
>
Re: Should we expect that IWrapModel.detach() does always detach the model it wraps?
Posted by Johan Compagner <jc...@gmail.com>.
yeah thats nice to say, but thats not really up to us.
So we can make sure that it is really detached by calling it.
As i said even all our own interal wrapmodels don't do that currently, so it
is that easy to forget.
johan
On Nov 14, 2007 12:16 AM, Eelco Hillenius <ee...@gmail.com> wrote:
> On Nov 13, 2007 3:03 PM, Igor Vaynberg <ig...@gmail.com> wrote:
> > i should always be able to simply call .detach() on any model and
> > expect it to be detached no matter how it was wrapped, etc
>
> +1
>
> Eelco
>
Re: Should we expect that IWrapModel.detach() does always detach the model it wraps?
Posted by Eelco Hillenius <ee...@gmail.com>.
On Nov 13, 2007 3:03 PM, Igor Vaynberg <ig...@gmail.com> wrote:
> i should always be able to simply call .detach() on any model and
> expect it to be detached no matter how it was wrapped, etc
+1
Eelco
Re: Should we expect that IWrapModel.detach() does always detach the model it wraps?
Posted by Igor Vaynberg <ig...@gmail.com>.
i should always be able to simply call .detach() on any model and
expect it to be detached no matter how it was wrapped, etc
-igor
On Nov 13, 2007 2:24 PM, Johan Compagner <jc...@gmail.com> wrote:
> Hi,
>
> now in Component.detachModel we do this:
>
>
> // also detach the wrapped model of a component assigned wrap (not
>
> // inherited)
>
> *if* (model *instanceof* IWrapModel && !getFlag(*FLAG_INHERITABLE_MODEL*))
>
> {
>
> ((IWrapModel)model).getWrappedModel().detach();
>
> }
>
> should we really do that there? Because there are quite al lot of places
> where we have models stored in properties that we call this.myModel =
> wrap(model) on.
>
> and all or most of those places dont do what the normal model detach will do
> and that is calling also detach on the wrapped model.
>
> Now the question is should we do that anywhere so remove the code in
> detachModel() and just expect that users will also implement detach
> correctly?
>
> We dont even do that in the current code base so i need to fix that. Also
> then AbstractWrapModel can't have then an empty detach implementation
> because
>
> then people could easily miss it..
>
> What we also can do is introduce a protected final detachModel(IModel) so
> the "opposite" of protected final wrap(IModel) that doest the above check
>
> and call that one on all the places we now have internal IModel fields.
>
> johan
>