ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter reilly <peter.rei...@corvil.com>
Subject Re: patch for namespaced antlib
Date Wed, 04 Jun 2003 09:43:10 GMT
Hi Antoine,

Thanks for your detailed comments.

On Tuesday 03 June 2003 20:26, you wrote:
> ----- Original Message -----
> From: "peter reilly" <peter.reilly@corvil.com>
> To: "Antoine Levy-Lambert" <levylambert@tiscali-dsl.de>
> Sent: Tuesday, June 03, 2003 2:01 PM
>
> >TypeAdapter behaves in the same way as the current TaskAdapter.
> >(It is a generalization of TaskAdapter).
> >The current TaskAdapter has a proxy object, which is of the
> >class defined in the taskdef. The getters/setters/adders etc operate
> >on that object. The adapter class is the exposed class - the class
> >that execute is seen , or the class that is presented at the add(class)
> >method.
>
> After a closer read, I think that the javadoc comments of TaskAdapter,
> which you have copied in the new TypeAdapter interface, are the problem.
> This one for instance "Sets the target object to proxy for." for
> setProxy(Object o) is not very well formulated.
> I would rather say : "Sets the proxy object, whose methods are going to be
> invoked by ant"

Ok will do.

>
> Similarly for getProxy
> "Returns the target object being proxied." does not mean anything to me.
> I would suggest simply
> "Returns the proxy object"

Ok will do.

>
> >Yes, I did forget to copy the roleClass.  I probally should have made
> >a clone method - but I have never liked the clonable interface and in
> >this case I want to set the project on the copied object.
> >
> > >3) In AntTypeDefinition
> >>
> >> What is the difference between AssignableClass and Role ?
> >
> >AssignableClass is the class "adapter" is meant to adapt the user's class
> >to. If the user class already implements/extends ("isAssignable" from)
> >the AssignableClass, they there is no need for the adapter.
>
> From reading your code, AssignableClass seems to be a synonym for the
> adaptto attribute you are introducing in typedef.
Yes.
> I think that it is also the same as a role. To be sure, ask yourself what
> it going to be the adaptto class for a filter ? I would bet it is going to
> be ChainableReader ? For me this is also a role class.

No,  adaptto is not a role, it is only used in association with
"adapter" - the adapter class gets used if the defined class does
not fullfill the contract specified by "adaptto". The idea is to
allow <taskdef classname="org.acme.MySuperTask ... /> to be implemented by
<typedef adapter="o.a.t.a.TaskAdapter" adaptto="o.a.t.a.Task" 
classname="org.acme.MySuperTask" ....>
If mySuperTask does not extend Task, then the TaskAdapter will be used
to adapt MySuperTask.

The idea behind "role" is to restrict the definition to only be used
for the "add(Class)" methods and for "ant-type=" polymorphic attribute.
This is to allow the conditions/filters etc to be defined without polluting
the namespace. The definitions will only be active in the add(Class)/ant-type
context.

To give a example: (and to use one of Jose's examples)
Say one has a ton of classes that use the method "isValid()" to
assert that the object is correct and one wants to use these
as conditions in ant.

One can write an adapter class that implements
 o.a.t.a.taskdefs.condition.Condition and adapts the isValid()
method to eval().  - org.acme.ValidConditionAdapter.

case 1:
<typedef adapter="org.acme.ValidConditionAdapter"
              classname="Class1" name="def1"/>

This will unconditionally adapt Class1 to a condition using 
org.acme.ValidConditionAdapter even if Class1 implements
Condition.

The definition of "def1" may be used outside the context
of "add()". When used with add(o.a.t.a.taskdefs.c.Condition x),
the x.getClass() is org.acme.ValidConditionAdapter.

case 2:
<typedef adapter="org.acme.ValidConditionAdapter"
              adaptto="org.apache.tools.ant.taskdefs.condition.Condition"
              classname="Class1" name="def1"/>
This adapts Class1 to a condition using 
org.acme.ValidConditionAdapter unless Class1 implements Condition.

The definition of "def1" may be used outside the context
of "add()". When used with add(o.a.t.a.taskdefs.c.Condition x),
the x.getClass() is org.acme.ValidConditionAdapter if Class1
does not implement Condition and Class1 if it does implement
condition.

case 3:
<typedef adapter="org.acme.ValidConditionAdapter"
              role="org.apache.tools.ant.taskdefs.condition.Condition"
              classname="Class1" name="def1"/>

This will unconditionally adapt Class1 to a condition using 
org.acme.ValidConditionAdapter even if Class1 implements
Condition.

The definition of "def1" WILL ONLY be used in the context
of add(Condition x) and for ant-type="def1" polymorhic.
When used with add(o.a.t.a.taskdefs.c.Condition x),
the x.getClass() is org.acme.ValidConditionAdapter.
One may have a number of definitions of "def1" for different
roles.

case 4:
<typedef adapter="org.acme.ValidConditionAdapter"
              adaptto="org.apache.tools.ant.taskdefs.condition.Condition"
              role="org.apache.tools.ant.taskdefs.condition.Condition"
              classname="Class1" name="def1"/>

This will adapt Class1 to a condition using
org.acme.ValidConditionAdapter even if Class1 implements
Condition.

The definition of "def1" WILL ONLY be used in the context
of add(Condition x) and for ant-type="def1" polymorhic.
When used with add(o.a.t.a.taskdefs.c.Condition x),
the x.getClass() is org.acme.ValidConditionAdapter if
Class1 does not implement Condition and Class1 if
it does.
One may have a number of definitions of "def1" for different
roles.


I am happy to accept suggestions on different names for
"adaptto", "role" .......

I will also pull the role stuff from AntTypeDefinition
in this patch as it should be considered separately.

>
> <Correct me if I am wrong> What about calling AssignableClass AdaptToClass
> ? it would maintain a continuity with the attribute name used in
> typedef.</Correct me if I am wrong>

Will do. Initialially I used the attribute "assignable", but I found that the
build scripts looked wrong.

Cheers,
Peter


Mime
View raw message