ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jose Alberto Fernandez" <jalbe...@cellectivity.com>
Subject RE: Roles (was: antlib)
Date Tue, 29 Apr 2003 09:46:12 GMT
> From: Costin Manolache [mailto:cmanolache@yahoo.com]
> 
> Jose Alberto Fernandez wrote:
> 
> > Assume class C implements role intrefaces P, Q, and R then
> > 
> > <typedef name="C1" classname="C"/>
> > <typedef name="C2" classname="C"/>
> > 
> > will cause two definitions for "P" and "Q" each. There is no way to
> > assign different names separately. On the other approach:
> > 
> > <role name="p" classname="P"/>
> > <role name="q" classname="Q"/>
> > 
> > <p name="C1" classname="C"/>
> > <q name="C2" classname="C"/>
> > 
> > here you can get different names for different roles if you so wish.
> 
> 
> If C implements both P and Q, then it can be used as either P or Q. 
> Whetever name you give it. What's the use case for having <p> useable
> only in the P role ( when the class supports both roles ), but not in
> Q role ? 
> 
> Let's not add restrictions and rules just because we can.
> 
> 

No one is restricting you. What I am giving you is the chance to 
use diferent names if you wish to do so.

> 
> 
> > What is the meaning of:
> > 
> > <condition property="x">
> > <and>
> > <if>
> > <istrue value="yes"/>
> > <then><echo>yes</echo></then>
> > <else><echo>no</echo></else>
> > </if>
> > <istrue value="yes"/>
> > </and>
> > </condition>
> > 
> > "x" should be set, but shall "yes" be echoed? Although <if>
> > implements the contract of condition, the meaning of <if> is to
> > execute its nested tasks also, but for that <condition> will have to
> > call the execute() by looking at if as a task and not as a 
> condition.
> > 
> > So the whole construct falls appart.
> 
> What do you mean "falls appart" ? 
> 
> If <if> implements the contract of condition, it means it 
> should be used 
> as a condition. If it has side-effects - that's part of the 
> implementation,
> and you should expect them when you use it. If I add a println() in 
> <istrue> constructor - that will be displayed.
> 
> If you don't want <if> to be useable as a condition - don't make it
> implement condition.
> 

It sounds very nice, but the reality is that <if> already exists and has 
existed for a long time. Hence we can not go and change it just because
it does not fit the pattern you would like. Heck, it is not even our code
and there is a BC contract we need to fulfill.

The proposal as defined in <antlib> covers this issues satisfactorily.
That is why the proposal is the way it is, we do not have a black sheet of
paper, we have an evolving product that imposes constraints on us.

> 
> 
> >> > Secondly, since classes do not need to implement the 
> interface but
> >> > may be adapted, you cannot assume much there either.
> >> 
> >> I think I mentioned - this is a special case and it clearly
> >> requires a
> >> special declaration ( which associates the adapter with 
> the class ).
> >> 
> >> Something like:
> >>   <taskdef name="tomcatEngine"
> >>            class="org.apache.catalina.core.StandardEngine"
> >>            
> adapter="org.apache.commons.modeler.ant.JMXTaskAdapter" />
> >> 
> >> 
> > 
> > So, how you manage the current data-types which can be used 
> as tasks?
> > Backward compatibility says it needs to be treated 
> internally, but your
> > notation suggests we would have to declare them passing a 
> TaskAdapter
> > otherwise we keep the special cases around in core.
> 
> Yes, TaskAdapter and <taskdef> will need to remain special cases for
> backward compat.
> 
> And probably tasks will need to be special cases for backward compat.
> IMO a <typedef> with a class that implements Task should make that
> class a task, and be equivalent with a <taskdef>. 
> 

The antlib proposal solves this without requiring special cases.
It just all naturally falls into place.

> 
> 
> > With the way the proposal does it, we get the cruft out of core
> > and the declarations will continue to be as before because the
> > adaptor association occurs at the role-name level.
> 
> I think the proposal adds far more cruft and complexity to the core.
> Backward compat will require special cases anyway. 
> 
> And associating the adapter with the role is completely wrong 
> use of the
> pattern.
> 
> 

What is wrong with been able to define a "default" value for the adaptor.
That is all what the role declaration is doing, indicating what adaptor
to use by default if the class does not implement the interface.
This is a generic concept and there is no special cases of anything.

> 
> 
> 
> > The case of the <if> for starts. I would like to get an error if I
> > tried to use an <if> as a condition because if was not designed to
> > be used that way. And we have to deal with the code we already have.
> 
> Then don't implement the condition interface. 
> 
> That's how OO programming works - if you implement an 
> interface, java allows
> that class to be passed as parameter to a method. 
> 

The class is already out there, we cannot go changing it, we cannot
go breaking peoples buildfiles. We need to support what is already
there.

> 
> 
> >> My argument is more for the "common" or "simple" case - 
> which I want
> >> to be as simple as possible.  For your particular use case 
> - using the
> >> interfaces would require the least ammount of work and seems very
> >> natural. You declare that a task implements an interface - 
> that means
> >> ( usually ) it can be used in that role, just like any other class
> >>  that implements that interface.
> >> 
> >> If you implement Runnable - it is supposed that you can be used as
> >> a parameter to Thread. I feel it's a programming error to 
> implement an
> >> interface but not support its contract, and it's wrong to require
> >> an additional declaration ("I really implement the interface").
> >> 
> >> 
> > 
> > Remember part of the problem is that we need to deal with 
> the code that is
> > already out there which is not neat at all on these respects.
> 
> However we shouldn't add complexity and make things harder 
> just because some
> old code is bad. If we think that <if> shouldn't implement 
> condition - and 
> preventing it to be used as a condition is very important - 
> then we should
> fix <if>.
> 
> BTW, another solution is to enhance the pattern and have a setParent()
> method in the child. Then <if> impl. can check the parent and 
> if it's not 
> a task container then throw an exception. 
> 

Look who is talking about complexity, you are now trying to dump
the problem on the task writer. Isn't that more complex to everyone?
And all this just because you do not want to allow declarations to
specify which interfaces are you declaring on a particular name.
A thing you still have to do for things needing adaptors anyway.

> >> If you have an addFoo() and a <foo> child - I assume the 
> old pattern
> >> will be used, not the roles ( backward compat, etc ).
> >> 
> >> The roles would apply if you have a <foo> child and no addFoo() -
> >> you first create <foo> using the typedef/taskdef/componentDef, then
> >> look at the interfaces and find the addInterface() method.
> >> 
> >> 
> > 
> > You cannot do it this way. Because <foo> may mean different classes
> > depending on the roles of interest. Think of the <weblogic> element
> > of <ejbjar> and <jspc> they are different APIs but the 
> element is named
> > the same. So you first need to collect what possible roles 
> are posible
> > at this point (by looking to all the "addInterface" as you call it).
> > Then we can go and look for <foo> on those particular interfaces.
> > If at the end you find one and only one unambiguosly then 
> you succeeded.
> > 
> > BTW, the reason I would prefer to use "addConfigured" is 
> because we have
> > already taken over that method name as one of our patterns, 
> and hence is
> > safer for BC. Using "addInterface" or "addRole" my clash 
> with somebodies
> > task that uses those names legitimately today.
> 
> By "addInterface" I meant using some specific name, like 
> "addCondition"
> or "addJspc".
> 
> But your example makes sense - there are cases where <jspc> needs to 
> create a different class based on context. I would consider this 
> the non-simple case, and I agree we should be able to 
> eventually handle it
> with some special syntax. 
> 

The proposal as defined in antlib, does not need to wait for eventuallys
or new syntaxes, it supports this functionality right now, naturally. 

The whole point on antlib is to be able to allow 3rd party to incorporate 
their own code and to be able to integrate with existing tasks either
defined by us or by other 3rd parties. This is the most important point of
the whole thing. 

> 
> 
> >> >> I'm not sure a special name is actually required (
> >> >> addConfigured) - just an
> >> >> add or set method with the interface/class as parameter, and
> >> >> then match
> >> >> them on type.
> >> >> 
> >> > 
> >> > the idea of using "addConfigured" as the name in because 
> I think we
> >> > need to be clear on the parsing rules for this objects. Allowing
> >> > passing not configured objects seem to make little sense since
> >> > the implementation is completely blackboxed to the parent node.
> >> 
> >> What about add[INTERFACE_NAME]( INTERFACE_NAME ) ?
> >> 
> > So you mean something like:
> >
> addorg.apache.tools.ant.role.Condition(org.apache.tools.ant.ro
> le.Condition)?
> > :-(
> 
> No, just addCondition( org.apache.tools.ant.role.Condition )
> 
>  
> > when I said the role is the interface I meant it. I see no 
> reason why
> > the name of the interface has to be part of the method name.
> 
> It's cleaner and more intuitive. And I think it's a common pattern.
> The actual name of the method is more for users convenience - 
> we use the
> parameter signatures for the association ( in this case ).
> 
> 

> 
> 
> >> set methods work with simple types only - and IMO if we add roles,
> >> setROLE( ROLE ) should be used for roles where a single child is
> >> allowed, while addROLE( ROLE ) for multi-childs.
> >> 
> > 
> > I just do not want to create more confusion on users. The 
> rules today
> > are quite simple and easy to follow. set for attributes, 
> add/create for
> > subelements. It is a very simple rule. I do not want some 
> new special case
> > that only applies roles. If or when set methods are allowed for
> > single elements or attributes, then we can generalize it to 
> roles also.
> 
> Ok. 
> 
> 
> 
> 
> > The interface for Tasks can have as many adaptors as you want
> > using diferent names to refer to them. Think of role names really
> > as role aliases. Aliases for the same interface, each providing a
> > different adaptor, if needed.
> 
> What do you mean ? I should use the name "task" if the task adapter 
> is used, but a different role name if a different adapter is used ? 
> 
> No, the pattern for adapter/wrapper is pretty clear, and your use 
> is wrong.
> 
> >>  <component name="elementName" className="...." [ 
> adapter="...." ] />
> >>  
> >> BTW, my prefference would be to actually just use <typedef> as the
> >> definer ( with the additional optional adapter attribute ). But I'm
> >> fine with a new element name.
> >> 
> > 
> > Yes we disagree, what you present will not solve the case of vendor
> > specific elements like <weblogic> or <jboss> whose meaning 
> depends on the
> > context.
> > 
> > Would you allowed something like:
> > 
> > <component name="jboss" classname="jboss.ebjjarAPI"/>
> > <component name="jboss" classname="jboss.jspcAPI"/>
> > <component name="jboss" classname="jboss.serverdeployAPI"/>
> > 
> > Now are you changing the definition of "jboss" or are you adding
> > distinct definitions for distinct roles. How would anone understand
> > what this means?
> 
> As I said above, this is a valid case - but it's not the 
> simple or common
> case.
> 
> For jboss - you actually need to express that the child name <jboss> 
> will have different implementations based on the parent ( or some 
> other condition ). 
> 
> Not sure I understand how you would resolve this with your proposal -
> by defining different roles of jboss ? I can live with having 
> such extra syntax for the non-common case, but not to require it for 
> the common case.
> 

If you read the rules I submitted carefully, you will see they are defined
to cover this case: First you look for all the addConfigured methods of the parent
to get all expected interface-roles (that is you get the context in which
"jboss" needs to be interpreted). Then you look for "jboss" on the tables for such
interfaces; if you find only one definition the there is no ambiguity and
you can proceed.

It is all spell out there in my emails.

> 
> 
> BTW, one alternative solution for associating the name with an
> implementation  class is the use of namespaces. Axis does that quite 
> extensively, and seems to work very well.
> 
>   <parent> 
>     <ejb:jboss ns:ejb="java:jboss.ejbjarAPIImpl" /> 
>   </parent>
> or something similar.
> 

Everytime people do not have a solution, we get a new magic name space
declaration that magically is suppose to solve every problem in the world
(forget the ugly syntax no user will ever understand).

The proposal solves this without misusing namespaces.

> 
> 
> > Still we need this if we want to be able to send the 
> responsability for
> > maintaining vendor specific code back to the vendors.
> > 
> > Roles came up from the need to achieve exactly that.
> 
> > Adaptor came from looking at the actual mess we have in our 
> type system
> > and try to put some standarized mechanism that can dealt wih it.
> 
> I think the type system is not that bad ( or at least it's 
> cleaner than the 
> mess you propose :-)
> 

I would not even comment on that

Jose Alberto


Mime
View raw message