[jsr-314-open-mirror] [jsr-314-open] [2.1 Spec Review] Transient State [Was: Fix UIData state saving model]

[Blake Sullivan]

Given that we are stuck with whatever apis we come up with forever.  I 
don't think that we should be adding public apis unless we are certain 
that we need them.  As long as we get the spec updates out on a 
consistent basis, it won't be a burden for our customers if we need to 
add these apis later.  And when we do add them, we should have a clearer 
set of use cases.

I am also still worried about the performance of these apis for 
complicated pages.  It is already the case that saving and restoring the 
EditableValueHolder state for each row is the most expensive thing that 
we do when processing the Table's lifecycle (assuming that the data is 
cached).  If we do have problems and end up needing a more complicated 
solution, the less api we have floating around out there, the better.

-- Blake Sullivan


On 10/25/10 4:53 AM, Andy Schwartz wrote:
> Hi Martin -
>
> I don't find it especially concerning that stateful/transient 
> components accessors look slightly different, though I am open to your 
> recommendation that we provide symmetry here.  However, I am concerned 
> that providing direct access to the TransientStateHelper outside of 
> the component (ie. via a public getTransientStateHelper() method) is a 
> (minor) violation of encapsulation - ie. no code other than the 
> component itself needs to know that the component happens to delegate 
> transient state management to a TransientStateHelper.  Perhaps a 
> solution that would satisfy both of our concerns would be public 
> get/putTransient() on UIComponent and protected getTransientStateHelper().
>
> Though if we've got public get/putTransient() on UIComponent, I still 
> wonder how much value is added by introducing the TransientStateHelper 
> concept - ie. seems like component authors would be able to use the 
> get/putTransient() APIs with relative ease.
>
> I am also wondering whether anyone other than UIComponent is going to 
> implement TransientStateHolder - and whether any code will ever 
> interact with this contract (as opposed to interacting with the 
> UIComponent methods).
>
> Andy
>
> On 10/22/10 4:26 PM, Martin Marinschek wrote:
>> Hi,
>>
>> my concern that stateful and transient component getters/setters look
>> different is not relevant to you?
>>
>> I am just thinking that users might be confused.
>>
>> best regards,
>>
>> Martin
>>
>> On 10/22/10, Blake Sullivan<blake.sullivan at oracle.com>  wrote:
>>    
>>>    Since we can always add the TransientStateHelper later if we need it,
>>> we shouldn't have it in the specification for this release.
>>>
>>> -- Blake Sullivan
>>>
>>> On 10/22/10 4:25 AM, Andy Schwartz wrote:
>>>      
>>>> Hey Martin -
>>>>
>>>> Here is how I was thinking about this...
>>>>
>>>> With transient properties, we have two types of use cases:
>>>>
>>>> 1.  Components storing transient attributes on themselves.
>>>> 2.  External classes storing transient attributes on components.
>>>>
>>>> The use case you mention below falls under #1.
>>>>
>>>> In earlier threads there were a references to use cases that fall
>>>> under #2 (eg. a renderer setting a transient property on a component).
>>>>
>>>> If we go with the original API proposal of adding public methods to
>>>> UIComponent, ie:
>>>>
>>>> - public Object getTransient(Object key)
>>>> - public Object putTransient(Object key, Object value)
>>>>
>>>> We have everything we need to address #1 and #2.  Your sample below
>>>> can be rewritten as:
>>>>
>>>> Double getCalculatedWeight() {
>>>>   return (Double) getTransient(PropertyKeys.weight, false);
>>>> }
>>>> void setCalculatedWeight(Double weight) {
>>>>   putTransient(PropertyKeys.weight, weight);
>>>> }
>>>>
>>>> I don't see how adding a new TransientStateHelper contract improves
>>>> this situation.  That's not to say that we cannot do this.  The first
>>>> patch that I provided leaves TransientStateHelper in place.  Just
>>>> seems like added conceptual overhead that might not be necessary.
>>>>
>>>> Andy
>>>>
>>>>
>>>> On 10/22/10 7:02 AM, Martin Marinschek wrote:
>>>>        
>>>>> Hi Andy,
>>>>>
>>>>> maybe I am missing something here, but isn't the normal use-case for
>>>>> this a new component which adds a transient attribute?
>>>>>
>>>>> something like: calculatedWeight. This should of course be exposed via
>>>>> the API of the component - so there should be a:
>>>>>
>>>>> Double getCalculatedWeight();
>>>>> void setCalculatedWeight(Double weight);
>>>>>
>>>>> And my point was that the implementation of these stateless getters
>>>>> and setters should look similar to the stateful version.
>>>>>
>>>>> What I would do:
>>>>>
>>>>> Double getCalculatedWeight() {
>>>>>    return (Double) getTransientStateHelper().get(PropertyKeys.weight,
>>>>> false);
>>>>> }
>>>>> void setCalculatedWeight(Double weight) {
>>>>>    getTransientStateHelper().put(PropertyKeys.weight, weight);
>>>>> }
>>>>>
>>>>> Could TransientStateHelper not be an alternative implementation of the
>>>>> StateHelper interface?
>>>>>
>>>>> best regards,
>>>>>
>>>>> Martin
>>>>>
>>>>>          
>>>      
>>
>>
>>    
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jsr-314-open-mirror/attachments/20101025/da7db5b2/attachment-0002.html