A short test didn't uncover any calls to isVisible during detach phase
(1.4). Might need more extensive tests though...
Martijn
On Thu, Jan 15, 2009 at 10:12 PM, Jonathan Locke
<jonathan.lo...@gmail.com> 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
>> <jonathan.lo...@gmail.com> 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 <jonathan.lo...@gmail.com>:
>>>>>>>
>>>>>>>
>>>>>>> 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
>>>>>>>> <jonathan.lo...@gmail.com> 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: users-unsubscr...@wicket.apache.org
>>>>>>>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> 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: users-unsubscr...@wicket.apache.org
>>>>>>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
>>>>>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> 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: users-unsubscr...@wicket.apache.org
>>>>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
>>>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> 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: users-unsubscr...@wicket.apache.org
>>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
>>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>>
>>>>
>>>>
>>>
>>> --
>>> 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: users-unsubscr...@wicket.apache.org
>>> For additional commands, e-mail: users-h...@wicket.apache.org
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
>> For additional commands, e-mail: users-h...@wicket.apache.org
>>
>>
>>
>
> --
> View this message in context:
> http://www.nabble.com/Why-you-should-not-override-isVisible-tp21474995p21486990.html
> Sent from the Wicket - User mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
> For additional commands, e-mail: users-h...@wicket.apache.org
>
>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Apache Wicket 1.3.5 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
