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