12:38 p.m.
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(a)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
>>>>
>>>>
>>
>
>
>