On Tue, Nov 25, 2014 at 2:01 AM, Martijn Dashorst <
martijn.dasho...@gmail.com> wrote:
> On Tue, Nov 25, 2014 at 12:11 AM, Sven Meier <s...@meiers.net> wrote:
> > Hi Martin,
> >
> >> isAttached() is a public method and it is valid to call inside load()
> >
> > that's a valid point to consider: But what purpose could it have to call
> > isAttached() in load()?
>
> Probably none, I'm playing devil's advocate here. It is one model we
> promote heavily to our users so I imagine it being quite popular. What
> could break if we move the attached = true; and people rely on it to
> be true at the beginning of load()?
>
I also don't see a use case for relying on attached being true during
load().
>
> But load() is probably not the problem here... detach is...
>
>
> /**
> * @see org.apache.wicket.model.IDetachable#detach()
> */
> @Override
> public void detach()
> {
> if (attached)
> {
> try
> {
> onDetach();
> }
> finally
> {
> attached = false;
> transientModelObject = null;
>
> log.debug("removed transient object for {}, requestCycle {}",
> this,
> RequestCycle.get());
> }
> }
> }
>
> now that attached = true comes after the call to load, the model is no
> longer attached. This means that any state built up during load() will
> no longer be detached (removed) when something goes awry during
> load().
>
Usually models are used to get the underlying model object, i.e.
myLDM.getObject().
So I don't expect an application to do something like:
MyLDM myLDM = ....
SomethingElse sthgElse = myLDM.getSomethingElse();
If sthgElse depends on load() then the only way to guarantee it is properly
initialized is to use #isAttached() in #getSomethingElse().
With the new improvement this should be even better than before.
We can add explanation to load()'s javadoc that the application should
clean up anything custom stored in the LDM itself
>
> Martijn
>
