ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles Scokart" <gscok...@gmail.com>
Subject Re: svn commit: r695402 - in /ant/core/trunk/src/main/org/apache/tools/ant: DirectoryScanner.java types/selectors/PathPattern.java types/selectors/SelectorUtils.java
Date Tue, 16 Sep 2008 06:43:32 GMT
2008/9/15 Stefan Bodewig <bodewig@apache.org>:
> On Mon, 15 Sep 2008, Gilles Scokart <gscokart@gmail.com> wrote:
>
>> Cool, I was thinking to do something like that but I was not sure
>> about the class to use, and wheter it was really usefull to store
>> tokenized path into an object in case we have a single include
>> patterns.  I missed the fact that isDeeper was also requiring a
>> tokenization.  So clearly my doubt about the usefullness was invalid
>> (I was thinking that we could eliminate a tokenization only when we
>> had 2 or more includes pattern on which we loop).
>
> True, even with a single include pattern (which we always have
> since "no include pattern" gets translated to "**") the file's name
> would get tokenized twice, as would the include pattern itself.
>>
>> However, with this change you are storing pattern and values in the
>> same class.
>
> Is your main problem the name of the class?
>
>> I think we should have two classes.  Both need to tokenize a
>> string using the same tokenizer, but what you can do with both is
>> different.
>
> I'm not convinced their behaviour is really any different, every
> name is a pattern as well.
>
> IIUC you'd prefer to move the match methods into a pattern specific
> class - and probably change the left and right side in the new
> matchPatternStartOf method so that the new method is on class
> representing the pattern (all delegating to SelecotrUtils).  This
> would class share the tokenization (delegated as well), depth and
> contains logic with a different class used for paths without any
> wildcards.
>
> We could do that, but I don't really see that much benefit.
>
> Stefan
>

I'm still thinking that a 'PathPattern' and a 'PathValue' (or whatever
name you preffer) are two different things that have enought reality
to exists as independent classes.

For the moment, we are just using those classes as a tokenized string
container, but I could very well imagine other behaviors.  For
instance, we could build a new PathValue based on an other PathValue
to avoid even more tokenization (I don't think it is possible by
keeping the DirectoryScanner protected method, but that could be used
somewhere else).

But maybe you are right, that's maybe a YAGNI.  Maybe this class
should just be a tokenized string container which is pattern oriented.
 In that case, I think it should be renamed. Maybe TokenizedPattern ,
TokeninzedPathPattern , TokenizedPatternOrValue  ?

As I said, I'm not sure.

-- 
Gilles Scokart

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


Mime
View raw message