[jira] Commented: (WICKET-435) fix documentation errors and review models

["Eelco Hillenius (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WICKET-435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12485415 ] 

Eelco Hillenius commented on WICKET-435:
----------------------------------------

Also, there is still absolutely nothing written about how to migrate from the old to the new models (well, except for removing the parameter, but that's the least interesting part).

Please someone fill in the TODO at http://cwiki.apache.org/WICKET/migrate-13.html#Migrate-1.3-IModelchange

> fix documentation errors and review models
> ------------------------------------------
>
>                 Key: WICKET-435
>                 URL: https://issues.apache.org/jira/browse/WICKET-435
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket
>    Affects Versions: 1.3
>            Reporter: Eelco Hillenius
>             Fix For: 1.3
>
>
> Review the newly backported models for bugs in documentation. Also consider these remarks from this http://www.nabble.com/Model-quirks-tf3489227.html email:
> - PropertyModel#propertyType(Component)
> Is never called, has a @see to non existent method
> AbstractPropertyModel#propertyType(Component)
> - BoundCompoundPropertyModel#propertyType(Component)
> Ditto, makes BoundCompoundPropertyModel.Binding#type worthless
> - CompoundPropertyModel
> Since we don't have a ICompoundModel any longer, shouldn't this class be
> better named InheritablePropertyModel?
> - AttachedCompoundPropertyModel
> This inner class implements IInheritableModel, which is superfluous
> because in Component#initModel() it is tested on IWrapModel before ever
> being able to be used as an IInheritableModel.
> - IWrapModel and IWrapModel#getNestedModel()
> The name of this class and method are misguiding:
> Wrapping models inside other models is quite common in Wicket (see
> PropertyModel() javadoc).
> But this interface is mainly used as a marker for cases, where models
> are inherited "from components higher in the hierarchy" - see
> Component#initModel() and MarkupContainer#setModel().
> Why not call it IInheritedModel and #getInheritableModel() then?
> I know that the same interface is used for
> IAssingmentAwareModel#wrapOnAssignment(Component), but this method could
> equally well just return an IModel or be a #setComponent(Component).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Re: [jira] Commented: (WICKET-435) fix documentation errors and review models

[Johan Compagner <jc...@gmail.com>]

>
> Please someone fill in the TODO at
> http://cwiki.apache.org/WICKET/migrate-13.html#Migrate-1.3-IModelchange



busy doing that.



> - AttachedCompoundPropertyModel
> > This inner class implements IInheritableModel, which is superfluous
> > because in Component#initModel() it is tested on IWrapModel before ever
> > being able to be used as an IInheritableModel.


yes i think that can be removed. And it is also not a performance
improvement because
if the parent of parent of child that has the compound. when then also have
the compound directly
after the unwrap call. (so a child doesn't go to the deepest parent but only
to its parent
thinking about that if we deeply nest childs.. arent we creating to many
models now??

for (Component current = getParent(); current != null; current =
current.getParent())

this loop doesn't do anything i believe.

because the first call to the first parent:

IModel model = current.getModel();

will call initModel() of its own. That agian calls parent.getModel() and so
on and so on.
So the complete tree of parents will have a WrapModel assigned...

i think we shoiuld do this:

IModel model = current.model;

so directly get the variable. And if it is null then go directly to the
parent (then the loop really does do something)
only one problem with that as far as i can see. getModel() isn't final. So
it could be overridden and have there
own impl..


> IAssingmentAwareModel#wrapOnAssignment(Component), but this method could
> > equally well just return an IModel or be a #setComponent(Component).


no setComponent would be a bad thing.
Then assignableaware models couldn't be shared over multiply components
(they can  now in 1.2 see ResourceModel in 1.2 and 1.3 which is our
assigableaware 'example')

But the interesting questing is does
IAssingmentAwareModel#wrapOnAssignment(Component)
really need to give a IWrapModel back? Because if we say it doesn't it can
be a normal model
then we could rename IWrapModel and make it special for inheritable. And if
that is the case
i can in detach much easier clean those models (that they are reconstructed
on the next request)
so we dont have the memory overhead in the session of the wrapmodels.
And i dont have to have a flag that i know i can remove it.

hmm come to think about it. I don't need that flag we have the flag:

Component.onDetach()
{
  if (model instanceof IWrapModel && ((IWrapModel)model).getWrappedModel()
instanceof IComponentInhertiableModel)
  {
         model = null;
  }
}

done.

What would be the performance penalty (cpu and gc) when on large pages with
a parent and childs deeply nested and we
constantly generating all the models.. (by looping over the parents , now it
is even horrible because all the parents in between also are getting a
model)

johan