You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Eelco Hillenius (JIRA)" <ji...@apache.org> on 2007/03/30 07:16:25 UTC

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

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

Posted by 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

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

Posted by "Eelco Hillenius (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-435?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Eelco Hillenius resolved WICKET-435.
------------------------------------

    Resolution: Fixed

> 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.

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

Posted by "Eelco Hillenius (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12487069 ] 

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

Re: - CompoundPropertyModel
Since we don't have a ICompoundModel any longer, shouldn't this class be
better named InheritablePropertyModel? 

I think CompoundPropertyModel is still a good name. It implements IComponentInheritedModel, but that can be regarded as an implementation detail. 

> 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.

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

Posted by "Eelco Hillenius (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12487071 ] 

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

We had a bit of talk about the various alternatives to IWrapModel, but I think (and a few others) this is really the best name for it. Some names are better depending on the ocassion, but this one seems to be best in general. Changed getNestedModel to getWrappedModel

> 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.

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

Posted by "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.

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

Posted by "Eelco Hillenius (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12487080 ] 

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

Re: AbstractPropertyModel#propertyType

I think we should get rid of it. It's use is rather theoretic, and since you typically tell a form component what the target type should be, it may even conflict.

> 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.