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
