[
https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12987077#action_12987077
]
Szádeczky-Kardoss Szabolcs commented on WICKET-3218:
----------------------------------------------------
Hi Igor and Carl,
I think this issue is really a misunderstanding of the onInitialize concept, as
it was a really needed feature. This was the reason I switched to 1.5-M3, and
then BANG! in RC1 this is broken. Let me explain how to use it in the right
way, and then my use case for which this is much needed.
USAGE
As you have pointed out, onInitialize gets called at the first time when a
component gets added to the Page, or - if this doesn't happen in the
constructor then - in the render phase (internalPrepareForRender method) at
latest. Well, if you add any component to the page in the constructor then
onInitialize is really not of much use, actually in this case it's better not
to use it all. However if you start making pages as I do, then it becomes a joy
to work with.
The solution is simple: Don't add any component to the Page in the constructor,
use onInitialize for that purpose. If you advocate this as a best practice then
the use cases stated below will be much easier to make than without the
onInitialize (and using only constructor).
The constructor is anyway best suited only for setting up models, performing
(some or all of the) service calls and other things needed to ensure that the
given Page is the one to show to the user, without going into costly component
additions to the Page.
USE CASES
- In my current project we have a common base page from which all other pages
subclasses and so we share a common layout, some common panels and common
functionality in all pages. However once in a while it might be needed to hide
or replace one of the common panels in only one page but leave it as common in
all of the others. If you only could do this in the constructor then that will
be a really pain. The reason in short is that irrespective of whether you use
overriden "panel adder methods" or any other solution you are still in object
construction phase and that puts quite a few restrictions on variable
initialization order. I know, I did it, and there are only hacking workarounds
for this. On the other hand, onInitialize is a super elegant way to use
overriden methods or any cool OO technique since you are not constrained any
more by the "Construction phase".
- An other use case is that the user is doing stuff on any page, it can be
anything. Irrespective of what he/she is doing, something is happening in the
background, perhaps by an other user's action. The next time the first user is
refreshing this page or going to another page, I want the user to be notified,
in a common way, in a common code. This can be also achieved without
onInitialize with more or less hacking (especially when we want the user
notified when staying on the same page), but with onInitialize this is a much
cleaner. The reason is that prepareForRender (which by the way also got final,
why?) precedes onInitialize and so it is possible to do this check there.
FINAL/NON-FINAL :) THOUGHTS
Sorry for getting so long, my only point is that onInitialize (and
prepareForRender) not being final was one of the great achievements of 1.5
(backported into 1.4, or was it the other way around?). There were really use
cases for this, especially the avoidance of the Java Object Construction Phase
limitations (and the possibility of not having ugly big constructors :)). Sure
this can be misused or used wrongly as a lot of other things in Wicket, but
this is not a reason to limit the good usage of this. You can put it in the
javadoc, that only use onInitialize when you don't do add operations in the
constructor. Actually I would also ask what's the point of onInitialize
anywhere else than for Pages? At least I could live without it anywhere else,
but not for Pages.
I hope you will change both methods back to non-final. If not, then I will have
to revert to 1.4 and possibly never use 1.5, which would be a sad thing.
> Component#onInitialize is broken for Pages
> ------------------------------------------
>
> Key: WICKET-3218
> URL: https://issues.apache.org/jira/browse/WICKET-3218
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
> Assignee: Igor Vaynberg
> Fix For: 1.5-RC1
>
> Attachments:
> 0001-delay-oninitialize-until-just-before-onconfigure.patch
>
>
> As first mentioned at
> http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%[email protected]%3E
> , I think the current (Wicket 1.4.14) implementation of
> Component#onInitialize is broken for Pages. Pages get initialized as soon as
> the first component is added, which is correct. But this usually happens
> within the constructor of the page, which means that the page object isn't
> fully initialized yet. The entire point of having onInitialize, however, is
> to be able to do further work once all constructors have run. See
> https://github.com/duesenklipper/wicket-oninitialize for a quickstart that
> demonstrates the problem.
> Pedro Santos suggested in the above thread to just switch the entire object
> construction to onInitialize. I don't think this is a good idea, because
> 1) it is completely counter-intuitive
> 2) it is not always realistic to have an entire class hierarchy not using the
> constructor just because a subclass somewhere might want to use onInitialize
> 3) it is inconsistent with onInitialize behavior for all other (non-Page)
> components. Here I can easily mix work in the constructor with onInitialize.
> I propose the following patch:
> - override onInitialize in Page and make it final, so Pages can't use this
> any more. This should not cause any unnecessary breaking, since currently
> it's not working for pages anyway.
> - introduce Page#onPageInitialize to provide a safe alternative to
> onInitialize
> - make a special case for Page in Component's beforeRender to fire
> Page#onPageInitialize if necessary
> Yes, this is a bit of special casing for Page, but there's quite a lot of
> that needed for Page anyway. I think the impact of this should be minimal.
> My page includes documentation and a new testcase that verifies the new
> behavior. I modified the old ComponentInitializationTest to reflect the fact
> that Page doesn't get onInitialize any more.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
