ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Siegerman <>
Subject Re: [REVIEW/SUBMIT] Pattern-matching optimization
Date Tue, 03 Apr 2001 22:54:00 GMT
On Sun, Apr 01, 2001 at 02:34:06PM -0700, David Rees wrote:
> Let me prepend this with the general comment that I agree with
> creating Pattern objects and your general changes from a design
> perspective.


> But I am worried because this is such a core part of the
> system. My belief is that for such a core piece that it is preferable
> to accept a "uglier" solution that makes less changes than a cleaner
> one that makes more.

I don't find this compelling.  Maybe it's just that I've been
reading Martin Fowler's "Refactoring" book :-)  With unit tests,
he says, such caution should be unnecessary.  That's why I wrote
so many -- and why, when some of them failed, I asked for help in
deciding whether it was the code or the tests that needed fixing.
Nobody's commented on that yet, btw -- I guess I picked a bad
week to ask, what with all the voting etc. going on...

> >FWIW, in order to make ant compile with my changes, the only
> >other file that needed to be touched was types/,
> Did you check the optional tasks? Specifically VAJWorkspaceScanner and
> FTP$FTPDirectoryScanner?

This, on the other hand, is rather more compelling.  No, I hadn't
checked either of those, because the build silently excluded them
both due to missing prerequisites.  Now that you point them out
to me, I see your point -- FTP$FTPDirectoryScanner will be as
easy to fix as ZipScanner was, but VAJWorkspaceScanner might be
rather a pain.

> >> Couldn't you just apply your patch to the existing methods?
> >
> >No.  Or at least, not at all cleanly!  Without the refactoring,
> >there's no place to store isSimpleFilename -- without some gory
> >hack like keeping arrays of booleans parallel to "includes" and
> >"excludes", which I agree with you would be very dangerous.
> >
> Is caching the value what saves your time? From what I see I _think_
> you could just put this in a static method and still get your
> performance improvements.

Yeah, I could do that.  It goes against the grain, of course --
as it is, it was some effort to resist making the
isSimpleFilename initialization lazy...

> I wonder maybe if a simpler static method check for "simple filename"
> would make more sense for Ant 1.X and this idea should be included
> into the general new design of scanners and fileset in Ant2.

On this, I can only defer to the committers.  Anyone care to make
a ruling?


|  | /\
|-_|/  >   Eric Siegerman, Toronto, Ont.
|  |  /
With sufficient thrust, pigs fly just fine. However, this is not
necessarily a good idea.
	- RFC 1925 (quoting an unnamed source)

View raw message