Igor Vaynberg wrote:
On 1/4/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
sounds pretty good to me. the main other question to ask is how
much code this would break and how easy that would be to detect.
and of course how to educate users about it. any thoughts on those
things?
i dont know how much code it would break. depends on how many people
overrode onattach() in their subclasses, how much of that is for resources
and how much of that is for component hierarchy.
we can detect modifications to component hierarchy and throw the exception
just like what we do in onbeforerender() right now. our users can easily
pull up any overrides they have in their own code, eclipse makes this
trivial and i would imagine so do other ides.
as far as educating users, we have a migration wiki page, that should be
enough.
but now that i think about it, i think i would be ok with removing
onattach() altogether. the reason is that it can be quite wasteful. when
invoking a simple listener, like link's onclick we have to attach the
entire
page, and i think most of that will go to waste. i think a lazy-load
pattern, just like the one we use in models works much better here and
maybe
we should force it onto our users. i would like to see if anyone can
come up
with viable usecases for the onattach() within this new definition.
I wouldn't be against removing onAttach either. If we can change
component hierarchy in onBeforeRender. I've seen a lot of code when
people were doing unnecessary initialization in onAttach instead of lazy
loading through model. On the other hand, this would probably just lead
to moving the code from onAttach to onBeforeRender :)
Anyway, I don't see much sense of having onAttach after this refactor.
Perhaps we should have vote on removing it?
-Matej
-igor
igor.vaynberg wrote:
>
> yeah, you got it
>
> attach means you attach any resources you might need. a better way to
> think
> about it is the opposite of detach() in the model - everyone is
familiar
> with that one. i honestly do not see many use cases for onattach, most
of
> the time i use the lazy loading pattern so all i really need is
> ondetach().
> but i guess it is there for symmetry.
>
> and you are spot on about onbeforerender.
>
> -igor
>
>
> On 1/4/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
>>
>>
>>
>> actually, it's onBeforeRender. this is a hint for sure to users that
>> you ought to be able to muck with the hierarchy in this method.
>>
>> can you state the proposed semantics in terms of the meanings
>> of the method names? the code movement is not as important
>> as transparency of meaning for users. in other words, what should
>> you put in onAttach and why? and onBeforeRender and why? the
>> answers should be intuitive with respect to the names. in the long
>> run this is more important. my intuition about a method called
>> onBeforeRender is that this would be a place to set up for rendering,
>> including changing the component hierarchy. a method called onAttach
>> would be a place to handle any sort of model stuff dealing with
>> attachment
>> of the page to all its models.
>>
>>
>> igor.vaynberg wrote:
>> >
>> > ive been discussing with johan offline, and here is what we came up
>> with
>> >
>> > there is still a big onattach refactor to do in 2.0. big meaning it
>> will
>> > affect a lot of users.
>> >
>> > the contract for onattach/ondetach was that these methods are called
>> > before
>> > the framework executes any method on the component, but right now
>> onattach
>> > is pretty much onbeginrender with the exception that you are allowed
to
>> > modify the component hieararchy.
>> >
>> > take a simple case of clicking link.
>> >
>> > the story should be:
>> >
>> > currently it is
>> >
>> > 1) resolve link component
>> > 2) invoke onclick
>> > 3) invoke onattach
>> > 4) invoke onbeforerender
>> > 5) invoke render
>> > 6) invoke onafterrender
>> > 7) invoke ondetach
>> >
>> > but should be
>> >
>> > 1) resolve link component
>> > 2) invoke onattach
>> > 3) invoke onclick
>> > 4) invoke onbeforerender
>> > 5) invoke render
>> > 6) invoke onafterrender
>> > 7) invoke ondetach
>> >
>> > notice 2 and 3 are reversed as per contract of onattach
>> > but there is a problem with this. suppose link has a compound model,
>> and
>> > inside the link you call getmodelobject() since the model belongs to
>> > link's
>> > parent shouldnt link parent's onattach be called as well? the
contract
>> > says
>> > so.
>> >
>> > the only way to guarantee onattach contract is to alter the event
chain
>> to
>> > this
>> >
>> > 1) resolve link component
>> > 2) invoke page.onattach
>> > 3) invoke onclick
>> > 4) invoke onbeforerender
>> > 5) invoke render
>> > 6) invoke onafterrender
>> > 7) invoke page.ondetach
>> >
>> > this means that all code that is in onattach needs to move to
>> > onbeforerender
>> > and onbeforerender needs to be relaxed to allow changes in the
>> component
>> > hierarchy, while onattach should not allow modification of
hierarchy.
>> this
>> > restriction is necessary so, for example, link's parent doesnt
remove
>> the
>> > link before onclick is invoked on it.
>> >
>> > invocation chain for a page set into the request cycle via
>> > setresponsepage()
>> > would be the same but with step 3 taken out.
>> >
>> > the refactor itself will be pretty easy
>> > make onbeforerender final - this will make all overrides visible
>> > move code from overrides to onattach
>> > delete onbeforerender
>> > rename onattach to onbeforerender
>> > create new onattach
>> >
>> >
>> > thoughts?
>> >
>> >
>> >
>> > -igor
>> >
>> >
>>
>> --
>> View this message in context:
>> http://www.nabble.com/another-big-2.0-refactor-tf2923331.html#a8171948
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>>
>
>
--
View this message in context:
http://www.nabble.com/another-big-2.0-refactor-tf2923331.html#a8172562
Sent from the Wicket - Dev mailing list archive at Nabble.com.
