> 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
