cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sylvain Wallez <>
Subject Re: [cforms] refactoring questions (was Re: cvs commit: cocoon-2.1/src/blocks/forms/java/org/apache/cocoon/forms/formmodel
Date Sun, 25 Apr 2004 13:10:41 GMT
Marc Portier wrote:


> Sorry for the massive commit, however when walking around the code it 
> only looked like the proverbial tip of the iceberg.

Sorry for the delay, but, as we say here "later is better than never"!

> >   - left quite some TODO markers for next sweaps
> Maybe some of you have some suggestions on some of them, feel free to 
> step in and comment:
> 1/ should getWidget(id) be removed from Widget? It is already on 
> ContainerWidget (which is the true context that makes sense IMHO)

+1 from a theoretical POV, but -1 from a practical one! This will lead 
to many casts to traverse a widget tree, e.g.
will become

Or we may extend getWidget() so that it accepts a path (dotted notation) 
instead of a simple name, which would allow e.g.

> 2/ should getNamespace() exist at all, it seems to return the same 
> thing as getFullQualifiedId()? Maybe a historical idea waiting to get 
> thrown out?

+1 to remove

> 3/ can getId() ever return null or "" on a widget instance? Can't we 
> carefully asume programming error and allow for the accidental NPE to 
> be thrown
> 4/ same question on getDefinition()

What's the need for getDefinition() for users of a widget? I consider 
this as an implementation concern of widgets and would remove it from 
the public API (i.e. make it protected in AbstractWidget).

> 5/ should we rename ContainerDelegate to simply WidgetList (and the 
> ContainerDefintionDelegate to WidgetDefinitionList)

WidgetList is more understandable than ContainerDelegate ;-)

> 6/ union seems to generate fi:field in stead of fi:union this 
> surprised me a bit, is that the goal?

+1 for fi:field as, for the view, it isn't different from a field and 
avoids duplicating the styling (the same is already done for action, 
submit, repeater-action and row-action)

> 7/ should validation stop as soon as possible or continue to allow all 
> validation errors to be set?

Continue to get all validation errors.

> 8/ setParent() on abstractWidget should be write-once IMHO, possibly 
> yielding RTE (IllegalState?) when someone tries to reset it

Don't know... A sure thing is that definitions don't need a parent (also 
they should be immutable). As for reparenting widgets, I don't know if 
there are some valid use cases...

> 9/ should not all generateSAXFragments include the 
> getDefinition.generateDisplayData() by default


And I would add:

10/ Split generateSAXFragment() into startSAXFragment() and 
endSAXFragment(), which will make it so much easier on the view side to 
handle custom SAX fragments for container widgets and embedding of the 

Note that I'd like also that <wi:styling> could be written in the 
definition also, as defining the styling in the widget definition can be 
a productivity boost with widget repositories!


Sylvain Wallez                                  Anyware Technologies 
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }

View raw message