cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marc Portier <...@outerthought.org>
Subject Re: [CForms] Repeater is not a ContainerWidget?
Date Sun, 02 May 2004 21:18:00 GMT


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

Mime
View raw message