so how is it different if they can still override something that needs to be checked all the time?
-igor On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pedros...@gmail.com> wrote: > I understand the concern about possible isVisible implementations like > > isVisible(return currentlyTime < 10:00:00;) //imagine this component being > rendered at 09:59:59 > isVisible(return dao.list().size() > 0);// performance issues > > But maybe we can have the best from both approaches. This is an copy/paste > from java.awt.Component: > > public boolean isVisible() { > return isVisible_NoClientCode(); > } > final boolean isVisible_NoClientCode() { > return visible; > } > > There are some points in the awt framework were the isVisible method is not > used in benefit of isVisible_NoClientCode > I'm in favor of create an final isVisible/Enabled version and change the > Wicket core to use it. Also maintain the hotspot to users provide their > isVisible/Enable implementations that will serve to feed the core component > state. > > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <igor.vaynb...@gmail.com>wrote: > >> ive run into plenty of weird problems with overrides, but maybe >> because this was in a high concurrency app where data changed >> frequently. the problems arise from the fact that the value returned >> from isvisible() can change while we are doing traversals, etc. >> >> eg we run a traversal for all visible components and put them in a >> list. later we iterate over the list and try to render these >> components. the render function also checks their visibility and if >> they are no longer visible it throws an exception. >> >> if isvisible() override depends on some external factor like the >> database there is a small window where the value can change and now >> you can have a weird exception: such as "tried to invoke a listener on >> a component that is not visible or not enabled". these are very >> intermittent and damn near impossible to reproduce. >> >> another problem is performance. isvisible() is called multiple times >> during the request and if it depends on the database it can be a >> performance problem. in fact a couple of users have complained about >> this on the list in the past. at least now we have an easy solution >> for them - use onconfigure(). >> >> so as of right now the developers have two choices: override >> isvisible() and potentially suffer the consequences. or, override >> onconfigure() and set visibility there in a more deterministic >> fashion. >> >> -igor >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius >> <eelco.hillen...@gmail.com> wrote: >> > To expand, unless I'm missing something (new?), things are really only >> > problematic when both the mutable value and the override are mixed. In >> > a way, I think that using the override is 'more pure', as it's a >> > simple function that is executed when needed, whereas mutable state >> > can be harder to deal with when trying to figure out how it got to be >> > in that state. So, sorry Igor, but we disagree on this one. >> > >> > Eelco >> > >> > >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius >> > <eelco.hillen...@gmail.com> wrote: >> >> Niether is evil. It has potential pitfalls, which you should just be >> >> aware of. We use such overrides all over the place and never have >> >> problems with them either. :-) Avoiding it is safer, but also more >> >> verbose (in 1.3.x at least). >> >> >> >> Eelco >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <igor.vaynb...@gmail.com> >> wrote: >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <s...@meiers.net> wrote: >> >>>> Hi Douglas, >> >>>> >> >>>> WICKET-3171 describes a problematic case, where visibility of a >> >>>> component changes while its form is being processed. >> >>>> In our projects we're overriding isVisible() where appropriate and >> never >> >>>> encountered a similar problem. >> >>>> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding >> >>>> isEnabled() going to be declared evil too? ;) >> >>> >> >>> yes >> >>> >> >>> -igor >> >>> >> >>>> >> >>>> Sven >> >>>> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote: >> >>>> >> >>>>> Can you explain why? We have done this all over the place. >> >>>>> >> >>>>> D/ >> >>>>> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote: >> >>>>> >> >>>>> > The recommended way since a few 1.4 releases is to override >> onConfigure() >> >>>>> > and call setVisible(true|false) depending on your conditions. >> >>>>> > >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson < >> >>>>> > doug...@douglasferguson.us> wrote: >> >>>>> > >> >>>>> >> Igor posted a comment to this bug saying that overriding >> isVisible() is >> >>>>> >> "evil" >> >>>>> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171 >> >>>>> >> >> >>>>> >> I was surprised by this and am curious to hear more. >> >>>>> >> >> >>>>> >> D/ >> >>>>> >> >>>> >> >>>> >> >>>> >> >>> >> >> >> > >> > > > > -- > Pedro Henrique Oliveira dos Santos >