Re: MethodClosure (BCEL Delegator implementation)

 > To be clear, replacing the Excalibur implemenation wasn't my primary goal.

Understood. I, however, want to move the impl in Excalibur to a project 
better suited for it. If that "move" consists of developing a similar thing 
in another project (as happens here) or by the copying of source code, I 
don't care. Actually, the first thing Berin did after we had developed the 
Excalibur stuff was to try getting it into Commons/Lang.

 > > 1. The resulting delegate class must implement equals() in the same way
 > > as the Excalibur delegate.
 >
 > I've added this. Is it correct that you could have two equal delegates
 > that implement different interfaces?

Hmmm... You know, I didn't think about that. The issue only surfaces when 
you have a multi delegate that calls a method in an object through two 
interfaces, or when you subclass a delegate interface:

public interface DelegateInterface {
     public void doStuff ();
}

public interface MyDelegateInterface extends DelegateInterface {}

So is a delegate that implements MyDelegateInterface equal to one that 
implements DelegateInterface?

I'd say yes, it is correct that you could have two equal delegates
that implement different interfaces.

 > > 2. I lack the MultiDelegate from Excalibur.
 >
 > Yes, I have to think more about that. The advantages will not be quite as
 > great, since you'll have to loop over the list of registered listeners in
 > either case.

If I remember correctly, the Excalibur version will take a copy of the list 
of delegates at every invocation. I'm sure that can be sped up without 
losing thread-safety...

Seriously, though, I'm much less attracted by the speedup than I am by the 
fact that delegates are in a project outside of Avalon. Given that the 
method being delegated to probably has a runtime orders of magnitude 
greater than the delegate's own, the actual invocation speed is only 
important in a few cases. (Consider a GUI event handler: The overhead by 
the delegate code is negligible to the processing in the method itself.) 
Having Delegates outside of Avalon means this, though:

  + A standard delegate library => code reuse.

  + Avalon's scope is kept (no bloat).

  + No dependencies on Avalon, real or percieved.

  + Less code maintenance for Avalon. Given that the code is so 
out-of-scope for Avalon, we have much more trouble maintaining it than you 
have. (For example, you already have a cache class, we'd have to develop 
one specifically for this.)

Don't get me wrong - having fast delegates is good, and I think that you've 
done a great job -  but from my point of view, the great things you bring 
are not the performance gains, but the organizational.

 > > 3. I don't understand the MethodClosureKey interface. Is this a way to
 > > get a new delegate with the same signature, but pointing to another
 > > instance of the handling class? (I.e. a clone() equivalent) If so, why
 > > is the first parameter a Class and not an Object?
 >
 > MethodClosureKey should be a package-protected interface, but that
 > causes problems under JDK1.2. It is used internally to maintain a cache
 > of generated MethodClosure classes (see the KeyFactory class for more
 > info).

...

 > > 5. I'm not sure I like the fact that the delegate object also doubles
 > > as a factory. You can get method signature collisions.
 >
 > I've changed the code so that you'll get a ClassCastException if you
 > make a newInstance of the wrong type (before it was deferred until you
 > invoked the delegate).
 >
 > The method is used internally to generate new instances from cached
 > factory versions of the delegates. It could be made a protected method,
 > but I think it can be useful as a public API too (faster than going
 > through the cache). I'm not set on this though.

I'm more worried about method signature collisions that you may get. 
Suppose I want a delegate and I define the interface to have a newInstance 
method with the exact same parameters as the existing newInstance method. 
Protected or public, you will see some strange things happening.

It exposes an implementation detail (the signature newInstance (...) is 
already taken) that doesn't have to be exposed.

I would prefer it this way:

You use BCEL to generate a class that looks like this:

public class $Delegate implements <Interface>
     private final Object target;

     public $Delegate (Object target) {
         this.target = target;
     }

     // This is the delegate's method
     public <Interface.Method> {
         ...
     }

     public boolean equals (Object o) { ... }
     public int hashCode () { ... }
}

This is equivalent to the current proxy class that you generate, but with a 
constructor, and without a newInstance method.

In the cache you keep a mapping between methods and classes of the type 
above. Then, when you create a delegate, you see if you have defined a 
class for it before (check the cache), and if not, create it. Then you 
invoke the single-arg constructor with the target object and return the 
instance.

That separates the factory and delegate roles for the objects and 
eliminates the mixing of concerns (factory and proxy).

/LS