Hi Phil,
As to 1): the overhead of calling a no-op method, even if many times, is not great. When you compare it to the overhead of say, rendering the component, I think you will find the overhead is insignificant. This assumes that tapestry would not need to create anything extra in order to make the call. And the list of components is already available so the list of page listeners need not be expanded.
I don't quite agree. If the list of components is used, that would imply recursion, which is not well known for efficiency. If listeners are used, then things are better (just a for cycle over an ArrayList), except for the fact that invocation of an empty method through an interface is not as trival in terms of cost as invocation of a method defined in a class -- it cannot be optimized as well in most cases, especially when the invocations refer to objects of different classes, as in this case.
Anyway, if tapestry is going implement Geoff's suggestion for components, I think pages should work the same way and overriding Page.detach() should be deprecated. I'd like that approach. It's like the move from jdk 1.0 event handling to 1.1 event handling: listening instead of overriding.
The listener approach makes using a method like initialize() tricky by default -- it requires explicit extra code that is often forgotten. In our experience the problem that initialize() solves (differences in component state after construction and detachment) is very common and often hard to diagnoze. I am all for consistency, but I don't think the price is good enough in this case.
Can the listener approach be used, and still provide the initialize() functionality (especially for components)? That will be really ideal, but I cannot think of an easy way to do that at the moment.
Sorry for disagreeing :-)
-mb
Phil Surette <[EMAIL PROTECTED]> wrote:
As to 1): the overhead of calling a no-op method, even if many times, is not great. When you compare it to the overhead of say, rendering the component, I think you will find the overhead is insignificant. This assumes that tapestry would not need to create anything extra in order to make the call. And the list of components is already available so the list of page listeners need not be expanded.
As to 2): yes there are virtues to marker interfaces. There are also minuses: it's a bit of magic that will be hard to discover while coding (as opposed to say, discovering that ther is a cleanUpComponent method, which your IDE would proabaly tell you about).
Anyway, if tapestry is going implement Geoff's suggestion for components, I think pages should work the same way and overriding Page.detach() should be deprecated. I'd like that approach. It's like the move from jdk 1.0 event handling to 1.1 event handling: listening instead of overriding.
-----Original Message-----
From: Joseph Panico [mailto:[EMAIL PROTECTED]]
Sent: Monday, November 11, 2002 10:36 AM
To: Phil Surette; [EMAIL PROTECTED];
[EMAIL PROTECTED]
Subject: RE: [Tapestry-developer] Recommended way to clean up a
component?
I like Geoff's suggestion. In fact, that is what we already do in our base
page class.>PageDetachListeners always and just have a detach() method like pages?
There are 2 problems with this approach.
1) There is a potentially very large number of components on a page and
there is no point in adding needless overhead to message all those
components if they are not interested in the event.2) Having a class implement an Interface is not only a run-time/compile-time
artifact, but it is also a valuable design signal. You are signalling that
this class is special in some way because it implements this interface. You
can use these cues later on when you are trying to figure out where
something is happening. It is also a signal to other developers that
something a little out of the orginary is going on. And by other developers,
I also mean the original developer 6 months after the initial code write,
when that developer is trying to figure out what the hell he was thinking 6
months earlier ;)joe
Joseph Panico
[EMAIL PROTECTED]
>From: Phil Surette <[EMAIL PROTECTED]>
>To: "'Geoff Longman'" <[EMAIL PROTECTED]>, "Tapestry
>Developer (E-mail)" <[EMAIL PROTECTED]>
>Subject: RE: [Tapestry-developer] Recommended way to clean up a component?
>Date: Mon, 11 Nov 2002 10:21:30 -0500
>
>...or something more obvious.
>
>To me, the differences between pages and components should be minimized
>whereever possible, since in the course of refactoring your app you often
>extract components from pages. Why not have components be
>PageDetachListeners always and just have a detach() method like pages? Is
>there a problem I'm not seeing, or is this just a usability tweak that has
>been overlooked?
>
>-----Original Message-----
>From: Geoff Longman [mailto:[EMAIL PROTECTED]]
>Sent: Monday, November 11, 2002 10:11 AM
>To: Phil Surette; Tapestry Developer (E-mail)
>Subject: Re: [Tapestry-developer] Recommended way to clean up a component?
>
>
>By looking at the Vlib example your following the pattern.
>
>I would suggest that explicity registering a DetachListener should not be
>required. Could not the framework recognize that a component
>implements PageDetachListener and just register it? That would save
>developers from having to override finishLoad().
>
>Geoff
>
>----- Original Message -----
>From: Phil Surette
>To: Tapestry Developer (E-mail)
>Sent: Monday, November 11, 2002 9:30 AM
>Subject: [Tapestry-developer] Recommended way to clean up a component?
>
>
>I have some components that retrieve and store a copy of a db object in a
>local variable which is accessed many times by the component. The object is
>lazily loaded like this:
>Object getCachedDBObject() {
> if (m_cachedDBObject == null) {
> m_cachedDBObject = retrieveObjectFromDB();
> }
> return m_cachedDBObject;
>}
>At the end of the current cycle, m_cachedDBObject needs to be nulled out so
>the next time the component is used it'll get a new copy.
>What's the best way to do this? Right now I'm making my components
>implement
>PageDetachListener and override finishLoad() like this:
>public finishLoad() {
> getPage().addPageDetachListener(this);
>}
>public void pageDetached(PageEvent pageEvent){
> m_cachedDBObject = null;
>}
>Is this the right way to do it? Is there a better way? I tried overriding
>cleanUpAfterRender but that didn't seem to work... could easily have been
>user error though.
_________________________________________________________________
Add photos to your messages with MSN 8. Get 2 months FREE*.
http://join.msn.com/?page=features/featuredemail
Do you Yahoo!?
U2 on LAUNCH - Exclusive medley & videos from Greatest Hits CD
