well yes, it might break existing behavior, that's why i suggested having a workaround for that. although i would think that well-designed isVisible methods shouldn't generally have this problem.
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). 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]
