On 09 Mar 2011, at 18:56, Igor Vaynberg wrote:
>
> On Wed, Mar 9, 2011 at 1:15 AM, Maarten Billemont <[email protected]> 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 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.
You're making it sound a little black and white. You don't need unlimited
memory and infinite bandwidth to support holding state and/or components in
instance variables instead of casting results of get() operations on fragile
strings. I think the grey zone is quite large, and while the exceptional case
where you might want to optimize the maintainability out of your code would be
nice to support, it really shouldn't be condoned as the standard way of doing
things.
>
>>> *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(); }}
>
> }
I would if it weren't a "special cases where I'll have to forgive myself the
bad design". And I think you're implying I could do this per default in all my
pages.
>
>>> *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.
In the ideal world, putting your component initialization code in your
constructors has no issues or side-effects. You'd have access to your markup
and your parent, you'd have a fully initialized object, and you wouldn't have
to make the mental leap that post-constructing actually makes sense.
Unfortunately, it seems we can't have that. Using constructors for component
initialization does have issues:
1. You don't know your markup.
2. You don't know your parent.
3. Your subclasses haven't initialized yet.
And while it obviously also has advantages to be able to use the constructor,
it doesn't make sense to blindly argue the advantages without proposing a way
of dealing with the issues.
I've proposed a way of dealing with the issues: Use onInitialize always. This
also has issues, and I've proposed solutions for all of them. You'll find this
thread documents them well, and in that regard, the onInitialize side of the
argument has actually been worked out better. The disadvantages are clear and
the way you should design your code to cope with them have been explained fully.
You say you're working on a way of dealing with issue [1] for constructor-based
component initialization. That'd be grand. But that leaves [2] and [3]. I
can only think of one way to resolve [2]: In addition to taking an `String id`
in components' super calls, also take a `MarkupContainer parent`. But that
would make all component constructors even more verbose. And it still leaves
[3] unresolved.
That's ignoring the troubles you get when you use onInitialize and constructors
in your Page. However, that can be documented in the JavaDocs, if need be:
<b>If you add components in your constructor, don't use onInitialize</b>.
I see the only recurring case for constructor-based initialization being: We
shouldn't prohibit those that want to do it that way from doing it that way.
What about the fact that you're prohibiting me from using onInitialize in pages?
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
