Hi Guillaume,

I think your suggestion to move prevModel.detach() inside the check is good.
I don't see problems with it.

Please file a ticket with a patch/PR.

About #getModelImpl() and #setModelImpl() I think those should stay hidden
for the user code. Usually the applications should not modify these parts
of Wicket.
By making them protected inexperienced users may use them instead
#getModel()/#setModel().

Martin Grigorov
Wicket Training and Consulting


On Wed, Mar 19, 2014 at 4:46 AM, Guillaume Smet <[email protected]>wrote:

> Btw, it would be nice to make setModelImpl and getModelImpl protected
> because, right now, it's not really possible to override
> setDefaultModel() to change its behavior.
>
> As I'm already doing the detach thing myself as I'm switching between
> a finite number of models referenced in my component, I would like to
> override setDefaultModel to completely remove the detach behavior for
> this component.
>
> --
> Guillaume
>
> On Wed, Mar 19, 2014 at 3:08 AM, Guillaume Smet
> <[email protected]> wrote:
> > Hi,
> >
> > I don't see the point of detaching the prevModel if the model is not
> changing.
> >
> > I was thinking about changing the logic of Component.setDefaultModel
> like that:
> >     public Component setDefaultModel(final IModel<?> model)
> >     {
> >         IModel<?> prevModel = getModelImpl();
> >
> >         IModel<?> wrappedModel = prevModel;
> >         if (prevModel instanceof IWrapModel)
> >         {
> >             wrappedModel = ((IWrapModel<?>)prevModel).getWrappedModel();
> >         }
> >
> >         // Change model
> >         if (wrappedModel != model)
> >         {
> >             // Detach current model only if we really change the model
> >             if (prevModel != null)
> >             {
> >                   prevModel.detach();
> >             }
> >
> >             modelChanging();
> >             setModelImpl(wrap(model));
> >             modelChanged();
> >         }
> >
> >         return this;
> >     }
> >
> > Note that it's something that hurts us a lot in a particular pattern
> > we have here. We change the default model in onConfigure and every
> > time we do it it's detached even if it's the exact same model.
> >
> > We could add a check in our code but I'm thinking it could be better
> > to do it in Wicket directly.
> >
> > Thoughts?
> >
> > --
> > Guillaume
>

Reply via email to