ah, but also notice that this is too simple to check the order of invocation
across the class hierarchy. but its def better then nothing.
-igor
On 11/8/06, Igor Vaynberg <[EMAIL PROTECTED]> wrote:
yep, if we do this we dont need an inspector or an aspect. we can throw an
exception or log a warning at runtime.
-igor
On 11/8/06, Johan Compagner <[EMAIL PROTECTED]> wrote:
>
> Today i was searching with martijn why something didn't work completely
> when
> having some code in the detachModels of the page.
> Then end result was that we had to override internalAttach() to do it!
> And looking at it, it is big mess, all those methods. And it is really
> not
> clear which does what.
> and when something can be called.
>
> I think we just need to get rid of the fear of no super call.
> And we can test it
>
> Component.onAttach() // so the deepest..
> {
> if(development)
> {
> set.add(component);
> }
> }
>
> then after we know attach should be called
>
> a Visitor checks all the components against that set.
>
> johan
>
> On 11/9/06, Igor Vaynberg < [EMAIL PROTECTED]> wrote:
> >
> > after having implemented the annotations and thought about this some
> more,
> > i
> > think i ended up patching the problem instead of fixing it.
> >
> > there are a couple of problems with annotations that i now see:
> >
> > a) we are not removing the overridible listeners from the api, so
> > annotations are just an alternative approach.
> >
> > b) annotations have poor api visibility - unless you know about them
> its
> > very likely you are not going to find them
> >
> > the reason why we have things like
> > Component.internalAttach
> > ()+Component.internalOnAttach()+Component.onAttach()
> > is that for methods like these the call to super() is very important.
> >
> > in init methods like onattach() it must be the first thing that is
> called.
> > int cleanup methods like ondetach() it must be the last thing to be
> > called.
> >
> > i think a more proper fix to the problem would be to properly document
>
> > these
> > and leave it to the users to implement properly. that way we can get
> rid
> > of
> > the annotations and get back some api clarity by remoing
> > Component.internalAttach() and Component.internalOnAttach () and
> moving
> > that
> > code into component.onattach().
> >
> > now this is problematic because it is easy to forget to call super,
> and
> > there is a lot of existing code out there that doesnt. i think what
> we
> > can
> > do is either create an aspect to check for the calls to super that is
> > loaded
> > only during devel mode, or create an inspector for a static analysis
> tool
> > like pmd or findbugs to look for the error.
> >
> > i think this will be a better solution then relying on annotations.
> >
> > in the end the user has to know the consquences when overriding a
> method.
> > us
> > trying to do as much as possible for the user only takes it part way -
> to
> > the Component level. But what if you extend Component and need to
> override
> > onAttach()? and then then another user subclasses this component? then
> the
> > problem of calling super is still there, we just deferred it for our
> > framework internal classes.
> >
> > what do you think?
> >
> > -igor
> >
> >
> > On 10/27/06, Igor Vaynberg <[EMAIL PROTECTED]> wrote:
> > >
> > > we have a couple of great candidate callbacks to be implemented with
> > > annotations instead of template methods. namely onattach/detach,
> > > onbegin/endrender
> > >
> > > what makes them great candidates is this
> > >
> > > 1) when implementing one you must always call super()
> > > this is not _always_ true. when extending a core component you dont
> need
> > > to because we have our own internalattach() we can use and
> onattach() is
> > > just an empty template.
> > >
> > > but when extending a non-core component you must call super()
> because
> > you
> > > never know if that component uses it or not and it is very very
> unlikely
> > > that it is a behavior you want to override
> > >
> > > 2) implementors of onattach(), if they are good, will make it final
> and
> > > provide another breakout template to ensure their onattach code
> cannot
> > be
> > > messed with. this leads you to have
> > >
> > > public final void onAttach(){ dosomething(); onAttach2(); } public
> void
> > > onAttach2() {}
> > >
> > > this is the general java pattern that i find very ugly personally,
> take
> > a
> > > look at our Component.internalAttach() impl :)
> > >
> > > so what i propose is that we provide some annotations
> > > @OnAttach @OnDetach @OnBeforeRender @OnAfterRender
> > >
> > > users can annotate any number of methods with these and have them
> > called.
> > >
> > > im sure there are other places that can benefit from this, but for
> now
> > we
> > > can cleanup the mess that is internalAttach(), internalOnAttach()
> > > onAttach(), etc
> > >
> > > of course this is wicket 2.0 only
> > >
> > > -Igor
> > >
> > >
> >
> >
>
>
