RE: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-13 Thread Chris Colman
So to summarize my rant: -1 for removing the ability to use add inside a constructor. +0 for improving the handling of oninitialize +1 for improving the documentation on the lifecycle of components and the event chain called during processing [2] I assume that means you don't care if 3218

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-10 Thread Maarten Billemont
On 10 Mar 2011, at 00:42, Igor Vaynberg wrote: i am +0 for refactoring the code so that oninitialize() cascade is not started from page#add() but from somewhere before page#onconfigure(). this way oninitialize() is safe to override in pages because it will not be invoked from page's

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-10 Thread Martijn Dashorst
Oh the horror I have been through the last 6 years where I could randomly call add() inside my page's and panel's constructor. Over 1500 hand crafted pages by about 4 hands of developers and they are all ready for anti-depressants because of the troubles the component constructor has caused. Yes,

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-10 Thread Maarten Billemont
[drama] So to summarize my rant: -1 for removing the ability to use add inside a constructor. +0 for improving the handling of oninitialize +1 for improving the documentation on the lifecycle of components and the event chain called during processing [2] I assume that means you don't

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-10 Thread Peter Ertl
-1 on removing add() from ctor, too it will piss of a huge load of wicket developers for a tiny and questionable benefit. things like this made me drop tapestry btw (thanks for the really big laugh, martijn :-) we don't want users to leave because we make decisions from within the ivory

RE: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-10 Thread Coleman, Chris
+1 for the approach you mention where onInitialize is called from the framework and not as a surprise side effect of calling page#add(). From: Maarten Billemont [mailto:lhun...@gmail.com] On 10 Mar 2011, at 00:42, Igor Vaynberg wrote: i am +0 for refactoring the code so that oninitialize()

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-10 Thread Scott Swank
+1 for keeping calls to add() in the Component constructor. This makes upgrading as painless as possible. +1 also for having onInitialize() mimic: private boolean initialized = false; /// etc protected void onBeforeRender() { if (!initialized) { onInitialize(); initialized = true; }

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Igor Vaynberg
On Tue, Mar 8, 2011 at 11:22 PM, Maarten Billemont lhun...@gmail.com wrote: Just like EJBs, you should be careful about how much interaction you do beyond your object's scope within the constructor.  Your component doesn't have a hierarchy, getPage() cannot be accessed, none of your subclass

RE: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Coleman, Chris
yep, calling overridable methods from constructors is bad - Yes I agree... you just made the case for making page.oninitialize() final... But isn't that the very thing that the whole overridable onInitialize method was intended to avoid as it gets called after construction by the framework.

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Igor Vaynberg
On Wed, Mar 9, 2011 at 12:22 AM, Coleman, Chris chris.cole...@thalesgroup.com.au wrote: yep, calling overridable methods from constructors is bad - Yes I agree... you just made the case for making page.oninitialize() final... But isn't that the very thing that the whole overridable

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Maarten Billemont
yep, calling overridable methods from constructors is bad - you just made the case for making page.oninitialize() final... No, I've made the case for this issue. Either add() should be disallowed from the constructor (meaning onInitialize won't get called from it), onInitialize should be

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Gary Thomas
On 3/8/11 12:33 PM, Martijn Dashorst wrote: On Tue, Mar 8, 2011 at 6:03 PM, GOODWIN, MATTHEW (ATTCORP) mg0...@att.com wrote: +1 for clear documentation/Javadoc explaining proper use of onInitialize. Does this exist somewhere? As someone new to Wicket I'm trying to learn as fast as I can and a

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Maarten Billemont
On 09 Mar 2011, at 10:44, Gary Thomas wrote: While a minority use-case, this allows for very elegant Wicket code when using Scala: add(new Form[Person](form, model) { add (new TextArea[String](text, (getModel - text)) { override def isVisible: Boolean = false }) }) Style

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Igor Vaynberg
On Wed, Mar 9, 2011 at 1:15 AM, Maarten Billemont lhun...@gmail.com wrote: yep, calling overridable methods from constructors is bad - you just made the case for making page.oninitialize() final... No, I've made the case for this issue.  Either add() should be disallowed from the constructor

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Eelco Hillenius
I have a c++ background and this kind of problem is even more dangerous in c++ (virtual calls don't work as normal in constructors). In Java also, I think making this known to the outside world before it is fully constructed is unsafe, as illustrated above. It's a potential problem we've been

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Gary Thomas
On 3/9/11 2:18 AM, Maarten Billemont wrote: On 09 Mar 2011, at 10:44, Gary Thomas wrote: While a minority use-case, this allows for very elegant Wicket code when using Scala: add(new Form[Person](form, model) { add (new TextArea[String](text, (getModel - text)) { override def

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Maarten Billemont
On 09 Mar 2011, at 19:15, Gary Thomas wrote: On 3/9/11 2:18 AM, Maarten Billemont wrote: On 09 Mar 2011, at 10:44, Gary Thomas wrote: While a minority use-case, this allows for very elegant Wicket code when using Scala: add(new Form[Person](form, model) { add (new

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Maarten Billemont
On 09 Mar 2011, at 18:56, Igor Vaynberg wrote: On Wed, Mar 9, 2011 at 1:15 AM, Maarten Billemont lhun...@gmail.com wrote: yep, calling overridable methods from constructors is bad - you just made the case for making page.oninitialize() final... No, I've made the case for this issue.

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Igor Vaynberg
On Wed, Mar 9, 2011 at 11:54 AM, Maarten Billemont lhun...@gmail.com wrote: On 09 Mar 2011, at 18:56, Igor Vaynberg wrote: On Wed, Mar 9, 2011 at 1:15 AM, Maarten Billemont lhun...@gmail.com wrote: yep, calling overridable methods from constructors is bad - you just made the case for making

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Maarten Billemont
On 09 Mar 2011, at 21:42, Igor Vaynberg wrote: oninitialize() is an atomic callback that notifies components when the page becomes available. the page is special because since a page is always available to itself it doesnt need such a callback. how is that for a rationalization... :) Only,

RE: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Chris Colman
unchanged. -Original Message- From: Maarten Billemont [mailto:lhun...@gmail.com] Sent: Thursday, 10 March 2011 8:28 AM To: Igor Vaynberg Cc: users@wicket.apache.org Subject: Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages On 09 Mar 2011, at 21:42, Igor Vaynberg wrote

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-09 Thread Maarten Billemont
On 09 Mar 2011, at 23:31, Chris Colman wrote: Surely there must be a way to support both styles of 'construction' (I use that term loosely) within the one framework. My proposed [3] and [4] already does that without an external configuration option that would confuse things about as much

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Maarten Billemont
Perhaps one question: Why is onInitialize invoked immediately after a component is added to a page? Page#componentAdded(final Component component) (line 1603 in 1.4.15). Is it really necessary to initialize the page here, and if so, why? The only possibly useful thing I can see it doing is

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Jim Pinkham
Interesting problem. It helped me visualize the issue when I tried to suggest: myPage() { add(new Thing(this,...)); // 'this' doesn't exist yet } I think building component hierarchy from constructors feels natural and it's going to be a hard sell to change to support this use case where

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Pedro Santos
I vote for solution 3: postpone the onInitialize call, possible to the first Component#configure execution. Then the problem of initialization code being executed for not fully constructed components go away. If it is really important to have a callback method for path cognizant components, than

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Hans Lesmeister 2
+1 for making Page.onInitialize() non-final. Since 1.4.12 we have migrated a lot of code moving construction of the component hierarchies to onInitialize. We are really very happy with the way it is now in 1.4. +1 for clear documentation/Javadoc explaining proper use of onInitialize. (Developers

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Maarten Billemont
On 08 Mar 2011, at 16:39, Pedro Santos wrote: I vote for solution 3: postpone the onInitialize call, possible to the first Component#configure execution. Then the problem of initialization code being executed for not fully constructed components go away. I actually also thought of that,

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Maarten Billemont
On 08 Mar 2011, at 17:38, Hans Lesmeister 2 wrote: +1 for making Page.onInitialize() non-final. Since 1.4.12 we have migrated a lot of code moving construction of the component hierarchies to onInitialize. We are really very happy with the way it is now in 1.4. -1 for throwing an

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Carl-Eric Menzel
On Tue, 8 Mar 2011 17:43:29 +0100 Maarten Billemont lhun...@gmail.com wrote: On 08 Mar 2011, at 16:39, Pedro Santos wrote: I vote for solution 3: postpone the onInitialize call, possible to the first Component#configure execution. Then the problem of initialization code being executed

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Carl-Eric Menzel
On Tue, 8 Mar 2011 17:46:26 +0100 Maarten Billemont lhun...@gmail.com wrote: This is a valid vote if we can come up with a way of not having random failure side-effects from mixing the two (which is the whole reason the the issue exists in the first place), without a final onInitialize and an

RE: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread GOODWIN, MATTHEW (ATTCORP)
-pattern from the sound of this thread. Thanks, Matt -Original Message- From: Hans Lesmeister 2 [mailto:hans.lesmeis...@lessy-software.de] Sent: Tuesday, March 08, 2011 11:38 AM To: users@wicket.apache.org Subject: Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages +1

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Maarten Billemont
On 08 Mar 2011, at 17:54, Carl-Eric Menzel wrote: However, there are much better arguments than existing codebase for still allowing constructors. First, constructors are natural places to do stuff like that. Yes, they have limitations, and yes, PostConstruct exists for a reason, but that

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Martijn Dashorst
On Tue, Mar 8, 2011 at 6:03 PM, GOODWIN, MATTHEW (ATTCORP) mg0...@att.com wrote: +1 for clear documentation/Javadoc explaining proper use of onInitialize. Does this exist somewhere? As someone new to Wicket I'm trying to learn as fast as I can and a lot of examples (almost exclusively) I see

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Igor Vaynberg
On Tue, Mar 8, 2011 at 11:42 AM, Maarten Billemont lhun...@gmail.com wrote: On 08 Mar 2011, at 17:54, Carl-Eric Menzel wrote: However, there are much better arguments than existing codebase for still allowing constructors. First, constructors are natural places to do stuff like that. Yes,

Re: [VOTE] WICKET-3218 - Component#onInitialize is broken for Pages

2011-03-08 Thread Maarten Billemont
Just like EJBs, you should be careful about how much interaction you do beyond your object's scope within the constructor. Your component doesn't have a hierarchy, getPage() cannot be accessed, none of your subclass constructors have been invoked and therefore your instance is not