Yes, the refactorings of the model stuff looks excellent. The approach is clean, the nested model stuff solves the problem and there is still the clean conceptual different between ordinary models and detached models.
>
> > I tend to agree with Jon on this. We thought long and hard about
> > whether to have a separate interface and decided it was
> important for
> > the following reasons:
> >
> > - It makes a very clear distinction between simple models that get
> > stored in the session and detachable models that don't and
> thus forces
> > people to consider their selection of the best type of
> model for their
> > particular application characteristics. Rolling everything
> into model
> > just seems to blur what is currently a very clear choice
> >
> > - As you would always need to subclass in order to provide the
> > onAttach() and onDetach() methods I can't see any win in extending
> > Model as opposed to DetatchableModel. In fact I think it is
> more risky
> > because a developer may or may not remember to override both
> > onAttach() and onDetach() if subclassing models but they
> can be forced
> > to to this with detachable models so there is much less
> margin for error.
>
> please take a look at the refactorings i finished today. i spent all
> day working on and thinking about models (both models in wicket.model
> and those involved in choices). with the changes i've
> checked in (and i
> apologize hugely for any code that i broke!), i think we're
> in really,
> really good shape now.
>
> in fact, i think i feel a release candidate coming on this week! ;-)
>
> > The question that I have is why do we have any models that are not
> > associated with components? I would tend to consider this to be an
> > anti-pattern. The Wicket Model/DetachableModel concept has (in my
> > opinion) a very clear responsibility in providing the
> binding between
> > Wicket components and the underlying business data or
> objects. Thus,
> > using models for things other than this is outside the primary
> > contract of the interface. I would like to see some examples of the
> > exact use cases of why we need to use models that are not
> associated
> > with components and then to provide a mechanism, guidelines or
> > additional functionality that solves this problem without
> needing to
> > alter the existing model approach.
>
> the trouble is really just with nested models. i think the
> design i've
> got checked in is really pretty fool-proof in every
> direction. if i'm
> wrong about that, please point out the flaw in my cunning plan! ;-)
>
> i'm sorry for charging ahead here, but i was really inspired to work
> this problem today and the time difference makes it just about
> impossible to talk these things through one issue at a time.
>
> and most of all... i really want to get an RC out this week!
>
> if there are any problems with my changes, i'm happy to
> address them. i
> definitely don't want to lose any of the value we had. i just think
> what's in there now is really solid...
>
> jon
>
> >
> > -1 for changes to Model/DetachableModel
> > +1 for finding out why and stopping the anti-pattern of using models
> > that are not associated with Wicket components
> >
> > regards,
> > Chris
> >
> >
> > Jonathan Locke wrote:
> >
> >>
> >> i'm in favor of keeping the interface.
> >> isn't the only problem just that you need to call attach() in
> >> getObject()?
> >>
> >> for that problem, i like your solution of adding onGetObject() and
> >> onSetObject() and making getObject() final. but we can do this to
> >> DetachableModel without changing anything else.
> >>
> >> getObject would stay overridable in Model. and it would
> be final in
> >> DetachableModel, forcing users to implement onGetObject(). this
> >> seems like exactly the right thing. normal models implement
> >> getObject() like they always did and DetachableModels have to
> >> implement onGetObject() so we can do the attachment magic.
> make sense?
> >>
> >> or am i missing something?
> >>
> >> Eelco Hillenius wrote:
> >>
> >>> The models keep giving problems. Though a powerfull concept, they
> >>> are not perfect yet.
> >>>
> >>> The problem was: as attach() was called in
> Component.getModel(), it
> >>> was never called for models that were not coupled to
> components, or
> >>> that were called directly (e.g. when a component holds a
> reference
> >>> to the model directly itself).
> >>>
> >>> The problem now is: as the calling of attach() is now put in
> >>> AttachableModel.getObject(), the models are not attached
> >>> automatically when a client overrides getObject(). So he
> has to call
> >>> attach himself, or we should call attach from both
> >>> Component.getModel() and AttachableModel.getObject().
> >>>
> >>> This clearly isn't very nice either.
> >>>
> >>> Maybe we should make it all much simpler. I propose:
> >>>
> >>> 1. Loose the interfaces. The starting point is just one Model base
> >>> class that is smart enough to attach itself. Detaching
> still has to
> >>> come from outside... I can't think of anything nice here
> (note that
> >>> if you don't couple a model class to a component it will never be
> >>> detached for you).
> >>>
> >>> 2. Model could look like:
> >>>
> >>> public class Model extends AbstractModel
> >>> {
> >>> private Serializable object;
> >>>
> >>> private transient boolean attached = false;
> >>>
> >>> public Model()
> >>> {
> >>> }
> >>>
> >>> public Model(final Serializable object)
> >>> {
> >>> this.object = object;
> >>> }
> >>>
> >>> public final Object getObject()
> >>> {
> >>> attach();
> >>> return doGetObject();
> >>> }
> >>>
> >>> public Object doGetObject() // or any other name; but
> this one is
> >>> overridable
> >>> {
> >>> return object;
> >>> }
> >>>
> >>> public final void setObject(Object object)
> >>> {
> >>> doSetObject(object);
> >>> }
> >>>
> >>> public void doSetObject(Object object)
> >>> {
> >>> if (object != null)
> >>> {
> >>> if (!(object instanceof Serializable))
> >>> {
> >>> throw new WicketRuntimeException("Model object must
> >>> be Serializable");
> >>> }
> >>> }
> >>>
> >>> setObject((Serializable)object);
> >>> }
> >>>
> >>> public void setObject(Serializable object)
> >>> {
> >>> this.object = object;
> >>> }
> >>>
> >>> public final void attach()
> >>> {
> >>> if (!attached)
> >>> {
> >>> onAttach();
> >>> attached = true;
> >>> }
> >>> }
> >>>
> >>> public final void detach()
> >>> {
> >>> if (attached)
> >>> {
> >>> onDetach();
> >>> attached = false;
> >>> }
> >>> }
> >>> }
> >>>
> >>> Having this class would be enough for most uses. For advanced uses
> >>> we can still have NestedModels etc.
> >>>
> >>> I think having interfaces do not add much to the fun
> here. Sure it's
> >>> nice for users to be able to implement models with their
> own root
> >>> inherritence, but it's even better not to have to worry about
> >>> attaching. And, as you can allways write wrappers and
> such, it's not
> >>> that by not having interface we restrict users.
> >>>
> >>> Thoughts?
> >>>
> >>> Eelco
> >>>
> >>>
> >>> -------------------------------------------------------
> >>> SF email is sponsored by - The IT Product Guide
> >>> Read honest & candid reviews on hundreds of IT Products from real
> >>> users.
> >>> Discover which products truly live up to the hype. Start
> reading now.
> >>> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> >>> _______________________________________________
> >>> Wicket-develop mailing list
> >>> [email protected]
> >>> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >>>
> >>
> >>
> >> -------------------------------------------------------
> >> SF email is sponsored by - The IT Product Guide
> >> Read honest & candid reviews on hundreds of IT Products from real
> >> users. Discover which products truly live up to the hype. Start
> >> reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396> &op=click
> >>
> _______________________________________________
>
> >> Wicket-develop mailing list [email protected]
> >> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >>
> >
> >
> >
> > -------------------------------------------------------
> > SF email is sponsored by - The IT Product Guide
> > Read honest & candid reviews on hundreds of IT Products from real
> > users. Discover which products truly live up to the hype. Start
> > reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> > _______________________________________________
> > Wicket-develop mailing list [email protected]
> > https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >
>
>
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from
> real users. Discover which products truly live up to the
> hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396> &op=click
>
> _______________________________________________
>
> Wicket-develop mailing list [email protected]
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>
>
