On Sat, 2003-11-22 at 21:39, Timothy Larson wrote: > --- Bruno Dumon <[EMAIL PROTECTED]> wrote: > > On Fri, 2003-11-21 at 14:37, Timothy Larson wrote: > > I found the naming of the class/new widgets confusing. In fact, I think > > it is confusing to call them widgets at all, even if they might be > > implemented as widgets. Maybe calling them "define" and "use" is better, > > and I wouldn't document them as being widgets. > > I am fine with any changes that make it better/clearer. > You are right, Class and New are not widgets (they only have definitions > and builders after all). The names "Class" and "New" came from the idea > that at some point we might need to parameterize the instantiation of the > class of widgets. For example, we may want to give nested unions a > different default case than their parents. This could be specified as > parameters to the "New" element. BTW, the current names do not quite sit > well with me either. > > > I'm not a native english speaker, but I wonder if the struct/union names > > will say much to non-programmers, or even Java programmers. OTOH, I > > don't have any good alternatives yet. > > The name came from their counterparts in the "C" language. > More intuitive names would be greatly appreciated. > > > Anyway, these are just small polishing remarks, next step is to dive > > into the code. Could you also provide the complete patch, i.e. which > > includes changes to all the widgets, so that I can see better how the > > class/interface hierarchy fits together? > > When I modified all of the widgets the abstraction classes were not as > well developed as they are in the current patch. If the abstraction > is found beneficial, I will put the work into updating the old > (unsubmitted) patch. Here is the general idea however: > > (Roughly, if I remember right...) > Form: ContainerWidget, AbstractContainerWidget > FormDefinition: ContainerDefinition, AbstractContainerDefinition > Field: ValueWidget, AbstractValueWidget > FieldDefinition: ValueDefinition, AbstractValueDefinition > BooleanField: ValueWidget, custom code instead of AbstractValueWidget > BooleanField: ValueDefinitions, AbstractValueDefinitions > AggregateField: CompositeWidget, AbstractCompositeWidget > AggregateFieldDefinition: CompositeDefinition, AbstractCompositeDefinitions > Struct: ContainerWidget, AbstractContainerWidget > StructDefinitions: ContainerDefinitions, AbstractContainerDefinition > Union: CompositeWidget, AbstractCompositeWidget > UnionDefinitions: CompositeDefinition, AbstractCompositeDefinition > Repeater: ContainerWidget, custom code instead of AbstractContainerWidget > RepeaterRow: ContainerWidget, AbstractContainerWidget > RepeaterDefinition: ContainerDefinitions, AbstractContainerDefinition > ...etc.
I think it is useful, after all, the alternative is code duplication. > >From your other email: > > Need to look into the code yet, but I saw the comment in the TODO list > > about slow operation on deeper nesting levels, and was curious about > > what this is caused by? Something on the processRequest or template > > side? > > I did not get a chance to track this down yet. I just noticed it and > documented it to look into later. If you (or anyone) has a good > debugging/profiling environment you will probably beat me to it :) Don't have any either (damn that optimizeit thing is expensive), but suddenly the thought crossed my mind that this might be caused by the calls to getFullyQualifiedId(), and adding some little caching code there solved it indeed. Another thing I noticed is that in Union.readFromRequest, you first let the active case do the readFromRequest, and then call super.readFromRequest, which will let all cases do the readFromRequest. I don't think this is needed, and it solves the multiple row creation problem in the repeaters. (same applies to validate() I gues) Anyway, I'm starting to think that the easiest way to collaborate on this is if we just commit the code into CVS. Anyone got a problem with that? -- Bruno Dumon http://outerthought.org/ Outerthought - Open Source, Java & XML Competence Support Center [EMAIL PROTECTED] [EMAIL PROTECTED]
