well, one simple design that would avoid the reuse problem is:
Boolean Component#isCachedVisible() { return null; }
then override to use visibility caching and return true or false.
if you don't override you get the current functionality.
of course you need two more bits in Component to support this...
one for whether isCachedVisible returned non-null and another
for the value it returned.
Jonathan Locke wrote:
>
>
> this is a really good point. i think i'm -1 on the idea too unless there's
> a good proposal for how to avoid that problem.
>
>
> igor.vaynberg wrote:
>>
>> if we do this then as a reusable component developer you have to
>> always be aware of this default and code with it in mind because it is
>> outside your control yet impacts the behaviors of components. -1 from
>> me.
>>
>> -igor
>>
>> On Thu, Jan 15, 2009 at 12:25 PM, Jonathan Locke
>> <[email protected]> wrote:
>>>
>>>
>>> yeah, for full flexibility, you might just link a method in Component to
>>> a
>>> default in IRequestCycleSettings:
>>>
>>> boolean getCacheComponentVisibility() { return
>>> getApplication().getRequestCycleSettings().getCacheComponentVisibility();
>>> }
>>>
>>> then you can set a default and override for any individual component (or
>>> page).
>>>
>>>
>>> Martin Makundi wrote:
>>>>
>>>>> you could always work it the other way and have no caching by default
>>>>> but
>>>>> allow people who know what they're doing to enable it for an
>>>>> application
>>>>> with something like
>>>>> IRequestCycleSettings#setCacheComponentVisibility(boolean).
>>>>
>>>> This sounds more robust, so long as it can be controlled on a
>>>> per-request basis (i.e., you can configure it for a specific page and
>>>> state).
>>>>
>>>> And don't forget about isEnabled too though it is more rare ;)
>>>>
>>>> **
>>>> Martin
>>>>
>>>>>
>>>>>
>>>>> Martin Makundi wrote:
>>>>>>
>>>>>> Well... I would not build in the cache because different components
>>>>>> could interact in a way that if someone prematurely calls the
>>>>>> isVisible method, it freezes in the wrong state. It is known, that
>>>>>> the
>>>>>> isVisible is called multiple times for various reasons.
>>>>>>
>>>>>> **
>>>>>> Martin
>>>>>>
>>>>>> 2009/1/15 Jonathan Locke <[email protected]>:
>>>>>>>
>>>>>>>
>>>>>>> not sure. it /ought/ to make sense to cache during the render cycle.
>>>>>>> but
>>>>>>> i
>>>>>>> have the eerie sense that this came up before, so maybe there's some
>>>>>>> reason
>>>>>>> why it can't be cached? (although even if there are some odd cases
>>>>>>> where
>>>>>>> it
>>>>>>> can't be, it seems like you could make a workaround for those cases
>>>>>>> ..)
>>>>>>>
>>>>>>>
>>>>>>> Scott Swank wrote:
>>>>>>>>
>>>>>>>> Is there a reason why the default behavior is not to cache the
>>>>>>>> result
>>>>>>>> of isVisible()? Are there cases where the result of isVisible() is
>>>>>>>> expected to change over the course of rendering?
>>>>>>>>
>>>>>>>> Would a JIRA w/ code be welcome, or is the current behavior
>>>>>>>> required?
>>>>>>>>
>>>>>>>> Scott
>>>>>>>>
>>>>>>>> On Thu, Jan 15, 2009 at 10:23 AM, Jonathan Locke
>>>>>>>> <[email protected]> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I would be careful not to throw the baby out with the bath water
>>>>>>>>> here.
>>>>>>>>> The
>>>>>>>>> design decision you're making is push versus pull and implementing
>>>>>>>>> isVisible
>>>>>>>>> is often going to be a better design decision because it may be
>>>>>>>>> more
>>>>>>>>> clear
>>>>>>>>> that visibility state stays consistent for a given problem. If the
>>>>>>>>> performance actually shows up as a hotspot (don't prematurely
>>>>>>>>> optimize!),
>>>>>>>>> you can always do some appropriate caching of the value and still
>>>>>>>>> have
>>>>>>>>> it
>>>>>>>>> "pulled".
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Erik van Oosten wrote:
>>>>>>>>>>
>>>>>>>>>> In the thread "Where to process PageParameters" I was requested
>>>>>>>>>> to
>>>>>>>>>> explain why I think you should not override isVisible, but rather
>>>>>>>>>> should
>>>>>>>>>> call setVisible in onBeforeRender (slide 100 in my presentation
>>>>>>>>>> http://www.grons.nl/~erik/pub/20081112%20Effective%20Wicket.pdf).
>>>>>>>>>>
>>>>>>>>>> There are 2 reasons, but only the second one is really important.
>>>>>>>>>>
>>>>>>>>>> -1- isVisible is called a lot. It is easily called ten times
>>>>>>>>>> within
>>>>>>>>>> 1
>>>>>>>>>> request
>>>>>>>>>>
>>>>>>>>>> So if you have anything processor intensive going on, it will be
>>>>>>>>>> a
>>>>>>>>>> performance hit. Just doing a simple expression is of course no
>>>>>>>>>> problem.
>>>>>>>>>> (For fun, just set a breakpoint in something like
>>>>>>>>>> FeedbackPanel#isVisible and request a page that contains one.)
>>>>>>>>>>
>>>>>>>>>> -2- isVisible can make your model be reloaded multiple times
>>>>>>>>>> within
>>>>>>>>>> 1
>>>>>>>>>> request
>>>>>>>>>>
>>>>>>>>>> Consider the following case (pseudo code):
>>>>>>>>>>
>>>>>>>>>> MyPanel(id, personId) {
>>>>>>>>>> super(id, new CompoundPropertyModel(new
>>>>>>>>>> LoadableDetachablePersonModel(personId)));
>>>>>>>>>> add( new Label("address") {
>>>>>>>>>> @Override
>>>>>>>>>> isVisible() {
>>>>>>>>>> return getDefaultModel() != null;
>>>>>>>>>> }
>>>>>>>>>> });
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The label uses the property 'address' of a person to see if the
>>>>>>>>>> label
>>>>>>>>>> should be visible. The person is retrieved by a
>>>>>>>>>> LoadableDetachableModel
>>>>>>>>>> subclass (LDM) and then wrapped by a CompoundPropertyModel (CPM).
>>>>>>>>>>
>>>>>>>>>> During the render phase, isVisible will delegate getting the
>>>>>>>>>> address
>>>>>>>>>> property to the CPM and this model delegates it to the LDM. LDM
>>>>>>>>>> will
>>>>>>>>>> load the person from the database only once (well until it is
>>>>>>>>>> detached).
>>>>>>>>>>
>>>>>>>>>> At the end of the render phase everything will be detached. But
>>>>>>>>>> now
>>>>>>>>>> something weird happens. The problem is that isVisible is called
>>>>>>>>>> during
>>>>>>>>>> the detach phase, on the label /after/ the CPM (and therefore
>>>>>>>>>> also
>>>>>>>>>> the
>>>>>>>>>> LDM) are detached. As isVisible retrieves the model from the CPM,
>>>>>>>>>> and
>>>>>>>>>> therefore from the LDM, it will trigger a reload of the person
>>>>>>>>>> inside
>>>>>>>>>> the LDM.
>>>>>>>>>>
>>>>>>>>>> Now, as visibility is often (if not almost always) determined by
>>>>>>>>>> a
>>>>>>>>>> business object (e.g. very often a LDM) I think it makes sense to
>>>>>>>>>> avoid
>>>>>>>>>> having to think about the situation described above, and just
>>>>>>>>>> avoid
>>>>>>>>>> it
>>>>>>>>>> all together.
>>>>>>>>>>
>>>>>>>>>> Note: I observed this behavior in Wicket 1.3 (1.3.3 I think). If
>>>>>>>>>> it
>>>>>>>>>> works differently now, I would be very glad to withdraw this
>>>>>>>>>> recommendation.
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Erik.
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Erik van Oosten
>>>>>>>>>> http://day-to-day-stuff.blogspot.com/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>>> To unsubscribe, e-mail: [email protected]
>>>>>>>>>> For additional commands, e-mail: [email protected]
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> View this message in context:
>>>>>>>>> http://www.nabble.com/Why-you-should-not-override-isVisible-tp21474995p21483816.html
>>>>>>>>> Sent from the Wicket - User mailing list archive at Nabble.com.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>> To unsubscribe, e-mail: [email protected]
>>>>>>>>> For additional commands, e-mail: [email protected]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: [email protected]
>>>>>>>> For additional commands, e-mail: [email protected]
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> View this message in context:
>>>>>>> http://www.nabble.com/Why-you-should-not-override-isVisible-tp21474995p21484456.html
>>>>>>> Sent from the Wicket - User mailing list archive at Nabble.com.
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: [email protected]
>>>>>>> For additional commands, e-mail: [email protected]
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: [email protected]
>>>>>> For additional commands, e-mail: [email protected]
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> View this message in context:
>>>>> http://www.nabble.com/Why-you-should-not-override-isVisible-tp21474995p21485341.html
>>>>> Sent from the Wicket - User mailing list archive at Nabble.com.
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: [email protected]
>>>>> For additional commands, e-mail: [email protected]
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [email protected]
>>>> For additional commands, e-mail: [email protected]
>>>>
>>>>
>>>>
>>>
>>> --
>>> View this message in context:
>>> http://www.nabble.com/Why-you-should-not-override-isVisible-tp21474995p21486093.html
>>> Sent from the Wicket - User mailing list archive at Nabble.com.
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [email protected]
>>> For additional commands, e-mail: [email protected]
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>>
>>
>
>
--
View this message in context:
http://www.nabble.com/Why-you-should-not-override-isVisible-tp21474995p21488394.html
Sent from the Wicket - User mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
