Re: MethodClosure (BCEL Delegator implementation)

[Leo Sutic]

> 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


--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>