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 Binding Revisited.
Date Tue, 30 Mar 2004 07:13:17 GMT


Joerg Heinicke wrote:

> On 30.03.2004 00:24, Marc Portier wrote:
> 
>> Towards the larger user-base I think we should unify views, clean out 
>> the code and provide appropriate documentation.
>>
>> So, here is an attempt to bring the sheep back into the heard, and lay 
>> down a better foundation for the RepeaterBinding implementation as 
>> well as its use (and thus its documentation)
> 
> 
> +1
> 
>> looking into the <fb:repeater> syntax we see these mappings back in 
>> the names of the attributes:
>>
>> @id points to the repeater
>> @parent-path points to the matching contextbean
>> the repeater's rows each map to one item found at @row-path within the 
>> contextbean
>> the nested child-widgets will map @id of widget to @path of actual 
>> property.
> 
> 
> I found the (at that time) wb:repeater syntax really confusing when 
> using it the first time. There were 5 attributes and there usage was not 
> obvious to me: @id, @parent-path, @row-path, @unique-id, 
> @unique-row-path. That was why I liked Antonio's move of the last two 
> attrs into elements so much.
> 
> What's still confusing IMO is the name parent-path - the path of which 
> parent? For consistency this should also be named @path as for all other 
> binding elements, the name @row-path avoids any further confusion I think.
> 

thx for chiming in with my name change proposal upfront :-)

>> [case 1]
>> DESCRIPTION:
>> - the form handling (actions or events) will not reorder the rows
>> - no rows will be added or deleted
>> - item-properties could be sparsly bound to row-widgets (meaning: not 
>> all properties are mapped to actual widgets since they're not part of 
>> the foreseen end-user interaction scheme, of course the ones that were 
>> not bound should not be cleared on save just because the form-model 
>> isn't holding a value for them)
>>
>> In this case we don't really speak of identity of the items, and we 
>> don't really need it either: the position of each item in the list 
>> (it's index) serves as an implicit identifier, and since no 
>> adding/deleting or reordering on the rows can happen we know that the 
>> row-indexes will just keep in sync.
>>
>> PROPOSED SYNTAX:
>> <fb:repeater id="rep-id" parent-path="."
>>               row-path="item">
>>   <!-- no fb:identity -->
>>   <!--required elements for this case -->
>>   <fb:on-bind>...</fb:on-bind>
>>   <!--optional elements for this case -->
>>   <fb:on-insert>..</fb:on-insert>
>>   <fb:on-delete>..</fb:on-delete>
>> </fb:repeater>
>>
>> EXPECTATION DETECTION:
>> - no nested <fb:identity> element tells us we don't have an explicit 
>> identity, and we should just trust the index as a identity correlator.
>>
>> STRATEGY on 'save':
>> - foreach row in repeater
>>   - use the nested 'on-bind' binding
>>
>> A straightforward extension seems to be
>> - allow for add/delete of rows (by adding <on-insert> <on-delete>)
>> - at the cost of not surviving sparse binding of items.
>>
>> Then the Strategy becomes:
>> - foreach row in repeater with matching index in the items
>>   - use the nested 'on-bind' binding
>> - for excess rows in the repeater
>>   - use on-insert and on-bind
>> - for excess items in the context
>>   - use on-delete
>>
>>                           -o0o-
>>
>>
>> [case 2] (aka current SimpleRepeaterBinding)
>> DESCRIPTION:
>> - no fluff, just stuff
>> - the simplicity of 'load' ported over to 'save'
>>
>> - we can just remove all the items before binding them from the form
>>   (thus no expected support for sparse binding)
>> - form-model can be changed at will
>>
>> PROPOSED SYNTAX:
>> <fb:repeater id="rep-id" parent-path="."
>>              row-path="item"
>>              clear-items-on-save="true">
>>   <!-- no fb:identity -->
>>   <!--required elements for this case -->
>>   <fb:on-bind>...</fb:on-bind>
>>   <fb:on-insert>..</fb:on-insert>
>>   <fb:on-delete>..</fb:on-delete>
>> </fb:repeater>
>>
>> EXPECTATION DETECTION: @clear-items-on-save="true"
>>
>> STRATEGY on 'save':
>> - remove all items using 'on-delete'
>> - do as extended-case-1
>>
>>
>> REMARK:
>> This strategy obviously needs to delete and re-create all items. Thus 
>> it requires the presence of <fb:insert> and <fb:delete> bindings to 
>> operate.
>> However, taking into account the current operation, and some 
>> typing-economics for the most logical/frequent use we should probably 
>> agree on some default behaviour for those and request explicit removal 
>> of the insert and delete bindings by adding the empty configuration 
>> element.
> 
> 
> Maybe it's just to late for thinking, but what's the difference between 
> 1 (especially with "straightforward extension") and 2?
> 

none!
In fact I hope to be wrapping all these up in one imlementation 
algorithm: if we add the current indexes (of row and item) as arguments 
to the isIdentical() method in the IdentityBinding, then we could have a 
default identity-matching thingy implementing the interface in those 
cases where there was no fb:identity...

but that will become clear as I start coding
the counter-argument would be that detecting the strategy at build time 
and then installing the most appropriate algorithm probably will lead to 
simpler code, easier extensibility at the cost of some more classes...

>> [case 3]
>> DESCRIPTION:
>> - items have an explicit identity and can be sparsly bound
>> - form-model can add/remove/reorder the rows
>> - the sequence editing of the rows need to be reflected onto the items
>>
>> PROPOSED SYNTAX:
>> <fb:repeater id="rep-id" parent-path="."
>>              row-path="item" >
>>   <!--required elements for this case -->
>>   <fb:identity>...</fb:identity>
>>   <fb:on-bind>...</fb:on-bind>
>>   <fb:on-insert>..</fb:on-insert>
>>   <fb:on-delete>..</fb:on-delete>
>> </fb:repeater>
>>
>> EXPECTATION DETECTION: <fb:identity> is present
>>
>> STRATEGY on 'save':
>> - backup all items to a list OriginalItems
>> - foreach row in the repeater
>>   - if row-identity-match in OriginalItems
>>     - move item back into the context and bind
>>   - else
>>     - use on-insert and on-bind to create and bind
>> - for the items still left in the OriginalItems
>>   - use the on-delete
>>
>>

in fact, this approach makes a question pop up to the hibernate and ojb 
experts out there:

will this moving to buffer collection just work or am I to consider some 
constraints while doing so?

>>                           -o0o-
>>
>> [case 4] (aka current RepeaterBinding)
>> DESCRIPTION:
>> - items have an explicit identity and can be sparsly bound
>> - form-model can add/remove but should not allow reordering of the rows
>> - the original sequence of the items is to be maintained at all times 
>> (on-insert really is more of an on-append: no new ones can be inserted 
>> in between the old ones)
>>
>> PROPOSED SYNTAX:
>> not clear yet, I tend towards:
>>
>> <fb:repeater id="rep-id" parent-path="."
>>              row-path="item"
>>              row-path-insert="newItem">
>>   <!--required elements for this case -->
>>   <fb:identity>...</fb:identity>
>>   <fb:on-bind>...</fb:on-bind>
>>   <fb:on-insert>..</fb:on-insert>
>>   <fb:on-delete>..</fb:on-delete>
>> </fb:repeater>
>>
>>
>> EXPECTATION DETECTION:
>> fb:identity plus
>> the presense of a @row-path-insert seems to indicate that the new ones 
>> need to go into a different space then the exisiting ones, and thus 
>> denying the approach expressed in case3
>> (i.e. even if it would read the same as row-path! so there would be a 
>> difference between explicitely stating it as the same, and defaultly 
>> assuming it is the same)
>>
>>
>> STRATEGY on 'save':
>> - foreach row in repeater
>>   - if identity match found in items
>>     - bind to that
>>     - add it to the set of updatedItems
>>   - else
>>     - add it to some list of rowsToInsert
>> - run through items
>>   - if NOT found in updatedItems
>>     - add to list of toDeleteItems (ndx will do)
>> - register the on-insert as factory
>> - foreach row in rowsToInsert
>>   - create and bind it
>> - foreach ndx in toDeleteItems in reverse order
>>   - use on-delete to remove/mark
> 
> 
> 3 vs. 4: IMO @row-path-insert makes the strategy not really obvious. 
> Wouldn't it be better to specify explicitely whether reordering is 
> allowed/needed or not?
> 

yeah, that is the alternative which still makes me doubth

>> For completion:
>> implementation of the above assumes the refactoring of the current 
>> identity approach towards the earlier mentioned IdentityBinding 
>> interface.
>>
>> see: http://marc.theaimsgroup.com/?l=xml-cocoon-dev&m=107955523607012&w=2
>>
>> (Joerg, if you're ok, I'll start doing it when this discussion (if 
>> any) converges)
> 
> 
> Of course, you only remove another lame excuse for starting with my 
> diploma thesis :)
> 

oh, joerg, sorry man, I didn't know it was that important to you ;-)

>> While writing up this proposal I end up questioning a number of 
>> (historically chosen) names that could change:
>>
>> - repeater/@parent-path --> repeater/@path for consistency with the 
>> order bindings
> 
> 
> Have I written this somewhere above? :-)
> 
>> - repeater/@row-path --> repeater/@item-path since it is pointing to 
>> items, not rows
> 
> 
> Indeed, so it's reasonable.
> 

but it kinda requires me adding the above to the docos

which makes me think: I'm planning on only patching cforms (not woody) 
for this... we still don't have cforms docs in the wiki, right?

>> - on-insert is actually more of a on-create ('insert' seems to exclude 
>> 'append', while create is more nutral to exactly where the newly 
>> created is added, it is also more in sync with the actual factory 
>> registration IMHO )
> 
> 
> Not that important, but I'm also ok with this.
> 
>> - repeater/@row-path-insert --> repeater/@new-item-path which would be 
>> syncing up with the last two changes and arguments
> 
> 
> What do we need @row-path-insert/@new-item-path for if we have on-insert?
> 

well, that is part of the confusion of on-insert

on-insert isn't really used to insert stuff, it just registers a factory 
to create nodes when that would be needed

this means that it doesn't make sense to add path information on that 
element, since it will not actively use that. (it is the repeater 
deciding and doing that)

in any case, we added the new-item-path already some time ago (called 
row-path-insert) for those use cases were the new rows in a repeater 
needed to be added into a separate section

this use seems to collide with this case4, so that's why I figured to 
let it be it's trigger

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

Mime
View raw message