[Webmacro-devel] Re: ContextTool replacement - pseudo code to clarify

[Brian G. <br...@qu...>]

> However all these changes really are relatively trivial - moving loadTools 
> from Broker is the "biggest" job. I'm happy to do all this if you guys 
> agree on the plan, if Lane isn't fussed about doing it. It would seem 
> fitting that I have to do is, since I was the main protagonist in all this 
> rucus.

This seems mostly reasonable, see comments inline.  However, this
seems nothing like your original proposal.  What you've done is
renamed ContextTool, not gotten rid of it.  Now, perhaps that's what
you meant all along, but that's certainly not what it sounded like you
were proposing.

> --- New file: ContextAware.java

What does "context aware" mean?  Can any provide three examples of
something that is context aware that is not context tools?  I still
don't get the need for this concept beyond the existing tool
mechanism.

> --- New file: LazyVariableProvider.java I think this is the name that fits 
> the functionality best, getting away from the ContextTool terminology, 
> using WM provider terminology
> 
> public interface LazyVariableProvider extends Provider
> {
>     Object getVariable( String name, Context context);
> }

Good concept, but took a wrong turn somewhere.  If you are going to be
a Provider, be a Provider.  Implement the Provider interface and
that's it.  Me, I personally think that the Provider framework is a
terrible abstraction (I think you think this too) because it forces
everyone who uses it to make all sort of hidden casting assumptions.
This one is even worse -- its not really even a provider, since the 
meat of it is in an extra method that is outside Provider.  

In other words, either use Provider the way the rest of WM does
(personally, I'd like to see the rest of WM use it less, until it gets
to the point where it withers and dies on its own) or just define
the factory you need.

In any case, I don't think Context belongs in this interface.  

> --- New file: DefaultContextToolProvider.java The default impl providing 
> existing CT functionality by excised from core of WM

Why?  Why can't you just LEAVE IT THERE?  If its so worthless, it will
die of its own.

> --- Context.java Modifications....

>             Object tool = null;
>             if (lazyVariableProvider != null)
>                 tool = lazyVariableProvider.getVariable( name, this);
> 
>             if (tool != null) {
> // Remove this code if we decide Context will not handle this kind of thing
>                 if (tool instanceof ContextAware)
>                     ((ContextAware)tool).init(this);
>             }

I think this is OK here, but not in the put() call.  Putting it in the
put() call is totally magic!  (If the object implements this
interface, then this magic thing happens.)

>     /**
>      * Add an object to the context returning the object that was
>      * there previously under the same name, if any.
>      */
>     public final Object put (Object name, Object value)
>     {
> // Remove this code if we decide Context will not handle this kind of thing
>         if (value instanceof ContextAware)
>             ((ContextAware)value).init( this);
>  return _variables.put(name, value);

You can't put this here.

However, you could add this:
     public Object put(Object name, ContextAware value) { 
     }

like the auto-wrapping we have for int and other primitives.