You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Marc Portier <mp...@outerthought.org> on 2004/04/08 08:54:36 UTC
[cforms] widget values: set, get, validate, readfromrequest, parse,
fireEvents,... generateSAXEvent
Hi there,
just found a small glitch in the aggregate-binding which is somewhat
related to a broader discussion IMHO
(yeah, I know its related to the bug we closed only yesterday, just that
I found out a nicer test after that)
[just an apetizer]
The context under which I found the issue:
- definition holds an aggregate-widget (stolen from the aggregate
binding sample from Vadim) the thing has:
- @id='date'
- split parts 'day', 'month', 'year' all of type integer
- an own datatype of 'date',
- the month field has a selection-list mapping e.g. 4 to 'April'
- to the front end I style this widget to show the seperate parts in
stead of the one combined one
- on the backend I however have only one 'string' holding the date, so I
have a simple value-binding with a date-convertor that directly maps
that onto the 'date' widget
when I bind "07/04/2004" to the widget and show the form I get out the
tree html-inputfields you'ld expect, but with "string" not "integer"
values, i.e.
- day shows "07"
- month has a fi:value of "04" causing the select-box not to show
"April" but "pls select a month"
- year shows "2004"
the reason to this?
a setValue from the binding on the aggregate-field eventually calls the
setFieldsValues() in there, which splits the string-value of the
combined widget into it's parts and sets it on them by allowing them to
partField.readFromRequest(newValuePart) which will set on those fields
the 'enteredValue' (and mark the 'needsParse') but not the 'value'
between this and the moment that generateSAXEvent reads out the
split-part fields the value is never set (meaning the integer convertor
never comes into play) resulting in the sax snippet to smash in the
'enteredValue' since the value==null
of course the solution is clear to anyone: after letting the
readFromRequest() I should just getValue() on the part-fields
(honestly?
- getValue while I really want to set them?
- readFromRequest while there is no http-request at all?)
Index: AggregateField.java
===================================================================
RCS file:
/home/cvs/cocoon-2.1/src/blocks/forms/java/org/apache/cocoon/forms/formmodel/AggregateField.java,v
retrieving revision 1.3
diff -u -r1.3 AggregateField.java
--- AggregateField.java 9 Mar 2004 13:17:26 -0000 1.3
+++ AggregateField.java 8 Apr 2004 06:30:23 -0000
@@ -149,6 +149,7 @@
// Fields can have a non-string datatype, going to
the readFromRequest
Field field =
(Field)fieldsById.get(splitMapping.getFieldId());
field.readFromRequest(result);
+ field.getValue();
}
} else {
resetFieldsValues();
I'm a bit reluctant to check this in since as far as I understand this
will also fire the validation on the object, which (again AFAICS) should
be without danger in this case, but kinda blows high over what is really
needed here...
[the main course]
widgets are complex babes but our current code structure
(naming, atomicity and resposibility of its methods) isn't really
helping me... well, actually it might be useful for more people then
just me to clear the confusion out...
in stead of trying to guess from current code, I'ld like to get a hold
of what (and why) should happen from a higher level, and from there use
that knowledge to try clean up, refactor and make self explaining what
we have since now IMHO we've ended up in some historical soup which is
soon going to prevent us from effectively add features or fix bugs
if this is only my limited view of things just say so, in the other
case: who wants to take a first stab?
What is the state-machine behind the widgets/fields?
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org
Re: [cforms] validation errors on (was Re: [cforms]
widget values: set, get, validate, readfromrequest, parse, fireEvents,...
generateSAXEvent)
Posted by Marc Portier <mp...@outerthought.org>.
Vadim Gritsenko wrote:
> Marc Portier wrote:
> ...
>
>>> But where I expected to get the validation errors assembled in a way
>>> as well, they just get lost, due to the widget replacement (of
>>> <ft:aggregate-widget id="visa">), hmmm, unsatisfying. (or do I get
>>> the whole thing totally wrong?)
>>
>>
>>
>> if I get it correctly you're saying that the <ft:aggregate-widget>
>> after transformation and styling is not showing the top-level
>> validation errors
>
>
>
> <ft:aggregate-widget/> does not output anything into the stream, that's
> why validation errors are not shown on it. Either
> EffectWidgetReplacingPipe needs to be changed, or you can simply use
> <ft:validation-error/> to position and output aggregate widget's errors.
>
thx for clearing out Vadim,
given the fact that the positioning remains an issue I think the latter
is the correct way of working for aggregates that get split out in the
template
> Vadim
>
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org
Re: [cforms] validation errors on (was Re: [cforms]
widget values: set, get, validate, readfromrequest, parse, fireEvents,...
generateSAXEvent)
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Marc Portier wrote:
...
>> But where I expected to get the validation errors assembled in a way
>> as well, they just get lost, due to the widget replacement (of
>> <ft:aggregate-widget id="visa">), hmmm, unsatisfying. (or do I get
>> the whole thing totally wrong?)
>
>
> if I get it correctly you're saying that the <ft:aggregate-widget>
> after transformation and styling is not showing the top-level
> validation errors
<ft:aggregate-widget/> does not output anything into the stream, that's
why validation errors are not shown on it. Either
EffectWidgetReplacingPipe needs to be changed, or you can simply use
<ft:validation-error/> to position and output aggregate widget's errors.
Vadim
[cforms] validation errors on (was Re: [cforms] widget
values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent)
Posted by Marc Portier <mp...@outerthought.org>.
leo leonid wrote:
>
> On Apr 8, 2004, at 8:54 AM, Marc Portier wrote:
>
>> Hi there,
>>
>> just found a small glitch in the aggregate-binding which is somewhat
>> related to a broader discussion IMHO
>> (yeah, I know its related to the bug we closed only yesterday, just
>> that I found out a nicer test after that)
>>
>>
>> [just an apetizer]
>> The context under which I found the issue:
>> - definition holds an aggregate-widget (stolen from the aggregate
>> binding sample from Vadim) the thing has:
>>
>
> Thanks for pointing this out. I ran into the same problems, but lacking
> the proof, I assumed the problem must be my limited understanding of
well, I think my point is that we are getting into a position where
everyone develops a feeling of 'having a limited understanding'
simply put I hate it when I have to read every line of code to
understand what a method is doing, and IMO, Leo, in those cases we
should not hide behind some intelectual correct 'it might be my limited
understanding'
so feel free to point us to these occasions whenever you encounter them,
nobody should feel offended, right?
as argumented before: those cases prevent us (and potential new
contributors) to efficiently send in patches to fix bugs and add
functionality
> cforms, as it is usual the case, and potentially with this one, too:
>
> Well, apart from those datatype conversion issues, I'm malcontent with
> current aggregatefield and its widgets. Strictly speaking its the
> <ft:aggregate-widget/> that is IMHO insufficient conclusive. Since there
> was aggregatefield widget, "used to edit the value of multiple fields
> through one textbox" [wiki], I never considered it not complete without
> it's complementary widget.
>
> Then the aggregate-widget have been introduced, and even though it's
> misleading name, I expected it to fill the gap, say, it could be "used
> to edit the value of one textbox through multiple fields". Partly it
> does it, but to notice some of it's shortcomings, consider the following
> use case: you need way to input for a 16 digit credit card number that
> needs to be validated in multiple respects (required, number, mod10),
> very similar to the one in the form1 sample, but different in the way
> you present the form to the user. Cause you want to ease correctly
> composing such a long number by splitting it up to 4 fields, 4 digits
> each, that then gets reassembled and validated.
yep, that's the idea
> But where I expected to get the validation errors assembled in a way as
> well, they just get lost, due to the widget replacement (of
> <ft:aggregate-widget id="visa">), hmmm, unsatisfying. (or do I get the
> whole thing totally wrong?)
>
if I get it correctly you're saying that the <ft:aggregate-widget> after
transformation and styling is not showing the top-level validation errors
have you checked at which level we're missing this info?
1/ validation never executed on the form model level
2/ transformer doesn't inject the validation error in the fi: stream
3/ stylesheet ignores it
last remark makes me think we should probably provide some ft: tag to
explicitely position those top level validation messages?
also, your report sounds like you have the 'visa' test-case for this
ready? sharing it might give us a head-start to clear out (bugzilla
sounds useful for this?)
> /leo
>
regards,
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org
Re: [cforms] widget values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent
Posted by leo leonid <te...@leonid.de>.
On Apr 8, 2004, at 8:54 AM, Marc Portier wrote:
> Hi there,
>
> just found a small glitch in the aggregate-binding which is somewhat
> related to a broader discussion IMHO
> (yeah, I know its related to the bug we closed only yesterday, just
> that I found out a nicer test after that)
>
>
> [just an apetizer]
> The context under which I found the issue:
> - definition holds an aggregate-widget (stolen from the aggregate
> binding sample from Vadim) the thing has:
>
Thanks for pointing this out. I ran into the same problems, but lacking
the proof, I assumed the problem must be my limited understanding of
cforms, as it is usual the case, and potentially with this one, too:
Well, apart from those datatype conversion issues, I'm malcontent with
current aggregatefield and its widgets. Strictly speaking its the
<ft:aggregate-widget/> that is IMHO insufficient conclusive. Since
there was aggregatefield widget, "used to edit the value of multiple
fields through one textbox" [wiki], I never considered it not complete
without it's complementary widget.
Then the aggregate-widget have been introduced, and even though it's
misleading name, I expected it to fill the gap, say, it could be "used
to edit the value of one textbox through multiple fields". Partly it
does it, but to notice some of it's shortcomings, consider the
following use case: you need way to input for a 16 digit credit card
number that needs to be validated in multiple respects (required,
number, mod10), very similar to the one in the form1 sample, but
different in the way you present the form to the user. Cause you want
to ease correctly composing such a long number by splitting it up to 4
fields, 4 digits each, that then gets reassembled and validated.
But where I expected to get the validation errors assembled in a way as
well, they just get lost, due to the widget replacement (of
<ft:aggregate-widget id="visa">), hmmm, unsatisfying. (or do I get the
whole thing totally wrong?)
/leo
Re: [cforms] widget values: set, get, validate, readfromrequest,
parse, fireEvents,... generateSAXEvent
Posted by Bruno Dumon <br...@outerthought.org>.
On Mon, 2004-04-19 at 17:48, Joerg Heinicke wrote:
> Marc Portier <mpo <at> outerthought.org> writes:
>
> > I do propose:
> > - some refactoring of the kind
> > - introduce more granular methods
> > (e.g. to call parseValue() in stead of getValue() in all occasions
> > where the return is ignored anyway?)
>
> I found another reason that more granular methods are necessary:
>
> I have a field with a selection list and a submit button. The field is optional
> required based on the submit, i.e. if the submit button is pressed there must be
> something selected:
>
> <wd:field id="event">
> <wd:label><i18n:text key="events"/></wd:label>
> <wd:datatype base="string"/>
> <wd:validation>
> <wd:javascript>
> if (form.submitId == 'invoke' && widget.value == null) {
> // add ValidationError
> return false;
> }
> return true;
> </wd:javascript>
> </wd:validation>
> </wd:field>
> <wd:submit id="invoke" action-command="invoke" validate="false">
> <wd:label><i18n:text key="action.invoke"/></wd:label>
> </wd:submit>
>
> Problem: readFromRequest() causes validation on all widgets in their order in
> the form definition.
Which version are you using? I thought Sylvain fixed this a couple of
days ago.
> But readFromRequest() also causes the setting of
> form.submitWidget. So form.submitWidget is not set yet if the above much
> shortened custom validator is evaluated. I have to put the field definition
> 'event' to the end of the form.
>
> More granular methods help to solve the problem: 1st run through form model
> readFromRequest() -> also set form's submitWidget. 2nd run validate().
This separation has always been there (modulus bugs), exactly for your
use case.
--
Bruno Dumon http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
bruno@outerthought.org bruno@apache.org
Re: [cforms] widget values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent
Posted by Joerg Heinicke <jo...@gmx.de>.
Bruno Dumon <bruno <at> outerthought.org> writes:
> > Problem: readFromRequest() causes validation on all widgets in their order in
> > the form definition.
>
> Which version are you using? I thought Sylvain fixed this a couple of
> days ago.
Only the latest release 2.1.4. You mean the "early validation" on actions?
http://marc.theaimsgroup.com/?l=xml-cocoon-cvs&m=108193479001068&w=4
http://marc.theaimsgroup.com/?l=xml-cocoon-cvs&m=108189170017245&w=4
?? The cforms commit message already has the hint.
I hadn't this fix in mind as I do a "real" submit, not only an action. But,
yeah, it could of course be the same reason.
> > But readFromRequest() also causes the setting of
> > form.submitWidget. So form.submitWidget is not set yet if the above much
> > shortened custom validator is evaluated. I have to put the field definition
> > 'event' to the end of the form.
> >
> > More granular methods help to solve the problem: 1st run through form model
> > readFromRequest() -> also set form's submitWidget. 2nd run validate().
>
> This separation has always been there (modulus bugs), exactly for your
> use case.
Yes, sorry for the noise then.
Joerg
Re: [cforms] widget values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent
Posted by Joerg Heinicke <jo...@gmx.de>.
Marc Portier <mpo <at> outerthought.org> writes:
> I do propose:
> - some refactoring of the kind
> - introduce more granular methods
> (e.g. to call parseValue() in stead of getValue() in all occasions
> where the return is ignored anyway?)
I found another reason that more granular methods are necessary:
I have a field with a selection list and a submit button. The field is optional
required based on the submit, i.e. if the submit button is pressed there must be
something selected:
<wd:field id="event">
<wd:label><i18n:text key="events"/></wd:label>
<wd:datatype base="string"/>
<wd:validation>
<wd:javascript>
if (form.submitId == 'invoke' && widget.value == null) {
// add ValidationError
return false;
}
return true;
</wd:javascript>
</wd:validation>
</wd:field>
<wd:submit id="invoke" action-command="invoke" validate="false">
<wd:label><i18n:text key="action.invoke"/></wd:label>
</wd:submit>
Problem: readFromRequest() causes validation on all widgets in their order in
the form definition. But readFromRequest() also causes the setting of
form.submitWidget. So form.submitWidget is not set yet if the above much
shortened custom validator is evaluated. I have to put the field definition
'event' to the end of the form.
More granular methods help to solve the problem: 1st run through form model
readFromRequest() -> also set form's submitWidget. 2nd run validate().
Joerg
Re: [cforms] widget values: set, get, validate, readfromrequest,
parse, fireEvents,... generateSAXEvent
Posted by Marc Portier <mp...@outerthought.org>.
Tim Larson wrote:
> On Fri, Apr 09, 2004 at 02:16:43PM +0200, Marc Portier wrote:
>
>>Marc Portier wrote:
>
> <snip/>
>
>>>What is the state-machine behind the widgets/fields?
>
>
> This would make an excellent wiki page.
>
I'll give it an initial go then, we should add to it as we go along...
>
>>We jointly got the cforms internals code into a mess: too much features
>>have been hack-added (even worse: without updating javadocs) rather then
>>refactor-added.
>
> <snip/>
>
>>Wake up, people! This will not go away by ignoring it.
>>
>>I do propose:
>>- some refactoring of the kind
>> - introduce more granular methods
>> (e.g. to call parseValue() in stead of getValue() in all occasions
>> where the return is ignored anyway?)
>> - method/member renamings
>> - javadoc cleanups
>> - clean out the binding interface
>> (this will surely break class/new/union/ binding)
>> - solidify classes and interfaces by making immutables where possible
>>and appropriate (IMHO the form-definitions could benefit)
>>
>>- finally fix the binding of the repeaters
>> - add a row-identity aspect to the repeater-rows
>> - ATM I even think it should combine multivalue-binding since that
>>looks strongly like another (unneccesary?) fork of the repeater-binding code
>>
>>- get into adding the stuff from
>>http://wiki.cocoondev.org/Wiki.jsp?page=WoodyScratchpad
>
>
> +1 to the whole proposal above. We need to get out of "alpha", and
> this is the only way I see to get that to happen.
>
It would also expect this movement to be backed by a large enough group
asking questions and understanding and documenting more of the internals
>
>>While getting into above I'ld like us all to have the freedom to
>>(temporary) break things: the class, new, union, struct (possibly even
>>aggregate) comes to mind.
>
>
> +1 from me. I expect anyone else using those will welcome the better
> designs we are hammering out on the WoodyScratchpad wiki page.
>
>
>>This is not because I find those additions (nor the people that did it)
>>to be the target suspects for the current state of things.
>
>
> None taken ;)
>
LOL.
and thx, we can use all hands, I would of hated to see my explicit
mentioning to kill anyones motivation...
>
>>Plus, the more stuff we break, the more reason there will be for more
>>hands to help in! (now from who did I learn that :-))
>>
>>But seriouly: The above would be my main '-1' argumentation to anyone
>>proposing to do this in a separate CVS branch (just so you know)
>>(putting down some CFORMS_0.0.1 tag before on the cfroms block might be
>>meaningfull though)
>
>
> Yes, if we are going to break things to re-set the bones, we need to tag.
>
fair enough, any suggestions out there to a sensible name and scope for
this tag?
I'm -0 to link this to the 2.1.5 release, which means I'm open to other
opinions in this area.
To me there hasn't been any relation between actual sensible features or
milestones in the cforms block (clear 'alpha' state accross) and
Cocoon's release numbering, but I don't want to be so careless as to
ignore associations others might-have or want-to build up.
>
>>In any case looking for suspects is not the goal, nor is it to break
>>stuff. It rather is about creating a clean basis to re-add the needed
>>features of the class/new/union/struct/repeater-binding allong the lines
>>of how we've been discussing them lately.
>>
>>AFAICS it's this or keeping eternally waiting at the side of the
>>swimmingpool. (= mpo-logism for keeping cforms 'unstable')
>
>
> We need to finish the design and get CForms stable, especially since
> it is now the official forms framework. I am going to go back to your
> "Cubist Webapp" concepts and am trying to create some wiki page(s) to
> express the CForms design from the standpoint of the user, visual
> designer, event handler programmer, and data binding programmer.
>
cool, I've started thinking some on the build-and-definition-process of
the 'reusable' widgets, I was going to add that to the WoodyScratchpad page.
regards,
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org
Re: [cforms] widget values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent
Posted by Tim Larson <ti...@keow.org>.
On Fri, Apr 09, 2004 at 02:16:43PM +0200, Marc Portier wrote:
> Marc Portier wrote:
<snip/>
> >What is the state-machine behind the widgets/fields?
This would make an excellent wiki page.
> We jointly got the cforms internals code into a mess: too much features
> have been hack-added (even worse: without updating javadocs) rather then
> refactor-added.
<snip/>
> Wake up, people! This will not go away by ignoring it.
>
> I do propose:
> - some refactoring of the kind
> - introduce more granular methods
> (e.g. to call parseValue() in stead of getValue() in all occasions
> where the return is ignored anyway?)
> - method/member renamings
> - javadoc cleanups
> - clean out the binding interface
> (this will surely break class/new/union/ binding)
> - solidify classes and interfaces by making immutables where possible
> and appropriate (IMHO the form-definitions could benefit)
>
> - finally fix the binding of the repeaters
> - add a row-identity aspect to the repeater-rows
> - ATM I even think it should combine multivalue-binding since that
> looks strongly like another (unneccesary?) fork of the repeater-binding code
>
> - get into adding the stuff from
> http://wiki.cocoondev.org/Wiki.jsp?page=WoodyScratchpad
+1 to the whole proposal above. We need to get out of "alpha", and
this is the only way I see to get that to happen.
> While getting into above I'ld like us all to have the freedom to
> (temporary) break things: the class, new, union, struct (possibly even
> aggregate) comes to mind.
+1 from me. I expect anyone else using those will welcome the better
designs we are hammering out on the WoodyScratchpad wiki page.
> This is not because I find those additions (nor the people that did it)
> to be the target suspects for the current state of things.
None taken ;)
> Plus, the more stuff we break, the more reason there will be for more
> hands to help in! (now from who did I learn that :-))
>
> But seriouly: The above would be my main '-1' argumentation to anyone
> proposing to do this in a separate CVS branch (just so you know)
> (putting down some CFORMS_0.0.1 tag before on the cfroms block might be
> meaningfull though)
Yes, if we are going to break things to re-set the bones, we need to tag.
> In any case looking for suspects is not the goal, nor is it to break
> stuff. It rather is about creating a clean basis to re-add the needed
> features of the class/new/union/struct/repeater-binding allong the lines
> of how we've been discussing them lately.
>
> AFAICS it's this or keeping eternally waiting at the side of the
> swimmingpool. (= mpo-logism for keeping cforms 'unstable')
We need to finish the design and get CForms stable, especially since
it is now the official forms framework. I am going to go back to your
"Cubist Webapp" concepts and am trying to create some wiki page(s) to
express the CForms design from the standpoint of the user, visual
designer, event handler programmer, and data binding programmer.
--Tim Larson
Re: testing (was Re: [cforms] widget values: set, get, validate,
readfromrequest, parse, fireEvents,... generateSAXEvent)
Posted by Marc Portier <mp...@outerthought.org>.
Ugo Cei wrote:
> Il giorno 09/apr/04, alle 16:11, Marc Portier ha scritto:
>
>>
>>
>> Ugo Cei wrote:
>>
>>> Il giorno 09/apr/04, alle 14:16, Marc Portier ha scritto:
>>>
>>>> more?
>>>
>>> Write tests, maybe? ;-)
>>
>>
>> agree, in fact I'm just looking at most of the samples being broken
>> ATM (hoping it's only local)
>>
>> do you have any suggestions on a sensible template for those tests?
>
>
> I'm not sure I understand what you mean by "template" here. When I did a
I meant 'pattern', way of working,
hopefully described in a wiki page :-)
> contribution to Woody, I wrote a small set of Junit test cases for it
> (see src/blocks/forms/test/org/apache/cocoon/forms/datatype), extending
> ExcaliburTestCase. Another approach is to use Anteater. I don't think
> there's a generic template that you can reuse easily.
>
too bad, having some proclaimed guidelines/cookbook-receipes might get
us past the "I know I should but, I lack the spirit to investigate how I
should" feeling
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org
testing (was Re: [cforms] widget values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent)
Posted by Ugo Cei <ug...@apache.org>.
Il giorno 09/apr/04, alle 16:11, Marc Portier ha scritto:
>
>
> Ugo Cei wrote:
>> Il giorno 09/apr/04, alle 14:16, Marc Portier ha scritto:
>>> more?
>> Write tests, maybe? ;-)
>
> agree, in fact I'm just looking at most of the samples being broken
> ATM (hoping it's only local)
>
> do you have any suggestions on a sensible template for those tests?
I'm not sure I understand what you mean by "template" here. When I did
a contribution to Woody, I wrote a small set of Junit test cases for it
(see src/blocks/forms/test/org/apache/cocoon/forms/datatype), extending
ExcaliburTestCase. Another approach is to use Anteater. I don't think
there's a generic template that you can reuse easily.
<rant>
Actually, I was just pointing out that, without a comprehensive suite
of tests, it's very hard to keep sanity in our codebase and understand
what's going on. I might go as far as to suggest that we shouldn't
accept new components and blocks without a comprehensive accompanying
test suite. And the same goes for refactorings and bugfixes on stable
blocks and the core.
I know I am guilty myself of occasional sloppiness, but when I see an
entire new "kernel" for our next generation container being committed
without a single test case, I cannot but feel a little worried.
</rant>
Ugo
Re: [cforms] widget values: set, get, validate, readfromrequest,
parse, fireEvents,... generateSAXEvent
Posted by Marc Portier <mp...@outerthought.org>.
Ugo Cei wrote:
> Il giorno 09/apr/04, alle 14:16, Marc Portier ha scritto:
>
>> more?
>
>
> Write tests, maybe? ;-)
>
agree, in fact I'm just looking at most of the samples being broken ATM
(hoping it's only local)
do you have any suggestions on a sensible template for those tests?
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org
Re: [cforms] widget values: set, get, validate, readfromrequest, parse, fireEvents,... generateSAXEvent
Posted by Ugo Cei <ug...@apache.org>.
Il giorno 09/apr/04, alle 14:16, Marc Portier ha scritto:
> more?
Write tests, maybe? ;-)
> Ugo
Re: [cforms] widget values: set, get, validate, readfromrequest,
parse, fireEvents,... generateSAXEvent
Posted by Joerg Heinicke <jo...@gmx.de>.
On 09.04.2004 14:16, Marc Portier wrote:
> We jointly got the cforms internals code into a mess: too much features
> have been hack-added (even worse: without updating javadocs) rather then
> refactor-added.
> I do propose:
> - some refactoring of the kind
> - introduce more granular methods
> (e.g. to call parseValue() in stead of getValue() in all occasions
> where the return is ignored anyway?)
> - method/member renamings
> - javadoc cleanups
> - clean out the binding interface
> (this will surely break class/new/union/ binding)
> - solidify classes and interfaces by making immutables where possible
> and appropriate (IMHO the form-definitions could benefit)
>
> - finally fix the binding of the repeaters
> - add a row-identity aspect to the repeater-rows
> - ATM I even think it should combine multivalue-binding since that
> looks strongly like another (unneccesary?) fork of the repeater-binding
> code
>
> - get into adding the stuff from
> http://wiki.cocoondev.org/Wiki.jsp?page=WoodyScratchpad
+1 I also got the feeling of mingled functionality in some methods (e.g.
getValue() causing always also the validation) when diving into the code
for finding out the reason for the to-early validation on wd:action.
> While getting into above I'ld like us all to have the freedom to
> (temporary) break things: the class, new, union, struct (possibly even
> aggregate) comes to mind.
>
> This is not because I find those additions (nor the people that did it)
> to be the target suspects for the current state of things.
>
> Plus, the more stuff we break, the more reason there will be for more
> hands to help in! (now from who did I learn that :-))
Huh? ;-)
> But seriouly: The above would be my main '-1' argumentation to anyone
> proposing to do this in a separate CVS branch (just so you know)
> (putting down some CFORMS_0.0.1 tag before on the cfroms block might be
> meaningfull though)
+1 for the tag. Maybe after 2.1.5 release, otherwise the release should
include the tagged version then.
> In any case looking for suspects is not the goal, nor is it to break
> stuff. It rather is about creating a clean basis to re-add the needed
> features of the class/new/union/struct/repeater-binding allong the lines
> of how we've been discussing them lately.
>
> AFAICS it's this or keeping eternally waiting at the side of the
> swimmingpool. (= mpo-logism for keeping cforms 'unstable')
Same feelings here.
Joerg
Re: [cforms] widget values: set, get, validate, readfromrequest,
parse, fireEvents,... generateSAXEvent
Posted by Marc Portier <mp...@outerthought.org>.
Marc Portier wrote:
> Hi there,
>
> just found a small glitch in the aggregate-binding which is somewhat
> related to a broader discussion IMHO
> (yeah, I know its related to the bug we closed only yesterday, just that
> I found out a nicer test after that)
>
now that the sample is in cvs, I opened a new bug for this specific
issue: http://nagoya.apache.org/bugzilla/show_bug.cgi?id=28307
>
> [just an apetizer]
>
<snip cause="now you can check for yourself with the new aggregate sample"/>
But the actual rant still holds:
> I'm a bit reluctant to check this in since as far as I understand this
> will also fire the validation on the object, which (again AFAICS) should
> be without danger in this case, but kinda blows high over what is really
> needed here...
>
>
>
> [the main course]
> widgets are complex babes but our current code structure
> (naming, atomicity and resposibility of its methods) isn't really
> helping me... well, actually it might be useful for more people then
> just me to clear the confusion out...
>
> in stead of trying to guess from current code, I'ld like to get a hold
> of what (and why) should happen from a higher level, and from there use
> that knowledge to try clean up, refactor and make self explaining what
> we have since now IMHO we've ended up in some historical soup which is
> soon going to prevent us from effectively add features or fix bugs
>
> if this is only my limited view of things just say so, in the other
> case: who wants to take a first stab?
>
> What is the state-machine behind the widgets/fields?
>
Hurray!
The more I dive into the code the merrier I get!
We jointly got the cforms internals code into a mess: too much features
have been hack-added (even worse: without updating javadocs) rather then
refactor-added.
<irony of-type="needless, but hey: I'm human.">
Adding to it the extra challenge to follow up on history and
argumentation thanx to a very important woody->cforms rename on a
version management system that doesn't support directory renames and
guess what: it really looks like we should get 2.1.5 out asap.
</irony>
Wake up, people! This will not go away by ignoring it.
I do propose:
- some refactoring of the kind
- introduce more granular methods
(e.g. to call parseValue() in stead of getValue() in all occasions
where the return is ignored anyway?)
- method/member renamings
- javadoc cleanups
- clean out the binding interface
(this will surely break class/new/union/ binding)
- solidify classes and interfaces by making immutables where possible
and appropriate (IMHO the form-definitions could benefit)
- finally fix the binding of the repeaters
- add a row-identity aspect to the repeater-rows
- ATM I even think it should combine multivalue-binding since that
looks strongly like another (unneccesary?) fork of the repeater-binding code
- get into adding the stuff from
http://wiki.cocoondev.org/Wiki.jsp?page=WoodyScratchpad
more?
While getting into above I'ld like us all to have the freedom to
(temporary) break things: the class, new, union, struct (possibly even
aggregate) comes to mind.
This is not because I find those additions (nor the people that did it)
to be the target suspects for the current state of things.
Plus, the more stuff we break, the more reason there will be for more
hands to help in! (now from who did I learn that :-))
But seriouly: The above would be my main '-1' argumentation to anyone
proposing to do this in a separate CVS branch (just so you know)
(putting down some CFORMS_0.0.1 tag before on the cfroms block might be
meaningfull though)
In any case looking for suspects is not the goal, nor is it to break
stuff. It rather is about creating a clean basis to re-add the needed
features of the class/new/union/struct/repeater-binding allong the lines
of how we've been discussing them lately.
AFAICS it's this or keeping eternally waiting at the side of the
swimmingpool. (= mpo-logism for keeping cforms 'unstable')
Comments, suggestions and help welcome.
(Guess, what I also don't have time for this)
-marc=
--
Marc Portier http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at http://blogs.cocoondev.org/mpo/
mpo@outerthought.org mpo@apache.org