<snip/>
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.
form.getWidget("choice").getWidget("union").getWidget("foo")
will become
((UnionWidget)((ChoiceWidget)form.getWidget("choice")).getWidget("union")).getWidget("foo")
Or we may extend getWidget() so that it accepts a path (dotted notation) instead of a simple name, which would allow e.g.
form.getWidget("choice.union.foo")
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
+1
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 <wi:styling>.
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
-- Sylvain Wallez Anyware Technologies http://www.apache.org/~sylvain http://www.anyware-tech.com { XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
