But the model's value is accessed by many other methods, not just
isVisible().
If you want to save the reflection overhead why don't you put a caching
model between your component and the CompoundPropertyModel?
Or access the model of the containing component if this is applicable:
public void CartPanel(IModel cart) {
super(new CompoundPropertyModel(cart));
add(new Label("total") {
public boolean isVisible() {
((Cart)CartPanel.this.getModelObject()).getTotal().isPositive();
}
});
}
I still don't see the need to introduce visibility caching into Wicket core.
Sven
Scott Swank schrieb:
That comes from a CompoundPropertyModel<Cart> and is bound to "total".
So getModelObject() corresponds to cart.getTotal(). From there,
isPositive() is quite cheap.
And we can, of course, keep implementing ad hoc caching of visibility.
It's in no way complex, however it seems preferable to have this as
the default behavior since only a very few components are likely to
want to change their visibility over the course of rendering. Is this
something that could be examined in 1.4 or 1.5 or is it simply
inappropriate -- perhaps due to component details with which I'm
unfamiliar?
On Fri, Jan 16, 2009 at 12:53 AM, <[email protected]> wrote:
What's taking so long in your isVisible() method?
The model object should be cached, and is isPositive() so expensive?
Sven
----- Ursprüngliche Nachricht -----
Von: Scott Swank
Gesendet: 16.01.09 02:06 Uhr
An: [email protected]
Betreff: Re: Why you should not override isVisible
We have implemented this, perhaps a dozen times or more across our
application. For example, there are several payment options whose
relevance is determined by whether the customer owes any money on
their purchase (e.g. as opposed to using a gift card). These "total
the order and determine visibility" methods were particular hot spots.
@Override
public boolean isVisible() {
if (visible == null)
visible = ((Money) getModelObject()).isPositive();
return visible;
}
While this is an idiosyncratic example, I can vouch for the fact that
performance woes in isVisible() show up in profiling.
On Thu, Jan 15, 2009 at 4:56 PM, Jonathan Locke
<[email protected]> wrote:
oh i suppose you also need to reset the value in onBeforeRender(). it's a
small pain, but how often does this really become a quantifiable problem and
not just a worry?
Jonathan Locke wrote:
sure, that's the clean way to do it, but it comes at the expense of
possibly breaking user code by surprise.
i'm not sure how big of a deal this is. i've heard people talk about it,
but i'd be interested in some examples of how performance of this method
has been a problem for people. i've never run into it myself and if i did
see it in a profiler, i'd probably just cache the value in a Boolean. it's
literally just this little bit in your anonymous class:
Boolean visible = null;
public isVisible() {
if (visible == null) {
visible = // whatever boolean computation
}
return visible;
}
and then it disappears from the profiler and who cares about the rest.
Scott Swank wrote:
My idea what an inversion of that one:
Add a method to Component, such as isVisibleInternal() [no I don't
love the name] that would cache the results of isVisible(). Then all
code that currently calls isVisible() would be changed to call
isVisibleInternal() instead. Someone who really wanted non-cached
visibility (seemingly the 1% case) could override isVisibleInternal(),
but everyone else would get caching for free with their current code.
On Thu, Jan 15, 2009 at 2:35 PM, Jonathan Locke
<[email protected]> wrote:
well, one simple design that would avoid the reuse problem is:
Boolean Component#isCachedVisible() { return null; }
then override to use visibility caching and return true or false.
if you don't override you get the current functionality.
of course you need two more bits in Component to support this...
one for whether isCachedVisible returned non-null and another
for the value it returned.
---------------------------------------------------------------------
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-tp21474995p21490479.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]
---------------------------------------------------------------------
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]
