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

Reply via email to