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 is marked as won't fix and
onInitialize remains overridable by those that choose to use it.

It depends if the current code calls onInitialize as a side effect of
calling page#add. If so then it would be good to change that so that
onInitialize is called by the framework after page contruction has
completed. The operation of onInitialize would then be nice a 'pure':
that is, it is never invoked during construction - even if someone calls
page#add in a constructor.


Documentation is a good enough alternative when there is an unresolved
issue that only occurs in rare cases.  So yes, document it, and let
those
that want to use onInitialize do so.

I never claimed using constructors will make your webapps eat your
young.
I simply outlined the pros and cons of each approach and argued the
design
advantages of not touching your components from outside wicket
lifecycle
methods.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 constructor but from some framework code at
 a later time.

Then I think that's the only actual solution that has positive votes.  Without 
any further feedback, can we agree to leave constructors as they are, make 
onInitialize overridable and do it this way?
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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, things are really bad when you execute
code in a constructor.

Seriously, in my 6+ years of developing Wicket applications, adding
components to the component hierarchy, I have yet to encounter the
problems you say are so detrimental to developing with Wicket.

Similarly calling methods from constructors that may be overridden.
Those things are easily discovered with a debugger. At my company I
have not encountered anything bad happening because of this. Perhaps
in 5 years one or two days in total lost on a team with 25 developers.
 whooptidoo. not. a. big. deal. Using Subversion, Eclipse, Hibernate,
Maven and Spring has caused much more lost hours than any problem you
mentioned.

There are just a couple of components, or rather models that require
the component path in order to work properly. However those are
typically better rewritten to evaluate their stuff at render time
rather than construction time. If that doesn't work, those usually
were better added in onBeforeRender, or more recently onInitialize or
onConfigure.

Given that in all my Wicket time I have not encountered unsurmountable
problems with using constructors for what they are intended for, I
will not be voting favorable for something that will throw away 6
years of code, libraries, books, blogs, articles and presentations,
just because one person dislikes constructors. If I wanted such an
experience, I'd used Tapestry instead.

Yes, we break compatibility between releases. We do this to improve
our framework and the way applications can be built. We strive to
minimize compatibility breaks and try to keep upgrades of medium sized
applications to a couple of days. We learnt our valuable lesson with
the constructor change—and we're not about to repeat that fiasco again
[1]. We try to balance the improvements we can achieve with the pain
it will cause for upgrading between versions. In this instance, the
pain is huge and community wide. The benefits are slim at best, as
evidenced by 6 years of successful application development. The
balance in this issue is severely off.

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]

Martijn

[1] we had to rewrite Wicket in Action twice, and still discovered old
2.0 code in the examples days before going to the press
[2] I'll have a go at that soonish if no-one beats me to the punch

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 care if 3218 is marked as won't fix and 
onInitialize remains overridable by those that choose to use it.

Documentation is a good enough alternative when there is an unresolved issue 
that only occurs in rare cases.  So yes, document it, and let those that want 
to use onInitialize do so.

I never claimed using constructors will make your webapps eat your young.  I 
simply outlined the pros and cons of each approach and argued the design 
advantages of not touching your components from outside wicket lifecycle 
methods.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 
tower.


Am 10.03.2011 um 14:07 schrieb 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 care if 3218 is marked as won't fix and 
 onInitialize remains overridable by those that choose to use it.
 
 Documentation is a good enough alternative when there is an unresolved issue 
 that only occurs in rare cases.  So yes, document it, and let those that want 
 to use onInitialize do so.
 
 I never claimed using constructors will make your webapps eat your young.  I 
 simply outlined the pros and cons of each approach and argued the design 
 advantages of not touching your components from outside wicket lifecycle 
 methods.
 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org
 


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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() 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 constructor but from some framework code at
 a later time.

Then I think that's the only actual solution that has positive votes.  Without 
any further feedback, can we agree to leave constructors as they are, make 
onInitialize overridable and do it this way?
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org




DISCLAIMER:---
This e-mail transmission and any documents, files and previous e-mail messages
attached to it are private and confidential. They may contain proprietary or 
copyright
material or information that is subject to legal professional privilege. They 
are for
the use of the intended recipient only.  Any unauthorised viewing, use, 
disclosure,
copying, alteration, storage or distribution of, or reliance on, this message is
strictly prohibited. No part may be reproduced, adapted or transmitted without 
the
written permission of the owner. If you have received this transmission in 
error, or
are not an authorised recipient, please immediately notify the sender by return 
email,
delete this message and all copies from your e-mail system, and destroy any 
printed
copies. Receipt by anyone other than the intended recipient should not be 
deemed a
waiver of any privilege or protection. Thales Australia does not warrant or 
represent
that this e-mail or any documents, files and previous e-mail messages attached 
are
error or virus free.
--


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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;
  }
  // other onBeforeRender stuff
}

Thank you all,
Scott

On Thu, Mar 10, 2011 at 2:54 PM, Coleman, Chris
chris.cole...@thalesgroup.com.au wrote:
 +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() 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 constructor but from some framework code at
 a later time.

Then I think that's the only actual solution that has positive votes.  
Without any further feedback, can we agree to leave constructors as they are, 
make onInitialize overridable and do it this way?
 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org




 DISCLAIMER:---
 This e-mail transmission and any documents, files and previous e-mail messages
 attached to it are private and confidential. They may contain proprietary or 
 copyright
 material or information that is subject to legal professional privilege. They 
 are for
 the use of the intended recipient only.  Any unauthorised viewing, use, 
 disclosure,
 copying, alteration, storage or distribution of, or reliance on, this message 
 is
 strictly prohibited. No part may be reproduced, adapted or transmitted 
 without the
 written permission of the owner. If you have received this transmission in 
 error, or
 are not an authorised recipient, please immediately notify the sender by 
 return email,
 delete this message and all copies from your e-mail system, and destroy any 
 printed
 copies. Receipt by anyone other than the intended recipient should not be 
 deemed a
 waiver of any privilege or protection. Thales Australia does not warrant or 
 represent
 that this e-mail or any documents, files and previous e-mail messages 
 attached are
 error or virus free.
 --


 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org



-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 
 constructors have been invoked and therefore your instance is not properly 
 initialized and ready for use.
 not really sure what you mean by subclass constructors or how they
 come into play when constructing an instance...
 If I understand correctly, here is an example of what he means :


 Exactly.  If B extends A and I do something in A's constructor that goes 
 beyond the scope of setting up A's instance variables, then I risk a 
 side-effect that relies on the instance to be constructed at B's level as 
 well.  There's nothing in the JVM that prevents this and these bugs are very 
 hard to see and debug.  They should be avoided by good coding practices.

yep, calling overridable methods from constructors is bad - you just
made the case for making page.oninitialize() final...


 On 08 Mar 2011, at 22:07, Igor Vaynberg wrote:

 i think code like this should be possible:

 NameEditor e=new NameEditor(name, firstNameModel, lastNameModel);
 e.getFirstNameEditor().setRequired(true);
 e.getLastNameEditor().setEnabled(false);

 taking the previous example of a name editor, with constructor we have:

 class nameeditor extends panel {
  public nameeditor(...) {
     add(new textfield(first,..);
     add(new textifled(last, ...);
  }
  public textfield getfirst() { return get(first); }
  public textfield getlast() { return get(last); }
 }

 Firstly, it's my opinion that you really shouldn't be doing anything to 
 components directly, especially not from outside your class.  As for why, see 
 Encapsulation and Law of Demeter.

neither of the two apply because nameeditor explicitly exposes the two
components as part of its contract via public getters.

 But if you really wanted to use this pattern, it really wouldn't have to be 
 as cumbersome as you make it out to be.  Also, I really don't like condoning 
 get(String), it's nasty and very type-unfriendly.  It also breaks as soon as 
 you do any refactoring in your component hierarchy.

yes, i traded off some refactorability for better memory footprint.
imho a perfectly valid trade off when memory and space are costly.

 class NameEditor extends Panel {
    TextFieldString firstField;
    TextFieldString lastField;
    @Override protected void onInitialize() { super.onInitialize(); 
 add(getFirst(), getLast()); }
    public textfield getFirst() { if (firstField == null) firstField = new 
 TextFieldString(); return firstField; }
    public textfield getLast() { if (lastField == null) lastField = new 
 TextFieldString(); return lastField; }
 }

there are still problems with this:
*you still have the two extra memory slots that are a complete waste
here - component references
*the lazy init code is cumbersome to write
*you are missing the two slots needed to pass models from constructor
to the two textfields - so thats four extra slots total that you have
now forced on users
* the components i get from the getters are not added to their parent
- for more complex components that need access to their markup this is
a problem, eg if
(text.equals(getFirst().getMarkupTag().get(type))...not
necessarily a good example taking this particular custom component as
context, but a valid usecase in plenty of other situations.

-igor


 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org



-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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. 
Any post construction code can simply be put in the onInitialize method.



DISCLAIMER:---
This e-mail transmission and any documents, files and previous e-mail messages
attached to it are private and confidential. They may contain proprietary or 
copyright
material or information that is subject to legal professional privilege. They 
are for
the use of the intended recipient only.  Any unauthorised viewing, use, 
disclosure,
copying, alteration, storage or distribution of, or reliance on, this message is
strictly prohibited. No part may be reproduced, adapted or transmitted without 
the
written permission of the owner. If you have received this transmission in 
error, or
are not an authorised recipient, please immediately notify the sender by return 
email,
delete this message and all copies from your e-mail system, and destroy any 
printed
copies. Receipt by anyone other than the intended recipient should not be 
deemed a
waiver of any privilege or protection. Thales Australia does not warrant or 
represent
that this e-mail or any documents, files and previous e-mail messages attached 
are
error or virus free.
--


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 onInitialize method 
 was intended to avoid as it gets called after construction by the framework. 
 Any post construction code can simply be put in the onInitialize method.

the way the code is implemented page's oninitialize() would get called
on first page#add() invocation and since the add() invocation is most
likely inside page's constructor...

-igor




 DISCLAIMER:---
 This e-mail transmission and any documents, files and previous e-mail messages
 attached to it are private and confidential. They may contain proprietary or 
 copyright
 material or information that is subject to legal professional privilege. They 
 are for
 the use of the intended recipient only.  Any unauthorised viewing, use, 
 disclosure,
 copying, alteration, storage or distribution of, or reliance on, this message 
 is
 strictly prohibited. No part may be reproduced, adapted or transmitted 
 without the
 written permission of the owner. If you have received this transmission in 
 error, or
 are not an authorised recipient, please immediately notify the sender by 
 return email,
 delete this message and all copies from your e-mail system, and destroy any 
 printed
 copies. Receipt by anyone other than the intended recipient should not be 
 deemed a
 waiver of any privilege or protection. Thales Australia does not warrant or 
 represent
 that this e-mail or any documents, files and previous e-mail messages 
 attached are
 error or virus free.
 --


 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org



-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 made final (meaning onInitialize will do nothing when it gets 
called), or Page should stop calling initialize() as a result of a component 
being added, and behave like any other component does.  Would you mind 
addressing this third option?  What exactly is the reason for Page behaving in 
this special way?


On 09 Mar 2011, at 09:15, Igor Vaynberg wrote:
 
 Firstly, it's my opinion that you really shouldn't be doing anything to 
 components directly, especially not from outside your class.  As for why, 
 see Encapsulation and Law of Demeter.
 
 neither of the two apply because nameeditor explicitly exposes the two
 components as part of its contract via public getters.

Allow me to remind you:

 More formally, the Law of Demeter for functions requires that a method M of 
 an object O may only invoke the methods of the following kinds of objects:
 
   • O itself
   • M's parameters
   • any objects created/instantiated within M
   • O's direct component objects
   • a global variable, accessible by O, in the scope of M

Making your components available via getters does not excuse anything.  The 
list does not include invoke methods on any objects created by any objects 
created within M, for good reason.

 Also, I really don't like condoning get(String), it's nasty and very 
 type-unfriendly.  It also breaks as soon as you do any refactoring in your 
 component hierarchy.
 
 yes, i traded off some refactorability for better memory footprint.
 imho a perfectly valid trade off when memory and space are costly.

 there are still problems with this:
 *you still have the two extra memory slots that are a complete waste
 here - component references


You're talking about the cost of a reference to a component.  It's not like 
you've got a COPY of the component in your field.  And no, it's not a good 
trade-off.  You're writing Java applications, you'll be deploying in Java 
application containers, you're working with a web framework that goes to great 
lengths to help the container provide solid clustering.  Your memory footprint 
is absolutely subordinate to decently maintainable and compile-time checked 
code.  If you're a developer that doesn't appreciate the latter, you'll find 
that you can be a lot more flexible using a language with weak type checking, 
at the cost of maintainable code.  Any Java developer should not be willing to 
make this trade-off, especially when it's as light as this one.  Your component 
gets added to your page anyway!

 *the lazy init code is cumbersome to write

Hardly, but you shouldn't be doing it anyway except for special cases where you 
can forgive yourself the bad design.

 *you are missing the two slots needed to pass models from constructor
 to the two textfields - so thats four extra slots total that you have
 now forced on users

You really shouldn't be having references to your components and have only two 
slots for your models, but if you want to provide getters for your components, 
yes, you need instance fields for them.  Because get(String) is nasty.  This 
really doesn't have anything to do with the difference between constructor and 
onInitialize initialization, except for the added bonus that you can't do nasty 
get(String) in the latter case before your component is initialized.  The 
second option is looking better and better to me.

 * the components i get from the getters are not added to their parent
 - for more complex components that need access to their markup this is
 a problem, eg if
 (text.equals(getFirst().getMarkupTag().get(type))...not
 necessarily a good example taking this particular custom component as
 context, but a valid usecase in plenty of other situations.

Irrelevant, even if you added them in your constructor, you don't have access 
to your markup before you add your newly created NameEditor to a page 
hierarchy.  This example does help to confirm the need for an onInitialize: 
Don't touch your components until onInitialize is called indicating that the 
page hierarchy and markup are available.


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 lot of examples (almost exclusively) I see out
there show the add(..) from within the constructor - which is apparently
an anti-pattern from the sound of this thread.


It is most certainly not an antipattern in my book. I find the
reaction of the anti-constructor folks too strong and am trying to
formulate a civil reaction to this whole anti constructor rant.

Martijn



*Please* don't deprecate add() from constructor.

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
})
})

etc.


Since the body of a class is the constructor in Scala, this is a perfect 
fit for closure-like structures when using Wicket.


Thanks,
Gary

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 is great when there are no issues involved with doing it.  If it turns 
out that adding components from the constructor is indeed dangerous when 
combined with other components (eg. in libraries) that use onInitialize then we 
should reconsider.

I think the best solution so far would be to not invoke onInitialize when 
adding components to a page, that would allow the constructor to be used still 
if the developer really wants to.

With regards to your Scala code, while I don't know any Scala whatsoever, 
wouldn't it be just as easy to do something like this:

add(new Form[Person](form, model) {
   override def onInitialize: add (new TextArea[String](text, (getModel - 
text)) {
   override def isVisible: Boolean = false
   })
})

Which would give you the added lazy component construction bonus that 
onInitialize provides, as well as your elegance.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 (meaning onInitialize won't get called from it), 
 onInitialize should be made final (meaning onInitialize will do nothing when 
 it gets called), or Page should stop calling initialize() as a result of a 
 component being added, and behave like any other component does.  Would you 
 mind addressing this third option?  What exactly is the reason for Page 
 behaving in this special way?

page.add() is where the oninitialize cascade is initiated because that
is the earliest point at which the page instance is known. doing it
from there ensures that the page is more or less fully constructed
after it is instantiated allowing you to call methods on it.

eg MyPage page=new MyPage();
page.foo() { where foo manipulates state of page's child components }

i know that is not the style you prefer to use in your applications,
but that doesnt mean no one should be able to use it.



 On 09 Mar 2011, at 09:15, Igor Vaynberg wrote:

 Firstly, it's my opinion that you really shouldn't be doing anything to 
 components directly, especially not from outside your class.  As for why, 
 see Encapsulation and Law of Demeter.

 neither of the two apply because nameeditor explicitly exposes the two
 components as part of its contract via public getters.

 Allow me to remind you:

 More formally, the Law of Demeter for functions requires that a method M of 
 an object O may only invoke the methods of the following kinds of objects:

       • O itself
       • M's parameters
       • any objects created/instantiated within M
       • O's direct component objects
       • a global variable, accessible by O, in the scope of M

 Making your components available via getters does not excuse anything.  The 
 list does not include invoke methods on any objects created by any objects 
 created within M, for good reason.

yes, and if we follow this good reason then i would have to add the
following methods:

nameditor {
  setfirstnamevisible() {}
  setlastnamevisible() {}
  setfirstnameenabled() {}
  setlastnameenabled() {}
  setfirstnamerequired() {}
  setlastnamerequired() {}
  addfirstnamevalidator() {}
  addlastnamevalidator() {}
  setfirstnamelabel() {}
  setlastnamelabel() {}
}

just to name a few, and all they do is forward stuff to the two inner
components. sounds like a lot of fun...

 Also, I really don't like condoning get(String), it's nasty and very 
 type-unfriendly.  It also breaks as soon as you do any refactoring in your 
 component hierarchy.

 yes, i traded off some refactorability for better memory footprint.
 imho a perfectly valid trade off when memory and space are costly.

 there are still problems with this:
 *you still have the two extra memory slots that are a complete waste
 here - component references


 You're talking about the cost of a reference to a component.  It's not like 
 you've got a COPY of the component in your field.  And no, it's not a good 
 trade-off.  You're writing Java applications, you'll be deploying in Java 
 application containers, you're working with a web framework that goes to 
 great lengths to help the container provide solid clustering.  Your memory 
 footprint is absolutely subordinate to decently maintainable and compile-time 
 checked code.  If you're a developer that doesn't appreciate the latter, 
 you'll find that you can be a lot more flexible using a language with weak 
 type checking, at the cost of maintainable code.  Any Java developer should 
 not be willing to make this trade-off, especially when it's as light as this 
 one.  Your component gets added to your page anyway!

in a perfect world where your servers have unlimited amounts of memory
and your cluster's backplane is infinitely fast that would be true.
apparently youve missed some recent threads on this list about people
trying to optimize state. when you have a table with 100-200 rows and
each row has 2-3 instances of such editors, all of a sudden those two
extra slots add up.

if you dont need to optimize your state thats great, but we should
leave the door open for when you do.

 *the lazy init code is cumbersome to write

 Hardly, but you shouldn't be doing it anyway except for special cases where 
 you can forgive yourself the bad design.

well, in that case you shouldnt mind writing code like this:

class mypage extends page {

   private boolean initialized=false;

   protected void onconfigure() { if (!initialized) {
initialized=true; myOneTimeInitializationCode(); }}

}

 *you are missing the two slots needed to pass models from constructor
 to the two textfields - so thats four extra slots total that you have
 now forced on users

 You really shouldn't be having references to your components and have only 
 

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 aware of and consciously allowed
to exist in Wicket for a long time after our failed attempt to address
it on Wicket 2.0 (years ago). In the end I don't think it is a
concrete very often, and the difficulties that get introduced trying
to fix it are simply not worth it in our case.

Eelco

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 isVisible: Boolean = false
})
})


Style is great when there are no issues involved with doing it.  If it turns 
out that adding components from the constructor is indeed dangerous when 
combined with other components (eg. in libraries) that use onInitialize then we 
should reconsider.

I think the best solution so far would be to not invoke onInitialize when 
adding components to a page, that would allow the constructor to be used still 
if the developer really wants to.

With regards to your Scala code, while I don't know any Scala whatsoever, 
wouldn't it be just as easy to do something like this:

add(new Form[Person](form, model) {
override def onInitialize: add (new TextArea[String](text, (getModel -  
text)) {
override def isVisible: Boolean = false
})
})



Thanks for the reply -
Yes it would be as simple as that - but again, not quite as elegant (imho).

I admit my argument is pretty minor, but elegant code is a hallmark of 
an API or library that is designed well, and I think Wicket is great in 
that regard.


Scala aside, the current usage of add() from the constructor just makes 
sense to me, since you really are constructing the component.  Fully 
understood there are valid concerns about the side effects of this 
design, but would love to see that resolved transparently if at all 
possible.


I'm not an expert on the internals, but am a very happy user of Wicket :-)

Thanks,
Gary

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 TextArea[String](text, (getModel -  text)) {
override def isVisible: Boolean = false
})
 })
 
 Style is great when there are no issues involved with doing it.  If it turns 
 out that adding components from the constructor is indeed dangerous when 
 combined with other components (eg. in libraries) that use onInitialize then 
 we should reconsider.
 
 I think the best solution so far would be to not invoke onInitialize when 
 adding components to a page, that would allow the constructor to be used 
 still if the developer really wants to.
 
 With regards to your Scala code, while I don't know any Scala whatsoever, 
 wouldn't it be just as easy to do something like this:
 
 add(new Form[Person](form, model) {
override def onInitialize: add (new TextArea[String](text, (getModel - 
  text)) {
override def isVisible: Boolean = false
})
 })
 
 
 Thanks for the reply -
 Yes it would be as simple as that - but again, not quite as elegant (imho).

I actually think that it's more elegant to be explicit about the fact that your 
components are constructed lazily during initialization rather than object 
construction.

Given the recent feedback, cede that forcing onInitialize on everybody may not 
be necessary, although I am still very convinced that it is the right thing to 
do design-wise.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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.  Either add() should be disallowed 
 from the constructor (meaning onInitialize won't get called from it), 
 onInitialize should be made final (meaning onInitialize will do nothing when 
 it gets called), or Page should stop calling initialize() as a result of a 
 component being added, and behave like any other component does.  Would you 
 mind addressing this third option?  What exactly is the reason for Page 
 behaving in this special way?
 
 page.add() is where the oninitialize cascade is initiated because that
 is the earliest point at which the page instance is known. doing it
 from there ensures that the page is more or less fully constructed
 after it is instantiated allowing you to call methods on it.

Why should Page be a special case?  Why not for Panels too?

In fact, if you take away this special casing for Page, this issue probably 
wouldn't exist at all and we could go back to letting the constructor-favored 
users use constructors and the others use onInitialize.

 
 eg MyPage page=new MyPage();
 page.foo() { where foo manipulates state of page's child components }
 
 i know that is not the style you prefer to use in your applications,
 but that doesnt mean no one should be able to use it.

This issue is about the fact that this style, which I've argued is flawed, is 
basically causing you to finalize onInitialize, meaning no one will be able to 
use the alternative.  It cuts both ways, and I really don't see why 
onInitialize should come out as the looser when it condones safer code.

 
 
 
 On 09 Mar 2011, at 09:15, Igor Vaynberg wrote:
 
 Firstly, it's my opinion that you really shouldn't be doing anything to 
 components directly, especially not from outside your class.  As for why, 
 see Encapsulation and Law of Demeter.
 
 neither of the two apply because nameeditor explicitly exposes the two
 components as part of its contract via public getters.
 
 Allow me to remind you:
 
 More formally, the Law of Demeter for functions requires that a method M of 
 an object O may only invoke the methods of the following kinds of objects:
 
   • O itself
   • M's parameters
   • any objects created/instantiated within M
   • O's direct component objects
   • a global variable, accessible by O, in the scope of M
 
 Making your components available via getters does not excuse anything.  The 
 list does not include invoke methods on any objects created by any objects 
 created within M, for good reason.
 
 yes, and if we follow this good reason then i would have to add the
 following methods:
 
 nameditor {
  setfirstnamevisible() {}
  setlastnamevisible() {}
  setfirstnameenabled() {}
  setlastnameenabled() {}
  setfirstnamerequired() {}
  setlastnamerequired() {}
  addfirstnamevalidator() {}
  addlastnamevalidator() {}
  setfirstnamelabel() {}
  setlastnamelabel() {}
 }
 
 just to name a few, and all they do is forward stuff to the two inner
 components. sounds like a lot of fun...

Actually, that would be a workaround to hack away a warning your IDE might give 
you about the Law of Demeter.  The point of the practice is that outside of 
NameEditor, you shouldn't be doing anything that requires knowledge of how 
NameEditor works.  Which means you shouldn't be exposing any of its components, 
and definitely none of its components' properties.  If you want to do something 
to your NameEditor that changes the way it shows your name, you should express 
that in terms that make sense for a name editor.

new NameEditor(NameEditorMode.FIRST_NAME_ONLY);

Or whatever API makes sense to you.  Just don't expose NameEditor's guts.

 
 Also, I really don't like condoning get(String), it's nasty and very 
 type-unfriendly.  It also breaks as soon as you do any refactoring in your 
 component hierarchy.
 
 yes, i traded off some refactorability for better memory footprint.
 imho a perfectly valid trade off when memory and space are costly.
 
 there are still problems with this:
 *you still have the two extra memory slots that are a complete waste
 here - component references
 
 
 You're talking about the cost of a reference to a component.  It's not like 
 you've got a COPY of the component in your field.  And no, it's not a good 
 trade-off.  You're writing Java applications, you'll be deploying in Java 
 application containers, you're working with a web framework that goes to 
 great lengths to help the container provide solid clustering.  Your memory 
 footprint is absolutely subordinate to decently maintainable and 
 compile-time checked code.  If you're a developer that doesn't appreciate 
 the latter, you'll find that you can be a lot more flexible using a language 
 with weak type checking, at the 

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 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 made final (meaning onInitialize will do nothing 
 when it gets called), or Page should stop calling initialize() as a result 
 of a component being added, and behave like any other component does.  
 Would you mind addressing this third option?  What exactly is the reason 
 for Page behaving in this special way?

 page.add() is where the oninitialize cascade is initiated because that
 is the earliest point at which the page instance is known. doing it
 from there ensures that the page is more or less fully constructed
 after it is instantiated allowing you to call methods on it.

 Why should Page be a special case?  Why not for Panels too?

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... :)

you are mixing a generic post-construct callback with a more specific
usecase of oninitialize()

 In fact, if you take away this special casing for Page, this issue probably 
 wouldn't exist at all and we could go back to letting the constructor-favored 
 users use constructors and the others use onInitialize.

that may very well be true. but weve come a long way from the
beginning of this thread which was: forbid add() in constructors.



 eg MyPage page=new MyPage();
 page.foo() { where foo manipulates state of page's child components }

 i know that is not the style you prefer to use in your applications,
 but that doesnt mean no one should be able to use it.

 This issue is about the fact that this style, which I've argued is flawed, is 
 basically causing you to finalize onInitialize, meaning no one will be able 
 to use the alternative.  It cuts both ways, and I really don't see why 
 onInitialize should come out as the looser when it condones safer code.




 On 09 Mar 2011, at 09:15, Igor Vaynberg wrote:

 Firstly, it's my opinion that you really shouldn't be doing anything to 
 components directly, especially not from outside your class.  As for why, 
 see Encapsulation and Law of Demeter.

 neither of the two apply because nameeditor explicitly exposes the two
 components as part of its contract via public getters.

 Allow me to remind you:

 More formally, the Law of Demeter for functions requires that a method M 
 of an object O may only invoke the methods of the following kinds of 
 objects:

       • O itself
       • M's parameters
       • any objects created/instantiated within M
       • O's direct component objects
       • a global variable, accessible by O, in the scope of M

 Making your components available via getters does not excuse anything.  The 
 list does not include invoke methods on any objects created by any objects 
 created within M, for good reason.

 yes, and if we follow this good reason then i would have to add the
 following methods:

 nameditor {
  setfirstnamevisible() {}
  setlastnamevisible() {}
  setfirstnameenabled() {}
  setlastnameenabled() {}
  setfirstnamerequired() {}
  setlastnamerequired() {}
  addfirstnamevalidator() {}
  addlastnamevalidator() {}
  setfirstnamelabel() {}
  setlastnamelabel() {}
 }

 just to name a few, and all they do is forward stuff to the two inner
 components. sounds like a lot of fun...

 Actually, that would be a workaround to hack away a warning your IDE might 
 give you about the Law of Demeter.  The point of the practice is that outside 
 of NameEditor, you shouldn't be doing anything that requires knowledge of how 
 NameEditor works.  Which means you shouldn't be exposing any of its 
 components, and definitely none of its components' properties.  If you want 
 to do something to your NameEditor that changes the way it shows your name, 
 you should express that in terms that make sense for a name editor.

 new NameEditor(NameEditorMode.FIRST_NAME_ONLY);

 Or whatever API makes sense to you.  Just don't expose NameEditor's guts.

the two components are not considered guts. the nameeditor
encapsulates the markup used to render the two components in my
application, as well as a good set of defaults. however, it leaves
room open for overriding those defaults. there is nothing wrong with
this design.



 Also, I really don't like condoning get(String), it's nasty and very 
 type-unfriendly.  It also breaks as soon as you do any refactoring in 
 your component hierarchy.

 yes, i traded off some refactorability for better memory footprint.
 imho a perfectly valid trade 

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, you're special-casing initialize() in Pages so that onInitialize() gets 
called as soon as the page is available, only to then disallow developers from 
using onInitialize() by making it final.  Kind of defeats the whole point, 
doesn't it.  And I can't see initialize() doing anything else that the 
developer might notice.  So really, just don't do it.

 
 In fact, if you take away this special casing for Page, this issue probably 
 wouldn't exist at all and we could go back to letting the 
 constructor-favored users use constructors and the others use onInitialize.
 
 that may very well be true. but weve come a long way from the
 beginning of this thread which was: forbid add() in constructors.

We have come a long way, but if onInitialize didn't behave this way for pages, 
that would be an ideal solution to the issue that wouldn't force onInitialize 
or the constructor (or a lame onConfigure hack).

 interesting because in the beginning of this thread you wanted to
 blindly castrate constructors :)

I explained the issue and proposed solutions.  One of them involved not 
creating component hierarchies from the constructors.  I wouldn't call that 
castrating: You still get to use them for what they're supposed to do: 
initialize your instance's state.

 i have proposed a way to deal with
 the issue. you can create your own one-time callback in the page from
 inside onconfigure().

So to round up, the proposed solutions are:

1. Forbid add() in constructor.
2. Forbid onInitialize() in pages (and fake one with onConfigure code that 
really has nothing to do with configuring but is onInitialize code in a block 
that requires a test of an instance field that takes up a whole boolean field 
in your page state, and do that for each of your pages).
3. Don't initialize() on add() in pages.

 * -1 to forbid add() from constructors
 * +0 to delay oninitialize() cascade until right before the first call
 to onconfigure()


So that's [1] and [3] out the window, does that mean you vote +1 for [2]?  
Because as I mentioned before, you can't in all honesty argue the case of 
leaving the door open for add() in constructors if the developer want to do 
that and slam the door shut for add() in onInitialize() if the developer wants 
to do that.

Perhaps we can add a [4]:

4. Allow both and ignore the fact that mixing them will break things in pages.  
Heck, if we can allow doing potentially bad things in constructors because the 
bad is rare anyway, why not, right?

My vote, in order of preference, would be [1], [3], [4], [2].
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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

2011-03-09 Thread Chris Colman
Just a thought:

Surely there must be a way to support both styles of 'construction' (I
use that term loosely) within the one framework.

Perhaps a runtime switch in the properties could dictate the
'construction' mode.

You could leave the default as 'traditional wicket' mode but in 'two
phase' construction mode a clear two phase constructor/init strategy is
used whereby init is called only after parent object construction is
complete so that all virtual method calls will be made from within the
init method and so guatrateed to be operating on a 'fully constructed
object' instead of the current situation where it can be operating on a
partically constructed parent object - nasty! (I admit it is more
dangerous in languages like C++ but it can still cause significant
problems in Java). In this two phase mode exceptions could be thrown
if an attempt is made to do something which is dangerous or would go
against the strategy.

That means everybody used to other frameworks where a separate 'init'
phase takes place after parent object construction is complete will be
happy because they can switch to 'two phase' mode but at the same time
all existing wicket code will continue to work 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:
 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, you're special-casing initialize() in Pages so that
onInitialize()
gets called as soon as the page is available, only to then disallow
developers from using onInitialize() by making it final.  Kind of
defeats
the whole point, doesn't it.  And I can't see initialize() doing
anything
else that the developer might notice.  So really, just don't do it.


 In fact, if you take away this special casing for Page, this issue
probably wouldn't exist at all and we could go back to letting the
constructor-favored users use constructors and the others use
onInitialize.

 that may very well be true. but weve come a long way from the
 beginning of this thread which was: forbid add() in constructors.

We have come a long way, but if onInitialize didn't behave this way for
pages, that would be an ideal solution to the issue that wouldn't force
onInitialize or the constructor (or a lame onConfigure hack).

 interesting because in the beginning of this thread you wanted to
 blindly castrate constructors :)

I explained the issue and proposed solutions.  One of them involved not
creating component hierarchies from the constructors.  I wouldn't call
that
castrating: You still get to use them for what they're supposed to do:
initialize your instance's state.

 i have proposed a way to deal with
 the issue. you can create your own one-time callback in the page from
 inside onconfigure().

So to round up, the proposed solutions are:

1. Forbid add() in constructor.
2. Forbid onInitialize() in pages (and fake one with onConfigure code
that
really has nothing to do with configuring but is onInitialize code in a
block that requires a test of an instance field that takes up a whole
boolean field in your page state, and do that for each of your pages).
3. Don't initialize() on add() in pages.

 * -1 to forbid add() from constructors
 * +0 to delay oninitialize() cascade until right before the first
call
 to onconfigure()


So that's [1] and [3] out the window, does that mean you vote +1 for
[2]?
Because as I mentioned before, you can't in all honesty argue the case
of
leaving the door open for add() in constructors if the developer want
to do
that and slam the door shut for add() in onInitialize() if the
developer
wants to do that.

Perhaps we can add a [4]:

4. Allow both and ignore the fact that mixing them will break things in
pages.  Heck, if we can allow doing potentially bad things in
constructors
because the bad is rare anyway, why not, right?

My vote, in order of preference, would be [1], [3], [4], [2].
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 as JSF's faces-config.xml and 
all that good stuff.

But sure, you could implement [1] and allow the developer to turn off the RTE 
from an Application setting.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 
fireComponentInitializationListeners, but why can't that just happen later, 
before the first render?
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 some
components might need to know their parents.

I thought I was keeping pretty much up-to-speed on 1.5 changes, but I'm kind
of surprised this is the first I'm noticing this fairly substantial API
change.

Sorry if something like this has already been shot down - I didn't look -
but just off the top of my head, seems like there could be a way to have the
framework (as it is about to add a page to it's map or at some other
convenient early point) go back down thru the hierarchy and visit all the
components to offer them a parent reference?   Maybe guarantee it would
happen before the first onBeforeRender or something like that?

Then, we could let these visitors 'vote' on when the hierarchy is finished
changing -  (i.e. components could use this information to make additional
changes to the hierarchy) by returning a value.   Perhaps call this new
component visitor something like int onParentChanged(Component parent) -
return flags that would let the framework interrupt and restart, repeat
after finish, or just continue visiting (or maybe just return boolean repeat
flag).

-- Jim Pinkham
TogetherAuction.com

On Tue, Mar 8, 2011 at 8:33 AM, Zoltán Nagy zberke...@gmail.com wrote:

 Hi!

 My idea is this: Somewhere in the future (for example in wicket 1.6)
 adding copmonents from the constructor will be prohibited, but till
 than it should be allowed.

 --
 Zoltán Nagy

 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org




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 the API can be improved adding one more method like the awt
Component#addNotify, but with one change: this method needs to be called
again if the component is remove / re added and it will not be a good place
to add children.

- 1 for solution 2, it is up to user decide how to code objects, the related
problems in this thread can be addressed in different way, like I voted
before.

On Tue, Mar 8, 2011 at 8:11 AM, Maarten Billemont lhun...@gmail.com wrote:

 Dear fellow wicket users,

 Following is a call for your opinion and a vote on how Wicket should behave
 with regards to post-constructing components in 1.5.  The final solution
 that is chosen will affect the way you construct your components and your
 component hierarchies.



 First, some background on what onInitialize is and why you should be using
 it.

 THE PROBLEM

 Historically, we've generally used our component constructors to build our
 component's hierarchy.  We call add() to put new components in our custom
 component from our component's constructor:

 public class MyPanel extends Panel {

private final IModelString name = Model.of();

public MyPanel(String id) {

add(new TextFieldString(name, name));
}
 }

 This can cause problems, however, when initializing a component requires
 knowledge of the component's markup, or requires that the component's path
 in the hierarchy is known (during construction, the component is not yet
 added to any parent component).


 THE SOLUTION

 onInitialize was introduced to Component.  This gives us a place to
 initialize our component's hierarchy after our component has been added to
 its parent.  As a result, our component has a path to its page, making
 markup, parent and sibling components accessible.


 THE BAD

 The downside of initializing your component hierarchy in onInitialize as
 opposed to from the constructor, are:

 1. You need to move all your component hierarchy initialization code into a
 separate method.  Usually, this is trivial work (cut/paste), but it is work
 that needs to be done nonetheless.
 2. You cannot put your components in final instance fields anymore.
 3. You should never do anything with components from custom methods.  You
 should only ever reference your components directly from overridden wicket
 methods, such as onConfigure, onBeforeRender, onSubmit, etc.

 A little more on #3:

 If you add custom methods to your component, eg.

 void makeRequired() {
nameField.setRequired(true);
 }

 You allow things like: new MyPanel().makeRequired();
 That would throw a NullPointerException, since nameField is no longer
 initialized in the constructor, but much later, before the component first
 gets rendered.

 I would argue, though, that any custom methods that touch components are to
 be avoided at all cost.  Component behaviors should never be touched except
 from wicket lifecycle methods (you probably want onConfigure) or right after
 you construct it:  add(new TextFieldString(name,
 name).setRequired(true));  If you need a custom method such as makeRequired,
 it shouldn't touch components, it should set state that gets used by
 onConfigure:

 void makeRequired() {

this.required = true;
 }

 void onConfigure() {

setRequired(required);
 }

 (Obviously, the example is rather silly..)



 Fast-forward to https://issues.apache.org/jira/browse/WICKET-3218, it
 seems onInitialize has a problem.

 THE PROBLEM

 With regard to pages, if you add a component to a page from the
 constructor, onInitialize is invoked immediately.  As a result, onInitialize
 code is ran even though your instance hasn't been fully constructed yet.
  This is bad.

 The real issue here is that we can combine component hierarchy
 initialization with constructors and onInitialize in a single class.
  However, doing so causes dangerous situations.  We really should do only
 one, not both at the same time.


 THE SOLUTIONS

 This is where we need your vote.  Currently, two proposed solutions exist:

 1. Make onInitialize final for all Pages.  As a result, onInitialize will
 still get called when the page is constructed and components are added, but
 you won't be able to use it, avoiding the risk of you using it in a manner
 that requires your instance to be constructed.

 2. Make adding components from the constructor illegal.  This would have to
 be by means of a runtime exception thrown whenever you try to add components
 to the hierarchy from the constructor.


 THE GOOD

 1. This will be least painful for existing code.  In all components, you
 can still mix constructor an onInitialize use, in pages, you can't use
 onInitialize, but in all likelihood, the largest amount of existing 

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 who don't want to follow that explanation do also not have to be
protected agains themselves)

-1 for throwing an exception if add(..) is called from within a constructor
(but maybe logging a warning instead?)

I would also not have a problem with an alternative onInitialize()-method
for the Pages.


-
-- 
Regards, 
Hans 

http://cantaa.de 

--
View this message in context: 
http://apache-wicket.1842946.n4.nabble.com/VOTE-WICKET-3218-Component-onInitialize-is-broken-for-Pages-tp3341090p3341741.html
Sent from the Users forum mailing list archive at Nabble.com.

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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, which is why I asked for the reason as to why 
initialize() is invoked as a result of components being added to a page.  If it 
ends up not being necessary (delay the onInitialize to when it's called for any 
other component) then perhaps that's a fair middle ground.  People can still 
use constructors if they like (though really, the only single *advantage* I can 
think of for this is that it means you don't have to go back and fix your 
existing codebase), and others are free to use onInitialize.

Doing your work post-construct is a very common thing in Java.  That's the 
whole point of there being a javax.annotation.PostConstruct, for example.  
Anyone that claims doing this is bad or counter-intuitive probably hasn't seen 
enough Java code yet.

We're stuck with a legacy of how we used to do things in Wicket.  And yes, that 
is a burden we should consider.  But at some point, in favor of a clean API and 
consistency, you need to be able to give up legacy.  Perhaps 1.5 is that time, 
perhaps it should be 1.6, but kid yourself not: it will be all that much harder 
the longer we wait.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 exception if add(..) is called from within a constructor
 (but maybe logging a warning instead?)

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 exception in 
add().

If such a solution is not suggested in this thread, we're gonna need either a 
final onInitialize or an exception in add().  You can't have your cake and eat 
it too.
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 for not  fully constructed
  components go away.

Fun fact: There is already a patch for that...

 I actually also thought of that, which is why I asked for the reason
 as to why initialize() is invoked as a result of components being
 added to a page.  If it ends up not being necessary (delay the
 onInitialize to when it's called for any other component) then
 perhaps that's a fair middle ground.  People can still use
 constructors if they like (though really, the only single *advantage*
 I can think of for this is that it means you don't have to go back
 and fix your existing codebase), and others are free to use
 onInitialize.

I would like to use onInitialize for things that *need* to be done
outside of the constructor.

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 doesn't render constructors obsolete.

Second, if you need to use any kind of parameters for stuff you do in
onInitialize, you *must* store them in instance variables, even if you
need them just once and could have long let them be garbage collected.
Going purely post-construct would be a very bad idea for this reason.

Carl-Eric
www.wicketbuch.de

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 exception in add().
 
 If such a solution is not suggested in this thread, we're gonna need
 either a final onInitialize or an exception in add().  You can't have
 your cake and eat it too.

I think there are good arguments for both constructors and
onInitialize. But the developer needs to know what he/she is doing. I
think that both options are useful and valid. This is certainly not a
case of having the cake and eating it too.

Constructor-only makes some initializations more difficult (though of
course you can still use onBeforeRender or onConfigure, as we do now,
after my patch had been rejected originally).

onInitialize-only increases memory footprint and introduces
conceptually unnecessary state by forcing you to carry *every*
parameter via an instance field.

Having both lets the developer choose the most appropriate strategy for
the component at hand.

Carl-Eric
www.wicketbuch.de

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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

2011-03-08 Thread GOODWIN, MATTHEW (ATTCORP)
+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 out
there show the add(..) from within the constructor - which is apparently
an anti-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 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 who don't want to follow that explanation do also not have
to be
protected agains themselves)

-1 for throwing an exception if add(..) is called from within a
constructor
(but maybe logging a warning instead?)

I would also not have a problem with an alternative
onInitialize()-method
for the Pages.


-
-- 
Regards, 
Hans 

http://cantaa.de 

--
View this message in context:
http://apache-wicket.1842946.n4.nabble.com/VOTE-WICKET-3218-Component-on
Initialize-is-broken-for-Pages-tp3341090p3341741.html
Sent from the Users forum mailing list archive at Nabble.com.

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org


-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 doesn't render constructors obsolete.

Your constructors are the perfect place to initialize your instance, at least 
at the level of your constructor's type.  Build all your instance variables 
there, handle your page parameters, and that stuff.

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 properly initialized and 
ready for use.  You really shouldn't be doing anything in your constructor that 
is NOT related to initializing your constructor type's instance variables.  
Touch anything else, invoke any methods, and you are opening pandora's box.

It's important to understand *what* constructors are a natural place for.

 Second, if you need to use any kind of parameters for stuff you do in
 onInitialize, you *must* store them in instance variables, even if you
 need them just once and could have long let them be garbage collected.
 Going purely post-construct would be a very bad idea for this reason.

Sorry?  Parameters?  You mean arguments to your components' constructor?  Of 
course, but you really shouldn't ever need these in onInitialize if they're not 
state.  And yes, your component's state belongs in its instance variables.  If 
this is an issue, could you enlighten me?
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 out
 there show the add(..) from within the constructor - which is apparently
 an anti-pattern from the sound of this thread.

It is most certainly not an antipattern in my book. I find the
reaction of the anti-constructor folks too strong and am trying to
formulate a civil reaction to this whole anti constructor rant.

Martijn

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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, they have limitations, and yes, PostConstruct
 exists for a reason, but that doesn't render constructors obsolete.

 Your constructors are the perfect place to initialize your instance, at least 
 at the level of your constructor's type.  Build all your instance variables 
 there, handle your page parameters, and that stuff.

 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 properly 
 initialized and ready for use.

not really sure what you mean by subclass constructors or how they
come into play when constructing an instance...

  You really shouldn't be doing anything in your constructor that is NOT 
 related to initializing your constructor type's instance variables.  Touch 
 anything else, invoke any methods, and you are opening pandora's box.
 It's important to understand *what* constructors are a natural place for.

i think code like this should be possible:

NameEditor e=new NameEditor(name, firstNameModel, lastNameModel);
e.getFirstNameEditor().setRequired(true);
e.getLastNameEditor().setEnabled(false);

its simply the good citizen pattern where once you have an instance
that instance is fully constructed and is ready to use. i think this
applies to most components. i think there are much fewer components
that need a page or their environment to work, and most of them are
application-specific. wicket is all about reusability, and we should
not hinder how easy and convenient it is to create and use reusable
components.

 Second, if you need to use any kind of parameters for stuff you do in
 onInitialize, you *must* store them in instance variables, even if you
 need them just once and could have long let them be garbage collected.
 Going purely post-construct would be a very bad idea for this reason.

 Sorry?  Parameters?  You mean arguments to your components' constructor?  Of 
 course, but you really shouldn't ever need these in onInitialize if they're 
 not state.  And yes, your component's state belongs in its instance 
 variables.  If this is an issue, could you enlighten me?

taking the previous example of a name editor, with constructor we have:

class nameeditor extends panel {
  public nameeditor(...) {
 add(new textfield(first,..);
 add(new textifled(last, ...);
  }
  public textfield getfirst() { return get(first); }
  public textfield getlast() { return get(last); }
}

without constructing components i would have to do this:

class nameeditor extends panel {
  private boolean firstNameRequired, lastNameRequired;
  private boolean firstNameEnabled, lastNameEnabled;

  protected void oninitialize() {
 add(new textfield(first,..);
 add(new textifled(last, ...);
  }

  protected void onconfigure() {
 first.setrequired(firstNameRequired).setEnabled(firstNameEnabled);
 last.setrequired(lastNameRequired).setEnabled(lastNameEnabled);
  }

  public void setFirstNameRequired(value) { firstNameRequired=value; }
  public void setLastNameRequired(value) { lastNameRequired=value; }
  public void setFirstNameEnabled(value) { firstNameEnabled=value; }
  public void setLastNameEnabled(value) { lastNameEnabled=value; }
}

notice the verbosity, and four extra and redundant slots that this
component now takes up in memory and when serialized...

-igor


 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org



-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org



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 properly 
 initialized and ready for use.
 not really sure what you mean by subclass constructors or how they
 come into play when constructing an instance...
 If I understand correctly, here is an example of what he means :


Exactly.  If B extends A and I do something in A's constructor that goes beyond 
the scope of setting up A's instance variables, then I risk a side-effect that 
relies on the instance to be constructed at B's level as well.  There's nothing 
in the JVM that prevents this and these bugs are very hard to see and debug.  
They should be avoided by good coding practices.

On 08 Mar 2011, at 22:07, Igor Vaynberg wrote:
 
 i think code like this should be possible:
 
 NameEditor e=new NameEditor(name, firstNameModel, lastNameModel);
 e.getFirstNameEditor().setRequired(true);
 e.getLastNameEditor().setEnabled(false);

 taking the previous example of a name editor, with constructor we have:
 
 class nameeditor extends panel {
  public nameeditor(...) {
 add(new textfield(first,..);
 add(new textifled(last, ...);
  }
  public textfield getfirst() { return get(first); }
  public textfield getlast() { return get(last); }
 }

Firstly, it's my opinion that you really shouldn't be doing anything to 
components directly, especially not from outside your class.  As for why, see 
Encapsulation and Law of Demeter.

But if you really wanted to use this pattern, it really wouldn't have to be as 
cumbersome as you make it out to be.  Also, I really don't like condoning 
get(String), it's nasty and very type-unfriendly.  It also breaks as soon as 
you do any refactoring in your component hierarchy.

class NameEditor extends Panel {
TextFieldString firstField;
TextFieldString lastField;
@Override protected void onInitialize() { super.onInitialize(); 
add(getFirst(), getLast()); }
public textfield getFirst() { if (firstField == null) firstField = new 
TextFieldString(); return firstField; }
public textfield getLast() { if (lastField == null) lastField = new 
TextFieldString(); return lastField; }
}
-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org