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 <[email protected]> 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-tp21474995p21486093.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]
