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