ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Costin Manolache <cmanola...@yahoo.com>
Subject RE: Roles (was: antlib)
Date Mon, 28 Apr 2003 20:07:23 GMT
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.




> 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.



>> > 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>. 



> 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.





> 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. 



>> 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. 

>> 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. 



>> >> 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.role.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.



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.



> 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 :-)

Costin


Mime
View raw message