You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Matthijs Wensveen <m....@func.nl> on 2007/09/10 09:43:42 UTC
Form.updateFormComponentModels bug?
Hi,
According to the javadoc Form.updateFormComponentModels should update
all the models of all FormComponents in a form (without validation
presumably?). This sets all the model objects to the convertedInput
value (in FormComponent.updateModel), unfortunately convertedInput is
only set after a call to validate(). This really makes
updateFormComponentModels useless as a protected method. If validation
is really mandatory (which I doubt), processInput should be the only
callable method as it validates first and then updates the model.
In any case, the call to convertInput() inside validate() strikes me as
odd (the wrong place to call such a method).
Also, the javadoc of FormComponent.updateModel() states: ".. it expect
that the object is already converted through the convert() call", but
there is no convert() method in FormComponent.
Off-topic: Why are so much methods marked final? This might prevent
API-misuse but also prevents innovative _use_! I can understand you want
framework users to do it the wicket way, because they will probably only
spam the users list with questions that have obvious solutions (like me
:D ). But sometimes it's just frustrating.
Maybe this is a consequence of the fact that wicket forces you to
subclass components so much. It's just too tempting to override so much
methods. I love wicket (I really do!) but I'd rather see wicket more
event listener oriented. This would also make it easier to listen to
events that are very deeply nested (fetch the component, register a
listener), which is almost impossible now.
--
Matthijs Wensveen
Func. Internet Integration
W http://www.func.nl
T +31 20 4230000
F +31 20 4223500
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: Form.updateFormComponentModels bug?
Posted by Matthijs Wensveen <m....@func.nl>.
Johan Compagner wrote:
>> The extra memory for the ClickListener is negligible and does give you
>> more flexibility because now you can have more than one interested party
>> for the link click. My use case (fetch nested component, add listener)
>> was already answered sufficiently by Martijn (bad OO, components should
>> publish own events that make sense in the problem domain), but I still
>> an eventlistener-based design could have merit (see the move towards
>> Delegation Event Model in AWT:
>> http://java.sun.com/j2se/1.3/docs/guide/awt/designspec/events.html).
>> While wicket is not swing or awt, it's an interesting read.
>>
>
>
>
> with the link abstract class you don't have any extra memory overhead
> but with the listener interface you have quite a lot because you need
> and extra reference in link itself, that holds an extra List (that again has
> its own stuff)
>
The trick in awt is to only hold one reference to a listener that
recursively fires the event on registered listeners (see
AWTEventMultiCaster)
> and then you have an extra instance of the listener itself which by itself
> has its internal representation
> Don't underestimate this. especially because we also can use them in
> tableviews with a lot of cells.
>
> But as i said, if you want a listener interface then you can build it quite
> easily
>
True, but of course my goal is not to create a fork, but to discuss
different views on the used event model.
I'm not an expert on memory or profiling, so I'll believe you. I do
think that such a choice should not only be made on the basis of memory
usage. But since the choice is already made, I'll go with it until I can
find a more convincing argument (although the link I supplied has some
quite convincing ones).
Matthijs
--
Matthijs Wensveen
Func. Internet Integration
W http://www.func.nl
T +31 20 4230000
F +31 20 4223500
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: Form.updateFormComponentModels bug?
Posted by Johan Compagner <jc...@gmail.com>.
>
> The extra memory for the ClickListener is negligible and does give you
> more flexibility because now you can have more than one interested party
> for the link click. My use case (fetch nested component, add listener)
> was already answered sufficiently by Martijn (bad OO, components should
> publish own events that make sense in the problem domain), but I still
> an eventlistener-based design could have merit (see the move towards
> Delegation Event Model in AWT:
> http://java.sun.com/j2se/1.3/docs/guide/awt/designspec/events.html).
> While wicket is not swing or awt, it's an interesting read.
with the link abstract class you don't have any extra memory overhead
but with the listener interface you have quite a lot because you need
and extra reference in link itself, that holds an extra List (that again has
its own stuff)
and then you have an extra instance of the listener itself which by itself
has its internal representation
Don't underestimate this. especially because we also can use them in
tableviews with a lot of cells.
But as i said, if you want a listener interface then you can build it quite
easily
johan
Re: Form.updateFormComponentModels bug?
Posted by Igor Vaynberg <ig...@gmail.com>.
On 9/10/07, Matthijs Wensveen <m....@func.nl> wrote:
>
> Okay, I get the whole conversion is validation thingy. Not entirely
> convinced whether validation is the only place where conversion should
> take place, but I'll leave this subject for now. It's already a lot
> clearer to me now.
it is done this way because conversion might be expensive so we only want to
do it once and cache the result. there is no sense performing conversion
once to see if it works, and then another time to update the model on the
exact same input.
-igor
>
> >> We have this also because it preserves memory, If you have to make an
> >>
> >>> (inner)class instance
> >>> and a link instance and attach those 2 each other that can consume
> much
> >>>
> >> more
> >>
> >>> memory
> >>> then when just making a new class and have there one instance.
> >>>
> >>>
> >> I'm not sure I see what you mean, but doesn't the (inner) class also
> >> need instantiation, consuming just as much memory?
> >>
> >
> >
> >
> > Yes but only 1 instance.. for example:
> >
> > Link link = new Link()
> > {
> > onClick()
> > {
> > // do your thing
> > }
> > }
> >
> > or
> >
> > Link link = new Link();
> > link.add(new LinkClickListener()
> > {
> > onClick()
> > {
> > // do you thing.
> > }
> > });
> >
> > the second approach makes 2 instances and link has now also a special
> extra
> > map to hold the listener.
> >
> > johan
> >
> >
>
> The extra memory for the ClickListener is negligible and does give you
> more flexibility because now you can have more than one interested party
> for the link click. My use case (fetch nested component, add listener)
> was already answered sufficiently by Martijn (bad OO, components should
> publish own events that make sense in the problem domain), but I still
> an eventlistener-based design could have merit (see the move towards
> Delegation Event Model in AWT:
> http://java.sun.com/j2se/1.3/docs/guide/awt/designspec/events.html).
> While wicket is not swing or awt, it's an interesting read.
>
> Matthijs
>
> --
> Matthijs Wensveen
> Func. Internet Integration
> W http://www.func.nl
> T +31 20 4230000
> F +31 20 4223500
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>
Re: Form.updateFormComponentModels bug?
Posted by Matthijs Wensveen <m....@func.nl>.
Okay, I get the whole conversion is validation thingy. Not entirely
convinced whether validation is the only place where conversion should
take place, but I'll leave this subject for now. It's already a lot
clearer to me now.
>
>> We have this also because it preserves memory, If you have to make an
>>
>>> (inner)class instance
>>> and a link instance and attach those 2 each other that can consume much
>>>
>> more
>>
>>> memory
>>> then when just making a new class and have there one instance.
>>>
>>>
>> I'm not sure I see what you mean, but doesn't the (inner) class also
>> need instantiation, consuming just as much memory?
>>
>
>
>
> Yes but only 1 instance.. for example:
>
> Link link = new Link()
> {
> onClick()
> {
> // do your thing
> }
> }
>
> or
>
> Link link = new Link();
> link.add(new LinkClickListener()
> {
> onClick()
> {
> // do you thing.
> }
> });
>
> the second approach makes 2 instances and link has now also a special extra
> map to hold the listener.
>
> johan
>
>
The extra memory for the ClickListener is negligible and does give you
more flexibility because now you can have more than one interested party
for the link click. My use case (fetch nested component, add listener)
was already answered sufficiently by Martijn (bad OO, components should
publish own events that make sense in the problem domain), but I still
an eventlistener-based design could have merit (see the move towards
Delegation Event Model in AWT:
http://java.sun.com/j2se/1.3/docs/guide/awt/designspec/events.html).
While wicket is not swing or awt, it's an interesting read.
Matthijs
--
Matthijs Wensveen
Func. Internet Integration
W http://www.func.nl
T +31 20 4230000
F +31 20 4223500
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: Form.updateFormComponentModels bug?
Posted by Johan Compagner <jc...@gmail.com>.
>
>
> Okay, so the difference with Form.process() is that process() does not
> update at all when one of the FormComponents is invalid, and
> updateFormComponentModels updates all that ARE valid, right? So that's
> usually what you want (and what I want in this case). Thanks for making
> this clear.
Yes if one fails no models are updated at all. Only when everything is
going ok
the models are updated.
In Form.process() where this method is called, the form is validated
> first, so conversion took place as a side effect of validation. There I
> can see how it works. Maybe the javadoc of
> Form.updateFormComponentModels should state that Form.validate should be
> called first. Form.validate does state that: "This method is typically
> called before updating any models." which is good.
i changed it to this:
/**
* Update the model of all form components using the fields that were
sent with the current
* request. This method only updates models when the Form.validate() is
called first that takes
* care of the conversion for the FormComponents.
*
* Normally this method will not be called when a validation error
occurs in one of the form
* components.
*
* @see org.apache.wicket.markup.html.form.FormComponent#updateModel()
*/
> We could split up the methods again, like in Form.process()
> > which calls the above validate only. We could split that up in to
> > validateRequired(), validateConversion(), validateXX()
> > but what do we gain then?
> >
>
> Readability!
>
> When a method is called validate() I expect it to validate, and validate
> ONLY. Conversion of input is a side effect of validation.
no checking for required is validation.
type conversion is validation it, validates if it can convert '10.45455dddd'
to an integer..
as you can see in the method:
/**
* Performs full validation of the form component, which consists of
calling validateRequired(),
* convertInput(), and validateValidators(). This method should only be
used if the form
* component needs to be fully validated outside the form process.
*/
public final void validate()
{
validateRequired();
if (isValid())
{
convertInput();
if (isValid() && isRequired() && getConvertedInput() == null &&
isInputNullable())
{
reportRequiredError();
}
if (isValid())
{
validateValidators();
}
}
}
after every state it also calls isValid() so that it fails early and with a
right error.
> Ehmokay. Do I really need to file a bug for this small documentation
> bug? Done: https://issues.apache.org/jira/browse/WICKET-951
/**
* Updates this components model from the request, it expects that the
object is already
* converted through the convertInput() call that is called by the
validate() method when a form
* is being processed.
*
* By default it just does this:
*
* <pre>
* setModelObject(getConvertedInput());
* </pre>
*
* DO NOT CALL THIS METHOD DIRECTLY UNLESS YOU ARE SURE WHAT YOU ARE
DOING. USUALLY UPDATING
* YOUR MODEL IS HANDLED BY THE FORM, NOT DIRECTLY BY YOU.
*/
> We have this also because it preserves memory, If you have to make an
> > (inner)class instance
> > and a link instance and attach those 2 each other that can consume much
> more
> > memory
> > then when just making a new class and have there one instance.
> >
> I'm not sure I see what you mean, but doesn't the (inner) class also
> need instantiation, consuming just as much memory?
Yes but only 1 instance.. for example:
Link link = new Link()
{
onClick()
{
// do your thing
}
}
or
Link link = new Link();
link.add(new LinkClickListener()
{
onClick()
{
// do you thing.
}
});
the second approach makes 2 instances and link has now also a special extra
map to hold the listener.
johan
Re: Form.updateFormComponentModels bug?
Posted by Matthijs Wensveen <m....@func.nl>.
Johan Compagner wrote:
> On 9/10/07, Matthijs Wensveen <m....@func.nl> wrote:
>
>> Hi,
>>
>> According to the javadoc Form.updateFormComponentModels should update
>> all the models of all FormComponents in a form (without validation
>> presumably?). This sets all the model objects to the convertedInput
>>
>
>
>
> no it updates the form component models where conversion and validation did
> go alright.
>
Okay, so the difference with Form.process() is that process() does not
update at all when one of the FormComponents is invalid, and
updateFormComponentModels updates all that ARE valid, right? So that's
usually what you want (and what I want in this case). Thanks for making
this clear.
In Form.process() where this method is called, the form is validated
first, so conversion took place as a side effect of validation. There I
can see how it works. Maybe the javadoc of
Form.updateFormComponentModels should state that Form.validate should be
called first. Form.validate does state that: "This method is typically
called before updating any models." which is good.
>
> value (in FormComponent.updateModel), unfortunately convertedInput is
>
>> only set after a call to validate(). This really makes
>> updateFormComponentModels useless as a protected method. If validation
>>
>
>
>
> It is only protected final if you want to do some of your own processing
> see IFormSubmittingComponent.getDefaultFormProcessing()
>
>
> is really mandatory (which I doubt), processInput should be the only
>
>> callable method as it validates first and then updates the model.
>>
>
>
>
> checking required, conversion and validation is mandatory before updating
> models.
>
>
> In any case, the call to convertInput() inside validate() strikes me as
>
>> odd (the wrong place to call such a method).
>>
>
>
>
> no not really, FormComponent.validate() is really all the validation that
> can happen
> first required validation, then conversion validation and then user
> validation on the converted type
>
> We could split up the methods again, like in Form.process()
> which calls the above validate only. We could split that up in to
> validateRequired(), validateConversion(), validateXX()
> but what do we gain then?
>
Readability!
When a method is called validate() I expect it to validate, and validate
ONLY. Conversion of input is a side effect of validation.
>
> Also, the javadoc of FormComponent.updateModel() states: ".. it expect
>
>> that the object is already converted through the convert() call", but
>> there is no convert() method in FormComponent.
>>
>
>
> thats then an error in the doc, there are some changes in that area so that
> should be improved
> make a jira issue for this.
>
Ehmokay. Do I really need to file a bug for this small documentation
bug? Done: https://issues.apache.org/jira/browse/WICKET-951
>
> Maybe this is a consequence of the fact that wicket forces you to
>
>> subclass components so much. It's just too tempting to override so much
>> methods. I love wicket (I really do!) but I'd rather see wicket more
>> event listener oriented. This would also make it easier to listen to
>> events that are very deeply nested (fetch the component, register a
>> listener), which is almost impossible now
>>
>
>
>
> If you want listeners like swing then that is very easy for you to make
> just extend link or what ever component or behavior you have once
> and make the addXXXX/removeXXX and fireXXXX methods on them.
>
> We have this also because it preserves memory, If you have to make an
> (inner)class instance
> and a link instance and attach those 2 each other that can consume much more
> memory
> then when just making a new class and have there one instance.
>
I'm not sure I see what you mean, but doesn't the (inner) class also
need instantiation, consuming just as much memory?
Thanks for clearing some things up for me.
Matthijs
--
Matthijs Wensveen
Func. Internet Integration
W http://www.func.nl
T +31 20 4230000
F +31 20 4223500
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: Form.updateFormComponentModels bug?
Posted by Johan Compagner <jc...@gmail.com>.
On 9/10/07, Matthijs Wensveen <m....@func.nl> wrote:
>
> Hi,
>
> According to the javadoc Form.updateFormComponentModels should update
> all the models of all FormComponents in a form (without validation
> presumably?). This sets all the model objects to the convertedInput
no it updates the form component models where conversion and validation did
go alright.
value (in FormComponent.updateModel), unfortunately convertedInput is
> only set after a call to validate(). This really makes
> updateFormComponentModels useless as a protected method. If validation
It is only protected final if you want to do some of your own processing
see IFormSubmittingComponent.getDefaultFormProcessing()
is really mandatory (which I doubt), processInput should be the only
> callable method as it validates first and then updates the model.
checking required, conversion and validation is mandatory before updating
models.
In any case, the call to convertInput() inside validate() strikes me as
> odd (the wrong place to call such a method).
no not really, FormComponent.validate() is really all the validation that
can happen
first required validation, then conversion validation and then user
validation on the converted type
We could split up the methods again, like in Form.process()
which calls the above validate only. We could split that up in to
validateRequired(), validateConversion(), validateXX()
but what do we gain then?
Also, the javadoc of FormComponent.updateModel() states: ".. it expect
> that the object is already converted through the convert() call", but
> there is no convert() method in FormComponent.
thats then an error in the doc, there are some changes in that area so that
should be improved
make a jira issue for this.
Maybe this is a consequence of the fact that wicket forces you to
> subclass components so much. It's just too tempting to override so much
> methods. I love wicket (I really do!) but I'd rather see wicket more
> event listener oriented. This would also make it easier to listen to
> events that are very deeply nested (fetch the component, register a
> listener), which is almost impossible now
If you want listeners like swing then that is very easy for you to make
just extend link or what ever component or behavior you have once
and make the addXXXX/removeXXX and fireXXXX methods on them.
We have this also because it preserves memory, If you have to make an
(inner)class instance
and a link instance and attach those 2 each other that can consume much more
memory
then when just making a new class and have there one instance.
johan