You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Fredy Dobler <ap...@dobler.net> on 2004/04/30 16:40:35 UTC
[CForms] Repeater is not a ContainerWidget?
Hi all
I have the following problem:
If a child widget of a repeater row submits an on-change event,
I get a ClassCastExcpetion in the Class
org.apache.cocoon.forms.formmodel.Forms (Forms.java, Version 1.13,
cocoon-2.1.5-dev) in line 210.
line 210: submit = (ContainerWidget)submit).getWidget(stok.nextToken());
If the forms_submit_id is 'Repeater1.0.widget1', the submit variable
contains a repeater (the repeater with id 'Repeater1'). The Repeater class
is not a ContainerWidget -> a ClassCastExcpetion is thrown.
The repeater contains repeater rows. 'RepeaterRow' is a widget (extends
AbstractContainerWidget), does this not qualify the repeater itself as a
ContainerWidget?
Fredy
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Joerg Heinicke <jo...@gmx.de>.
On 02.05.2004 23:27, Marc Portier wrote:
>>>> 3/ make Repeater again a ContainerWidget (which it IMHO is not...
>>>
>>> but feels as the easiest way out of this.
>>>
>>> Maybe this case illustrates that Repeater should be a ContainerWidget
>>> after all? If it's not a ContainerWidget, how could it possibly contain
>>> widgets as children (which the RepeaterRows are)?
>>
>> Same feeling here: a repeater *contains* rows, which in turn *contain*
>> widgets.
>
> sure,
> just like both List and Map 'contain' values, yet they do so in a
> semantically distinct way, and there is little shared methods between them
>
>
> but just like you guys I can't ignore the use case from Fredy here, but
> that seems to be a reality that comes together with the existence of
> getFullyQualifiedId() and is thus better placed as a lookupWidget on the
> Widget interface (not on some container)
>
>>> Lets look at the interface (javadocs dropped):
>>>
>>> public interface ContainerWidget extends Widget {
>>> public void addWidget(Widget widget);
>>> public boolean hasWidget(String id);
>>> public Widget getWidget(String id);
>>> public Iterator getChildren();
>>> }
>>>
>>> Of these methods, the only one that is problematic is addWidget. But it
>>> seems like that method can be problematic for other ContainerWidgets as
>>> well, eg MultiValueField or RepeaterRow.
>>>
>>> I also see a problem with that method 'tout court', in that we have an
>>> addWidget but not a removeWidget?
>>
>> Mmmh... we need addWidget on some container widgets as part of their
>> setup by their widget builder. But it doesn't seem to me we need it at
>> all in the interface. It's a private contract between the buider and
>> the instances it creates.
>>
>
> yep, as mentioned there is probably only 'union' that would really need
> something like this during run-time execution (but I'ld need to check in
> detail)
>
>> Now about getWidget(): it is used to crawl down the widget tree, but
>> its name is inconsistent with getParent() which is used to crawl up
>> the tree. getChild() looks better to me. So what about having:
>> - getChild(id) searching in the _direct_ children of a widget.
>> - getWidget(path) performing a tree traversal using the dotted
>> notation (e.g. "contacts.1.name").
>>
>> These two methods would be available on ContainerWidget only
>> (including repeater).
>>
>> WDYT?
>>
>
> this sounds similar to my distinction between getWidget and lookupWidget
> but I think the latter could also be defined on the level of Widget
> directly.
>
> that would make 'navigating' the widget tree a feature of every widget
> and as mentioned before more logically come together with the
> getFullyQualifiedId (that would be equal to some getPath() in fact?)
Not much time to think through all this, so I will only add my comment:
I like the getWidget/lookupWidget much more than getWidget/getChildren.
The semantical difference of the latter one is not that obvious, the
same is true IMO for the path. Accessing descendant widgets by a yet
another path expression is somewhat unexpected. Real XPath would be an
alternative, but still not what I would like to see.
Joerg
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Fredy Dobler <fr...@dobler.net>.
Marc Portier wrote:
>
> Sylvain Wallez wrote:
>
>> Bruno Dumon wrote:
>>> On Fri, 2004-04-30 at 23:04, Marc Portier wrote:
>> <snip/>
>>>> 3/ make Repeater again a ContainerWidget (which it IMHO is not...
>>> but feels as the easiest way out of this.
>>> Maybe this case illustrates that Repeater should be a ContainerWidget
after all? If it's not a ContainerWidget, how could it possibly
contain
>>> widgets as children (which the RepeaterRows are)?
>> Same feeling here: a repeater *contains* rows, which in turn *contain*
widgets.
>
> sure,
> just like both List and Map 'contain' values, yet they do so in a
semantically distinct way, and there is little shared methods between
them
>
>
> but just like you guys I can't ignore the use case from Fredy here, but
that seems to be a reality that comes together with the existence of
getFullyQualifiedId() and is thus better placed as a lookupWidget on the
Widget interface (not on some container)
>
I think the semantic diffrence between a List and a Map are a lot more
distinct than the diffrence between the Repeater and the ContainerWidget
interface.
The only diffrence in the semantically way I see, is that the Repeater
only accepts one kind of Widget, the RepeaterRow. I think in all other
aspects the Repeater is semantically the same as a ContainerWidget.
>>> Lets look at the interface (javadocs dropped):
>>> public interface ContainerWidget extends Widget {
>>> public void addWidget(Widget widget);
>>> public boolean hasWidget(String id);
>>> public Widget getWidget(String id);
>>> public Iterator getChildren();
>>> }
>>> Of these methods, the only one that is problematic is addWidget. But
it
>>> seems like that method can be problematic for other ContainerWidgets
as
>>> well, eg MultiValueField or RepeaterRow.
>>> I also see a problem with that method 'tout court', in that we have an
addWidget but not a removeWidget?
>> Mmmh... we need addWidget on some container widgets as part of their
setup by their widget builder. But it doesn't seem to me we need it at
all in the interface. It's a private contract between the buider and
the
>> instances it creates.
>
> yep, as mentioned there is probably only 'union' that would really need
something like this during run-time execution (but I'ld need to check in
detail)
>
>> Now about getWidget(): it is used to crawl down the widget tree, but
its
>> name is inconsistent with getParent() which is used to crawl up the
tree. getChild() looks better to me. So what about having:
>> - getChild(id) searching in the _direct_ children of a widget.
>> - getWidget(path) performing a tree traversal using the dotted notation
(e.g. "contacts.1.name").
>> These two methods would be available on ContainerWidget only (including
repeater).
>> WDYT?
>
> this sounds similar to my distinction between getWidget and lookupWidget
but I think the latter could also be defined on the level of Widget
directly.
>
> that would make 'navigating' the widget tree a feature of every widget
and as mentioned before more logically come together with the
> getFullyQualifiedId (that would be equal to some getPath() in fact?)
>
A lookup function on the Widget level sounds good to me.
Fredy
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Marc Portier <mp...@outerthought.org>.
Sylvain Wallez wrote:
> Marc Portier wrote:
>
>> Sylvain Wallez wrote:
>
> <snip/>
>
>>> Same feeling here: a repeater *contains* rows, which in turn
>>> *contain* widgets.
>>
>> sure,
>> just like both List and Map 'contain' values, yet they do so in a
>> semantically distinct way, and there is little shared methods between
>> them
>
> IMO, it's more like Collection and List : a Collection lets you iterate
> through its content, whereas a List extends this behaviour by giving you
> positional access to its content. That's what repeater.getRow() gives us.
>
> There *must* be a unified interface for all widgets that aren't terminal
> (i.e. that contain other widgets). This is key to be able to write
> generic walkers, the simplest one being lookupWidget().
>
walkers would have an equally complex job either checking a return for
null or else checking an instanceof and performing typecast
in fact, it just came to me that this discussion is just a general
TO_DECIDE_YOURSELF part of the classic 'composite' pattern (see quote),
there doesn't seem to be a general answer/guideline on this, it's just
up to us to find a best fit
<qoute from="the book (section: composite/implementation/nr.3)">
Sometimes a little creativity shows how an operation that would appear
to make sense only for Composites can be implemented for all Components
by moving it to the Component class. For example, the interface for
accessing children is a fundamental part of a Composite class but not
necessarily Leaf classes. But if we view a Leaf as a Component that
never has children, then we can define a default operation for child
access in the Component class that never returns any children. Leaf
classes can use the default implementation, but Composite classes will
reimplement it to return their children.
</qoute>
<snip />
>> this sounds similar to my distinction between getWidget and
>> lookupWidget but I think the latter could also be defined on the level
>> of Widget directly.
>>
>> that would make 'navigating' the widget tree a feature of every widget
>> and as mentioned before more logically come together with the
>> getFullyQualifiedId (that would be equal to some getPath() in fact?)
>
I still find this somewhat an argument, but...
>
> Reading also Jörg's opinion, my preferred navigation API is :
> - getChild(String name) to get a direct child of a widget (replaces
> getWidget which isn't precise enough to say if it considers children
> only or also descendants)
> - lookupWidget(String qname) which does a descendant lookup only, based
> on child names sperarated by dots.
>
am quite ok with this as well...
I don't find my own 'vibes' amounting to a conclusive argumentation to
date, I was just hoping others would find more elements and was just
looking for some thoughts I'ld maybe missed...
sorry if this sounded as wasting time...
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] Repeater is not a ContainerWidget?
Posted by Marc Portier <mp...@outerthought.org>.
Bruno Dumon wrote:
> On Mon, 2004-05-03 at 18:43, Sylvain Wallez wrote:
> <snip/>
>
>>Reading also Jörg's opinion, my preferred navigation API is :
>>- getChild(String name) to get a direct child of a widget (replaces
>>getWidget which isn't precise enough to say if it considers children
>>only or also descendants)
>>- lookupWidget(String qname) which does a descendant lookup only, based
>>on child names sperarated by dots.
>
>
> Sorry for revisiting this yet another time, but I wanted to throw in
> another thought about the lookupWidget.
>
> How about separating the child names with slashes instead of dots?
>
> The reason: this would provide an obvious syntax for moving up in the
> tree: e.g. "../widget", and "/widget" for absolute navigation.
>
> This would, among other things, be useful in the expressions. If we
> change the variable resolving there to use lookupWidget instead of
> getChild, this would allow expressions to refer to any other widget in
> the tree.
>
> I don't think it is important to match the dotted notation we have in
> the request parameter names, as I see that as a completely unrelated
> thing. In that regard, getFullyQualifiedId() should maybe be renamed to
> getRequestParameterName().
>
+1
I'm giving this a go
IMO before codefreeze we need to have (at least) an answer to the
problem caused by removing the getWidget from repeater
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] Repeater is not a ContainerWidget?
Posted by Bruno Dumon <br...@outerthought.org>.
On Mon, 2004-05-03 at 18:43, Sylvain Wallez wrote:
<snip/>
> Reading also Jörg's opinion, my preferred navigation API is :
> - getChild(String name) to get a direct child of a widget (replaces
> getWidget which isn't precise enough to say if it considers children
> only or also descendants)
> - lookupWidget(String qname) which does a descendant lookup only, based
> on child names sperarated by dots.
Sorry for revisiting this yet another time, but I wanted to throw in
another thought about the lookupWidget.
How about separating the child names with slashes instead of dots?
The reason: this would provide an obvious syntax for moving up in the
tree: e.g. "../widget", and "/widget" for absolute navigation.
This would, among other things, be useful in the expressions. If we
change the variable resolving there to use lookupWidget instead of
getChild, this would allow expressions to refer to any other widget in
the tree.
I don't think it is important to match the dotted notation we have in
the request parameter names, as I see that as a completely unrelated
thing. In that regard, getFullyQualifiedId() should maybe be renamed to
getRequestParameterName().
--
Bruno Dumon http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
bruno@outerthought.org bruno@apache.org
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Sylvain Wallez <sy...@apache.org>.
Marc Portier wrote:
> Sylvain Wallez wrote:
<snip/>
>> Same feeling here: a repeater *contains* rows, which in turn
>> *contain* widgets.
>
>
> sure,
> just like both List and Map 'contain' values, yet they do so in a
> semantically distinct way, and there is little shared methods between them
IMO, it's more like Collection and List : a Collection lets you iterate
through its content, whereas a List extends this behaviour by giving you
positional access to its content. That's what repeater.getRow() gives us.
There *must* be a unified interface for all widgets that aren't terminal
(i.e. that contain other widgets). This is key to be able to write
generic walkers, the simplest one being lookupWidget().
> but just like you guys I can't ignore the use case from Fredy here,
> but that seems to be a reality that comes together with the existence
> of getFullyQualifiedId() and is thus better placed as a lookupWidget
> on the Widget interface (not on some container)
lookupWidget() only makes sense on ContainerWidget (if Repeater
implements it also, of course), since terminal widgets have no children.
<snip/>
> this sounds similar to my distinction between getWidget and
> lookupWidget but I think the latter could also be defined on the level
> of Widget directly.
>
> that would make 'navigating' the widget tree a feature of every widget
> and as mentioned before more logically come together with the
> getFullyQualifiedId (that would be equal to some getPath() in fact?)
Reading also Jörg's opinion, my preferred navigation API is :
- getChild(String name) to get a direct child of a widget (replaces
getWidget which isn't precise enough to say if it considers children
only or also descendants)
- lookupWidget(String qname) which does a descendant lookup only, based
on child names sperarated by dots.
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Marc Portier <mp...@outerthought.org>.
Sylvain Wallez wrote:
> Bruno Dumon wrote:
>
>> On Fri, 2004-04-30 at 23:04, Marc Portier wrote:
>>
>>
>
> <snip/>
>
>>> 3/ make Repeater again a ContainerWidget (which it IMHO is not...
>>>
>>
>>
>> but feels as the easiest way out of this.
>>
>> Maybe this case illustrates that Repeater should be a ContainerWidget
>> after all? If it's not a ContainerWidget, how could it possibly contain
>> widgets as children (which the RepeaterRows are)?
>>
>>
>
> Same feeling here: a repeater *contains* rows, which in turn *contain*
> widgets.
>
sure,
just like both List and Map 'contain' values, yet they do so in a
semantically distinct way, and there is little shared methods between them
but just like you guys I can't ignore the use case from Fredy here, but
that seems to be a reality that comes together with the existence of
getFullyQualifiedId() and is thus better placed as a lookupWidget on the
Widget interface (not on some container)
>> Lets look at the interface (javadocs dropped):
>>
>> public interface ContainerWidget extends Widget {
>> public void addWidget(Widget widget);
>> public boolean hasWidget(String id);
>> public Widget getWidget(String id);
>> public Iterator getChildren();
>> }
>>
>> Of these methods, the only one that is problematic is addWidget. But it
>> seems like that method can be problematic for other ContainerWidgets as
>> well, eg MultiValueField or RepeaterRow.
>>
>> I also see a problem with that method 'tout court', in that we have an
>> addWidget but not a removeWidget?
>>
>>
>
> Mmmh... we need addWidget on some container widgets as part of their
> setup by their widget builder. But it doesn't seem to me we need it at
> all in the interface. It's a private contract between the buider and the
> instances it creates.
>
yep, as mentioned there is probably only 'union' that would really need
something like this during run-time execution (but I'ld need to check in
detail)
> Now about getWidget(): it is used to crawl down the widget tree, but its
> name is inconsistent with getParent() which is used to crawl up the
> tree. getChild() looks better to me. So what about having:
> - getChild(id) searching in the _direct_ children of a widget.
> - getWidget(path) performing a tree traversal using the dotted notation
> (e.g. "contacts.1.name").
>
> These two methods would be available on ContainerWidget only (including
> repeater).
>
> WDYT?
>
this sounds similar to my distinction between getWidget and lookupWidget
but I think the latter could also be defined on the level of Widget
directly.
that would make 'navigating' the widget tree a feature of every widget
and as mentioned before more logically come together with the
getFullyQualifiedId (that would be equal to some getPath() in fact?)
-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] Repeater is not a ContainerWidget?
Posted by Sylvain Wallez <sy...@apache.org>.
Bruno Dumon wrote:
>On Fri, 2004-04-30 at 23:04, Marc Portier wrote:
>
>
<snip/>
>>3/ make Repeater again a ContainerWidget (which it IMHO is not...
>>
>>
>
>but feels as the easiest way out of this.
>
>Maybe this case illustrates that Repeater should be a ContainerWidget
>after all? If it's not a ContainerWidget, how could it possibly contain
>widgets as children (which the RepeaterRows are)?
>
>
Same feeling here: a repeater *contains* rows, which in turn *contain*
widgets.
>Lets look at the interface (javadocs dropped):
>
>public interface ContainerWidget extends Widget {
> public void addWidget(Widget widget);
> public boolean hasWidget(String id);
> public Widget getWidget(String id);
> public Iterator getChildren();
>}
>
>Of these methods, the only one that is problematic is addWidget. But it
>seems like that method can be problematic for other ContainerWidgets as
>well, eg MultiValueField or RepeaterRow.
>
>I also see a problem with that method 'tout court', in that we have an
>addWidget but not a removeWidget?
>
>
Mmmh... we need addWidget on some container widgets as part of their
setup by their widget builder. But it doesn't seem to me we need it at
all in the interface. It's a private contract between the buider and the
instances it creates.
Now about getWidget(): it is used to crawl down the widget tree, but its
name is inconsistent with getParent() which is used to crawl up the
tree. getChild() looks better to me. So what about having:
- getChild(id) searching in the _direct_ children of a widget.
- getWidget(path) performing a tree traversal using the dotted notation
(e.g. "contacts.1.name").
These two methods would be available on ContainerWidget only (including
repeater).
WDYT?
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Joerg Heinicke <jo...@gmx.de>.
On 03.05.2004 23:40, Sylvain Wallez wrote:
> If we aren't able to determine if widgets that do have children are
> containers or non-containers, let's put back this getWidget method (or
> getChild, for what my opinion is worth) on the Widget interface and go
> to the next item in the todo list.
+1 for the pragmatic way if there is no way to reach consensus.
Joerg
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Sylvain Wallez <sy...@apache.org>.
Tim Larson wrote:
>We can solve this container issue by pulling in a couple ideas:
> ReiserFS: Every file can also be a directory.
> Gnu-Hurd: Hierarchial filesystem translators.
>
>
<snip/>
>Widgets would reside in a WidgetSystem and navigation would be
>performed by negotiating with the WidgetSystem, which may call
>back to the widgets for rights, special semantics, views, etc.
>For example, the WidgetSystem could be made smart to understand
>that some widget collections are ordered and for these support
>positional inserts.
>
>
These are very interesting ideas...
>The main problem seems to be how to write java ioctl calls to
>the widgets...
>
>
... but which don't seem to help much :-/
C'mon guys, this repeater/container story starts to become crazy.
If we aren't able to determine if widgets that do have children are
containers or non-containers, let's put back this getWidget method (or
getChild, for what my opinion is worth) on the Widget interface and go
to the next item in the todo list.
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Tim Larson <ti...@keow.org>.
We can solve this container issue by pulling in a couple ideas:
ReiserFS: Every file can also be a directory.
Gnu-Hurd: Hierarchial filesystem translators.
Translated this is:
(1) Every widget can also be a container.
(2) We navigate a WidgetSystem instead of navigating widgets,
which calls back to the widgets to allow them to add their
own semantics and present their own views (database-like
query-views, not template-views) of their "children".
(1)
Why would we want any widget to also be able to be a container?
Well, what does containment imply right now?
Aggregate:
Records logical relationships.
Adds split/merge semantics.
Struct:
Disambiguates parameter names with another "<name>."
Repeater:
Maintains relative positioning/sort order
Enforces naming (e.g. repeater row names must be #'s)
Union:
Quarantines dynamically created and destroyed widgets.
And what do we gain by extending this to all widgets?
Control of access, permissions, and access logging.
Use of child widgets as attributes of the containing widget.
(2) What would indirect access via a WidgetSystem win us?
Generic ways to enforce:
Access rights/permissions
Logging of various types of accesses.
Read/write locking.
Control over access to widgets' type-specific ioctl's ;)
Widgets would reside in a WidgetSystem and navigation would be
performed by negotiating with the WidgetSystem, which may call
back to the widgets for rights, special semantics, views, etc.
For example, the WidgetSystem could be made smart to understand
that some widget collections are ordered and for these support
positional inserts.
The main problem seems to be how to write java ioctl calls to
the widgets...
--Tim Larson
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Marc Portier <mp...@outerthought.org>.
Bruno Dumon wrote:
> On Fri, 2004-04-30 at 23:04, Marc Portier wrote:
>
>>Fredy Dobler wrote:
>>
>>
>>>Hi all
>>>
>>>I have the following problem:
>>>If a child widget of a repeater row submits an on-change event,
>>>I get a ClassCastExcpetion in the Class
>>>org.apache.cocoon.forms.formmodel.Forms (Forms.java, Version 1.13,
>>>cocoon-2.1.5-dev) in line 210.
>>>
>>>line 210: submit = (ContainerWidget)submit).getWidget(stok.nextToken());
>>>
>>>If the forms_submit_id is 'Repeater1.0.widget1', the submit variable
>>>contains a repeater (the repeater with id 'Repeater1'). The Repeater class
>>>is not a ContainerWidget -> a ClassCastExcpetion is thrown.
>>>
>>>The repeater contains repeater rows. 'RepeaterRow' is a widget (extends
>>>AbstractContainerWidget), does this not qualify the repeater itself as a
>>>ContainerWidget?
>>>
>>
>>yo Fredy, I'm the culprit...
>>
>>when in clean-up mode visiting the code I found most methods of
>>ContainerWidget not applicable to RepeaterWidget (all except getWidget)
>>in fact most (other) implementations were doing nothing or threw RTE's
>>indicating they shouldn't be called...
>>
>>I also found it consistent with a growing conceptual feeling that the
>>repeater is not really 'composed' of different subwidgets (as e.g.
>>struct, aggregate and the inner class Repeater.Repeater.Row) but had
>>more in common with MultiValueField in the sense that it 'repeated'
>>multiple times the same value
>>
>>see, it has a getRow(int index) which is the more logical counterpart on
>>that level
>>
>>in any case, if I understand your use case you have a repeater holding
>>action buttons and it's quit obvious we should support this.
>>
>>3 ways out:
>>
>>1/ re-add getWidget(id) on the Widget interface and let Repeater NOT be
>>ContainerWidget (I removed the method from that interface in the same
>>clean-up sweep)
>>
>>this would also add it back on the scriptable interface allowing every
>>widget to be navigatable to children in javascript
>>
>>while at it I would in fact rather make it into some lookupWidget()
>>method and implement it on AbstractContainerWidget and Repeater to allow
>>immediately nested id's in there: so lookupWidget("contacts.0.whatever")
>>would be the use which I remember being suggested in the past
>>
>>that would also keep the distinction with ContainerWidget.getWidget(id)
>>which is what I still feel to be sematically a different thing?
>>
>>should pure singular widgets override to throw NPE or return NULL?
>>or rather:
>>1bis/ add lookupWidget to a separate WidgetResolver interface that would
>>then be added to (Abstract)ContainerWidget and Repeater (not to all Widgets)
>>
>>
>>2/ make the mentioned code snippet inside Form a bit smarter so it
>>recognises the Repeater and does a getRow instead?
>>
>>
>>3/ make Repeater again a ContainerWidget (which it IMHO is not...
>
>
> but feels as the easiest way out of this.
>
hm,
actually the difference between all 3 is really a matter of semantics,
(e.g. replace WidgetResolver idea from 1bis by ContainerWidget and
'tzadaam' they're almost a perfect match)
so I'm quite +1 to just take the easiest route out of this, but that
doesn't change the fact that in my head stuff is still fighting to get
the correct names and labels... (that's why I suggested the split
between getWidget and lookupWidget)
IMHO the repeater-repeaterrow relation bears different semantics then
the repeaterrow-subfields or the aggregate-part, even they all resort to
a similar technique of request-parameter namespacing
but let's be practical: if there is no observabale difference between
them, we can really ignore my bad vibes here
> Maybe this case illustrates that Repeater should be a ContainerWidget
> after all? If it's not a ContainerWidget, how could it possibly contain
> widgets as children (which the RepeaterRows are)?
>
I agree, but maybe we really should reconsider some lookupWidget() on
the level of Widget
I know the swing has been in this position before:
Jul 3 2003: "Dropped ContainerWidget interface and moved getWidget
method to Widget interface."
http://cvs.apache.org/viewcvs.cgi/cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/formmodel/ContainerWidget.java?rev=1.2&view=markup
Building up a tree of parent-child related widgets is what we do, no?
Now trying to explicitely make the distinction between nodes and leafs
in that tree is colliding IMHO with the various semantical relations or
reasons for possible containment. Each of those reasons seem to suggest
their proper methods associated with it, and adding those to the 'one
size fits all' interface then leads to the others doing 'bogus'
implementations of them.
> Lets look at the interface (javadocs dropped):
>
> public interface ContainerWidget extends Widget {
> public void addWidget(Widget widget);
> public boolean hasWidget(String id);
> public Widget getWidget(String id);
> public Iterator getChildren();
> }
>
> Of these methods, the only one that is problematic is addWidget. But it
yeah you made me verify my own statement :)
- addWidget was throwing RTE
- but the one that was really bogus implemented was getChildren() :-)
(see
http://cvs.apache.org/viewcvs.cgi/cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/formmodel/Repeater.java?r1=1.15&r2=1.16
eclipse generated by Carsten to just have stuff compile again)
> seems like that method can be problematic for other ContainerWidgets as
> well, eg MultiValueField or RepeaterRow.
>
(small slip here: multivalue is not a containerwidget)
> I also see a problem with that method 'tout court', in that we have an
> addWidget but not a removeWidget?
>
add- seems to suggest that we would need to be able to 'add' stuff at
runtime, the only widget actually needing that IMHO is 'union' (aka
choice) (the others could do it by setting the array/list of children at
construction time)
this makes me realize that ContainerWidget didn't get (re)introduced
until the introduction of class, new, struct and union...
http://cvs.apache.org/viewcvs.cgi/cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/formmodel/Repeater.java?r1=1.14&r2=1.15&diff_format=h
since those are reconsidered largely anyway, I don't see why we would
need to stick to some historically introduced and lingering mix of
semantics.
wdyt?
-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] Repeater is not a ContainerWidget?
Posted by Bruno Dumon <br...@outerthought.org>.
On Fri, 2004-04-30 at 23:04, Marc Portier wrote:
> Fredy Dobler wrote:
>
> > Hi all
> >
> > I have the following problem:
> > If a child widget of a repeater row submits an on-change event,
> > I get a ClassCastExcpetion in the Class
> > org.apache.cocoon.forms.formmodel.Forms (Forms.java, Version 1.13,
> > cocoon-2.1.5-dev) in line 210.
> >
> > line 210: submit = (ContainerWidget)submit).getWidget(stok.nextToken());
> >
> > If the forms_submit_id is 'Repeater1.0.widget1', the submit variable
> > contains a repeater (the repeater with id 'Repeater1'). The Repeater class
> > is not a ContainerWidget -> a ClassCastExcpetion is thrown.
> >
> > The repeater contains repeater rows. 'RepeaterRow' is a widget (extends
> > AbstractContainerWidget), does this not qualify the repeater itself as a
> > ContainerWidget?
> >
>
> yo Fredy, I'm the culprit...
>
> when in clean-up mode visiting the code I found most methods of
> ContainerWidget not applicable to RepeaterWidget (all except getWidget)
> in fact most (other) implementations were doing nothing or threw RTE's
> indicating they shouldn't be called...
>
> I also found it consistent with a growing conceptual feeling that the
> repeater is not really 'composed' of different subwidgets (as e.g.
> struct, aggregate and the inner class Repeater.Repeater.Row) but had
> more in common with MultiValueField in the sense that it 'repeated'
> multiple times the same value
>
> see, it has a getRow(int index) which is the more logical counterpart on
> that level
>
> in any case, if I understand your use case you have a repeater holding
> action buttons and it's quit obvious we should support this.
>
> 3 ways out:
>
> 1/ re-add getWidget(id) on the Widget interface and let Repeater NOT be
> ContainerWidget (I removed the method from that interface in the same
> clean-up sweep)
>
> this would also add it back on the scriptable interface allowing every
> widget to be navigatable to children in javascript
>
> while at it I would in fact rather make it into some lookupWidget()
> method and implement it on AbstractContainerWidget and Repeater to allow
> immediately nested id's in there: so lookupWidget("contacts.0.whatever")
> would be the use which I remember being suggested in the past
>
> that would also keep the distinction with ContainerWidget.getWidget(id)
> which is what I still feel to be sematically a different thing?
>
> should pure singular widgets override to throw NPE or return NULL?
> or rather:
> 1bis/ add lookupWidget to a separate WidgetResolver interface that would
> then be added to (Abstract)ContainerWidget and Repeater (not to all Widgets)
>
>
> 2/ make the mentioned code snippet inside Form a bit smarter so it
> recognises the Repeater and does a getRow instead?
>
>
> 3/ make Repeater again a ContainerWidget (which it IMHO is not...
but feels as the easiest way out of this.
Maybe this case illustrates that Repeater should be a ContainerWidget
after all? If it's not a ContainerWidget, how could it possibly contain
widgets as children (which the RepeaterRows are)?
Lets look at the interface (javadocs dropped):
public interface ContainerWidget extends Widget {
public void addWidget(Widget widget);
public boolean hasWidget(String id);
public Widget getWidget(String id);
public Iterator getChildren();
}
Of these methods, the only one that is problematic is addWidget. But it
seems like that method can be problematic for other ContainerWidgets as
well, eg MultiValueField or RepeaterRow.
I also see a problem with that method 'tout court', in that we have an
addWidget but not a removeWidget?
--
Bruno Dumon http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
bruno@outerthought.org bruno@apache.org
Re: [CForms] Repeater is not a ContainerWidget?
Posted by Marc Portier <mp...@outerthought.org>.
Fredy Dobler wrote:
> Hi all
>
> I have the following problem:
> If a child widget of a repeater row submits an on-change event,
> I get a ClassCastExcpetion in the Class
> org.apache.cocoon.forms.formmodel.Forms (Forms.java, Version 1.13,
> cocoon-2.1.5-dev) in line 210.
>
> line 210: submit = (ContainerWidget)submit).getWidget(stok.nextToken());
>
> If the forms_submit_id is 'Repeater1.0.widget1', the submit variable
> contains a repeater (the repeater with id 'Repeater1'). The Repeater class
> is not a ContainerWidget -> a ClassCastExcpetion is thrown.
>
> The repeater contains repeater rows. 'RepeaterRow' is a widget (extends
> AbstractContainerWidget), does this not qualify the repeater itself as a
> ContainerWidget?
>
yo Fredy, I'm the culprit...
when in clean-up mode visiting the code I found most methods of
ContainerWidget not applicable to RepeaterWidget (all except getWidget)
in fact most (other) implementations were doing nothing or threw RTE's
indicating they shouldn't be called...
I also found it consistent with a growing conceptual feeling that the
repeater is not really 'composed' of different subwidgets (as e.g.
struct, aggregate and the inner class Repeater.Repeater.Row) but had
more in common with MultiValueField in the sense that it 'repeated'
multiple times the same value
see, it has a getRow(int index) which is the more logical counterpart on
that level
in any case, if I understand your use case you have a repeater holding
action buttons and it's quit obvious we should support this.
3 ways out:
1/ re-add getWidget(id) on the Widget interface and let Repeater NOT be
ContainerWidget (I removed the method from that interface in the same
clean-up sweep)
this would also add it back on the scriptable interface allowing every
widget to be navigatable to children in javascript
while at it I would in fact rather make it into some lookupWidget()
method and implement it on AbstractContainerWidget and Repeater to allow
immediately nested id's in there: so lookupWidget("contacts.0.whatever")
would be the use which I remember being suggested in the past
that would also keep the distinction with ContainerWidget.getWidget(id)
which is what I still feel to be sematically a different thing?
should pure singular widgets override to throw NPE or return NULL?
or rather:
1bis/ add lookupWidget to a separate WidgetResolver interface that would
then be added to (Abstract)ContainerWidget and Repeater (not to all Widgets)
2/ make the mentioned code snippet inside Form a bit smarter so it
recognises the Repeater and does a getRow instead?
3/ make Repeater again a ContainerWidget (which it IMHO is not)
euh, I probably look a bit biassed towards option 1(bis) :-)
but feel free to comment/make other suggestions?
-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