i think the next step is to see what the core team thinks about this. filing
a ticket couldn't hurt.
Jonathan Locke 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-tp21474995p21486172.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]
