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]
