On of the main problems I see with onConfigure() is that is a general 'configuration' method. In our code, we ofter create an anonymous inner-class, overriding the isVisible() method. These methods often look like 'return super.isVisible() && someOtherCondition;'. This is a lot more difficult to implement in a onConfigure() method, especially when that method also configures a lot of other things. The only way to make that work, is by starting with a call to super.onConfigure(), and including super.isVisible() in the call to setVisible(). In this situation, I think overriding isVisible() is much cleaner.
Emond On Monday 29 November 2010 19:56:39 Igor Vaynberg 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/
