> > I believe that with the minor change that I have made everything is
> > still being done in the exact same order so there should be no
> > consequences of this change. It's just that all the stuff done in
> > commonInit is now being slightly deferred so that it uses completely
> > constructed objects instead of partially constructed objects - that
> > can't be a bad thing can it?
> 
> introducing a required post-construct init method is hardly a minor
> change.
> while this works well for you because you mostly use bookmarkable
pages it
> wont hold for normal apps where bookmarkable pages are a minority.

You can easily make it a switchable option in the application settings
and default to the traditional 'possibly get weird crash because you're
working on a partially constructed object' mode then everyone's existing
code will continue to work as is.

Other people, who like their objects fully constructed before using them
so that they can successfully use polymorphism in their page classes
during initialization, can switch to 'two phase initialization' mode.

> and there are in fact consequences to your change. i can no longer do
> getBodyContainer().addOnLoadModifier(...) from the constructor of my
page.
> so where am i to do it now? i need to use that post-construct
callback, so
> now there are two places where i need to initialize my component -
hardly
> simple still.

Easy if you deal with the parameters in the constructor and deal with
adding children, accessing markup etc., in the overridden init (or
commonInit) method. 

In cases where people have a number of constructors they normally create
their own separate init method anyway to avoid code duplication so it's
not that bad.

For me I'd be happy to have getVariation working properly right now but
I'm just trying to do something here for the greater good of the wicket
community and the children of the future that come to wicket
(...children are our future, teach them right and let them know the
way... but I digress) Seriously thought: there's going to be other cases
of partially constructed objects causing problems as more and more
people use wicket. Better to fix it properly now than wait until it's
too late to fix it.

> 
> Source changes: I've taken a more inclusive, simpler approach that
> > changes RequestCycle and BookmarkablePageRequestTarget instead of
> > changing the DefaultPageFactory. This means absolutely no explicit
> > calling of the commonInit method by users. I had to change 3 pages
in
> > the test cases so that they do their init in commonInit instead of
the
> > constructor and all test cases now succeed.
> 
> 
> have you also changed all other page-related request targets as well?
have
> you built safeguards to make sure this method is atomic?

Every unit test now runs successfully so if those scenarios are covered
by the wicket test suite then I've covered them with my changes.

> the fact that you had to call this method manually in tests is proof
> enough for me that what you propose does not work well.

I never had to call it manually anywhere. In a few of the tests I just
overrode commonInit and did the initialization in there after calling
super.commonInit(). There's even a technique to avoid needing to do that
call also:

WebPage.commonInit()
{
        // do all the regular commonInit stuff
        ...

        // then call virtual init() - users override init() instead of
        // commonInit and they don't have to call the super class method
        // to ensure that the commonInit code gets executed - it already
        // has been executed. If their own base classes do stuff in init
        // then they can call super.init() if they desire but the
        // WebPage.init() is just an empty method and does nothing.
        init();
}

> yes, we are all aware of the fact that it is not a good idea to call
> overloaded methods from the parent's constructor. like i said, the
problem
> is that getvariation() is called as a side effect of something done in
the
> commoninit method - and that is what needs to be fixed. there shouldnt
be
> a
> reason why we need to call getvariation()/load markup at construction
> time.

No one ever deliberately calls overloaded methods from parent
constructors. It always happens as an accidental side effect and will
happen in the future again I'm sure - whether in using a framework
feature or in a user's own WebPage derived classes.

> 
> Other frameworks (eg,. Echo2) handle this partial construction problem
> > by automatically calling an init method on the class whenever you
add it
> > to the system after instantiation. The user doesn't need to (and
> > shouldn't) explicitly call init() as it's done for them. They can
even
> > override init() if they want to but must first call super.init()
prior
> > to doing any of their own initialization work.
> 
> 
> yes, but in wicket we prefer constructors. they are in java for a
reason
> :)

I prefer them too - but only up to the point where they stop you from
using certain OO features during initialization.

> i dont know about echo2 but in wicket components are not beans.

Neither are they in Echo2. In Echo2 you add all your child components
and do all the dancing you want in the init() method that is
automagically called by the framework and everything works - even
overloaded methods during initialization.

> In fact this is the exact pattern of initialization that you have
> > implemented for wicket's WebApplication. It seems to make sense to
me
> > that the same pattern would apply to WebPage.
> 
> webapplication is a "managed" class, so there we didnt really have a
> choice.
> webapplication also does not implement the compound pattern where such
a
> call has to be cascaded. in short, webapplication and webpage (which
is a
> component) are very different.

The compound pattern is traditionally an excellent candidate for a two
phase initialization mechanism. Echo2, MFC, OWL (and many others) - all
implement the compound pattern and they all have a two phase
initialization mechanism: "Sort yourself out first in your constructor
then add your children, etc., in your init method".

It's a bit like the preflight emergency procedure recordings on a plane:
"get your own oxygen mask fitted to your own face (constructor) then
sort out your children's oxygen masks (init). You're no use to anyone
else until you're in a stable state."


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Wicket-user mailing list
Wicket-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-user

Reply via email to