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

it has to do with the fact that if i use a constructor and add those
components then i do not need the references. i can always get them
via get().

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

we are working on enabling this. if NameEditor is a panel or a border
then its children should be able to access its markup before
NameEditor instance is added to the page since its markup is loaded
independently of its parents.

-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

Reply via email to