You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Carl-Eric Menzel <cm...@wicketbuch.de> on 2010/12/17 21:41:54 UTC

Re: [jira] Resolved: (WICKET-3218) Component#onInitialize is broken for Pages

On Fri, 17 Dec 2010 11:05:01 -0500 (EST)
"Igor Vaynberg (JIRA)" <ji...@apache.org> wrote:

> i made the decision to make the method final in page. i think this
> makes more sense then delaying the call because by the time the
> page's constructor is finished the page should be initialized, eg
> code like this is commong
> 
> EditUserPage page=new EditUserPage(user);
> page.switchToAddressEditMode();
> 
> switchToAddressEditMode() may depend on certain components being
> added to the hierarchy and if they are added in oninitialize() which
> is delayed the method may fail.
> 
> thus, the principle of least surprise dictates we make it final in
> page.

Well if your superclass is doing things you don't know about then you
are usually screwed anyway. Is this case really that common? Making it
final at least makes it obvious that onInitialize doesn't work, however:

It leaves a question open: What is the "official" approach
that replaces onInitialize for a page? What if I want to provide an
extension point for sub-pages but still need to give them a chance to
have their own constructor run first?

Typical example with panel-based layout in a super page:

class AbstractLayoutPage {
  public AbstractLayoutPage() {
     add(getLeftColumn());
     add(getRightColumn());
  }
  
  protected Component getLeftColumn() {...}
  protected Component getRightColumn() {...}
}

class SomeConcretePage {
   private final Object someImportantObject;
   public SomeConcretePage(Object o) {
     this.someImportantObject = o;
   }

   @Override protected Component getLeftColumn() {
     return new CustomLeftColumn(this.someImportantObject);
     // fails because someImportantObject isn't initialized.
   }
}

With a delayed onInitialize (or my original proposal for a separate
onPageInitialize) I could do this in onInitialize. Right now I have
no way of doing this safely. How can I have an initialization in page
after the constructors are done? Without some sort of change to
onInitialize or a new event entirely I only see doing it in
onBeforeRender and manually keeping track of whether it's been done
already or not.

Personally I have seen the "overridable Panel" approach much more often
than hoping without looking that the superclass does its construction in
a particular order.

I'm not saying that my patch is the perfect solution - but I'd really
like an "official" hint at how to do something after the page
constructors.

Thanks
Carl-Eric
www.wicketbuch.de

Re: [jira] Resolved: (WICKET-3218) Component#onInitialize is broken for Pages

Posted by Igor Vaynberg <ig...@gmail.com>.
On Fri, Dec 17, 2010 at 10:41 PM, Carl-Eric Menzel
<cm...@wicketbuch.de> wrote:
> On Fri, 17 Dec 2010 11:05:01 -0500 (EST)
> "Igor Vaynberg (JIRA)" <ji...@apache.org> wrote:
>
>> i made the decision to make the method final in page. i think this
>> makes more sense then delaying the call because by the time the
>> page's constructor is finished the page should be initialized, eg
>> code like this is commong
>>
>> EditUserPage page=new EditUserPage(user);
>> page.switchToAddressEditMode();
>>
>> switchToAddressEditMode() may depend on certain components being
>> added to the hierarchy and if they are added in oninitialize() which
>> is delayed the method may fail.
>>
>> thus, the principle of least surprise dictates we make it final in
>> page.
>
> Well if your superclass is doing things you don't know about then you
> are usually screwed anyway. Is this case really that common? Making it
> final at least makes it obvious that onInitialize doesn't work, however:

its not only for superclass, its also for the same class

class mypage extends page {
  private Component container=null;

  mypage() {
     add(new label("foo"));
     continer=new webmarkupcontainer(...);
  }

  oninitialize() {
     container.add(getleftpanel("left"));
  }
}

here oninitialize() gets called as soon as the first add() in the
page's constructor and will result in an NPE because container is not
yet initialized.

> It leaves a question open: What is the "official" approach
> that replaces onInitialize for a page? What if I want to provide an
> extension point for sub-pages but still need to give them a chance to
> have their own constructor run first?

the official approach for this is the same as it has been without oninitialize()

class mypage extends page {
   onbeforerender() {
       if (get("left")==null) {
           add(getleftcolumn("leff"));
        }
    }
}

-igor


>
> Typical example with panel-based layout in a super page:
>
> class AbstractLayoutPage {
>  public AbstractLayoutPage() {
>     add(getLeftColumn());
>     add(getRightColumn());
>  }
>
>  protected Component getLeftColumn() {...}
>  protected Component getRightColumn() {...}
> }
>
> class SomeConcretePage {
>   private final Object someImportantObject;
>   public SomeConcretePage(Object o) {
>     this.someImportantObject = o;
>   }
>
>   @Override protected Component getLeftColumn() {
>     return new CustomLeftColumn(this.someImportantObject);
>     // fails because someImportantObject isn't initialized.
>   }
> }
>
> With a delayed onInitialize (or my original proposal for a separate
> onPageInitialize) I could do this in onInitialize. Right now I have
> no way of doing this safely. How can I have an initialization in page
> after the constructors are done? Without some sort of change to
> onInitialize or a new event entirely I only see doing it in
> onBeforeRender and manually keeping track of whether it's been done
> already or not.
>
> Personally I have seen the "overridable Panel" approach much more often
> than hoping without looking that the superclass does its construction in
> a particular order.
>
> I'm not saying that my patch is the perfect solution - but I'd really
> like an "official" hint at how to do something after the page
> constructors.
>
> Thanks
> Carl-Eric
> www.wicketbuch.de
>