Vote: Remove onAttach()

[Matej Knopp <ma...@gmail.com>]

Hi all,

I think the onAttach method/concept has serious flaws at the moment.
It is not called for components inside repeaters on the first render,
thus it's quite unreliable. And I don't really see an easy way of
fixing this. Also we should discourage users to move initialization
code to onAttach(). Rather then that, they should initialize
everything lazily, such as LoadableDetachableModel does. Not to
mention that internalAttach2 traverses the entire components 3 times
for no good reason, as the situation when anyone overrides onAttach is
very rare.

Given this, I propose removal of the onAttach method, but under the
following conditions:

* Page.onAttach() would still be called, so that it would be possible
to get the notification right before page is accessed (pulled out of
session/sessionstore). Also when user needs to notify components, this
would be a way to go (putting a visitor inside).

* We could rename Page.onAttach to Page.onPageAttached() and make
onAttach final with explainatory javadoc, so that users would know why
they can't override the method anymore.

* We would provide convenience methods markAttach and isAttached in
case users decide to implement the onAttach methods.

As for vote:

[   ] Remove onAttach()

[   ] Keep onAttach(), because...

Thanks.
-Matej

Re: Vote: Remove onAttach()

[Igor Vaynberg <ig...@gmail.com>]

> [X ] Remove onAttach()

-Igor

Re: Vote: Remove onAttach()

[Matej Knopp <ma...@gmail.com>]

On 10/8/07, Matej Knopp <ma...@gmail.com> wrote:
> Hi all,
>
> I think the onAttach method/concept has serious flaws at the moment.
> It is not called for components inside repeaters on the first render,
> thus it's quite unreliable. And I don't really see an easy way of
> fixing this. Also we should discourage users to move initialization
> code to onAttach(). Rather then that, they should initialize
> everything lazily, such as LoadableDetachableModel does. Not to
> mention that internalAttach2 traverses the entire components 3 times
> for no good reason, as the situation when anyone overrides onAttach is
> very rare.
>
> Given this, I propose removal of the onAttach method, but under the
> following conditions:
>
> * Page.onAttach() would still be called, so that it would be possible
> to get the notification right before page is accessed (pulled out of
> session/sessionstore). Also when user needs to notify components, this
> would be a way to go (putting a visitor inside).
>
> * We could rename Page.onAttach to Page.onPageAttached() and make
> onAttach final with explainatory javadoc, so that users would know why
> they can't override the method anymore.
>
> * We would provide convenience methods markAttach and isAttached in
> case users decide to implement the onAttach methods.
>
> As for vote:
>
> [ X ] Remove onAttach()
>
> [    ] Keep onAttach(), because...
>
> Thanks.
> -Matej
>