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
