ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Bodewig <bode...@apache.org>
Subject Re: cvs commit: ant/src/main/org/apache/tools/ant/types/selectors SignedSelector.java
Date Mon, 22 Nov 2004 10:26:47 GMT
On 22 Nov 2004, <peterreilly@apache.org> wrote:

>   Index: ConditionAndTask.java
>
>       protected abstract boolean evaluate();
>   
>       /**
>        * This method evaluates the condition. It calls evaluate in
>        * the derived class.  It sets the property if a property is
>        * present and if the evaluate returns true.  @return true if
>        * the condition passes, false otherwise.  */
>       public boolean eval() {
>           if (evaluate()) {
>               if (property != null) {
>                   getProject().setNewProperty(property, value);
>               }
>               return true;
>           } else {
>               return false;
>           }
>       }

Why do you introduce an additional abstract method?

Wouldn't it be easier to place the logic of your eval into execute()
directly and force subclasses to implement eval() just as they'd do as
plain conditions anyway?

I'm a bit uncomfortable with ConditionAndTask in general, but that's a
separate issue from the above.

The other issue simply is: why do we need it?

What's wrong with

<condition property="..." ...>
  <foo/>
</condition>

Why do we need an additional <foo> task?  <available> and the other
tasks that can be conditions as well have simply been there before
<condition> was added, I don't really see why we should add more
property setting tasks.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message