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]
