Mailing-List: contact ant-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: ant-dev@jakarta.apache.org
Date: Tue, 3 Apr 2001 18:54:00 -0400
From: Eric Siegerman <erics@telepres.com>
To: ant-dev@jakarta.apache.org
Subject: Re: [REVIEW/SUBMIT] Pattern-matching optimization
Message-ID: <20010403185400.B6308@telepres.com>
Mail-Followup-To: ant-dev@jakarta.apache.org
References: <20010326214614.A20377@telepres.com>
 <1v12ctk91o7avnl5sv2ike8rl709jurkpq@4ax.com>
 <20010327171403.B354@telepres.com>
 <s17fct8969l19d2l0f66bm6k5pmualokji@4ax.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <s17fct8969l19d2l0f66bm6k5pmualokji@4ax.com>;
 from d.rees.l@usa.net on Sun, Apr 01, 2001 at 02:34:06PM -0700
Organization: Telepresence Systems Inc.

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.

Thanks.

> 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/ZipScanner.java,
> 
> 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.        erics@telepres.com
|  |  /
With sufficient thrust, pigs fly just fine. However, this is not
necessarily a good idea.
	- RFC 1925 (quoting an unnamed source)