ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jon Skeet" <jon.sk...@peramon.com>
Subject RE: cvs commit: jakarta-ant/src/main/org/apache/tools/ant/filters BaseFilterReader.java
Date Wed, 24 Apr 2002 16:10:05 GMT
> > I noticed a couple of oddities when fixing this up:
> >
> > o readLine doesn't do the "normal" thing of coping with \r, 
> \n or \r\n. It
> does, however,
> > include the '\n' in the string, which is probably correct 
> given usage.
> 
> This is by design.  We are interested in returning
> the line with the EOL characters intact - hence,
> we keep adding characters to the line till we reach
> '\n' and include that too.

I thought that would probably be the case. What about being broken on Macs (which use '\r')
though?

> > o The no-arg constructor claims to be there just for Ant's 
> introspector:
> when does Ant's
> > introspector actually *need* this? What would break if we 
> got rid of it?
> Ant shouldn't be
> > trying to create any instances of this anyway, as it's abstract.
> 
> Let me try to explain the implementation -
> it may seem a bit complex - so be warned ;-)

<snip>

> Now, when the wrapper task (in this case, <copy>), starts working
> through its input streams, the ChainReaderHelper invokes the
> filterreader's chain method which chains one reader with another
> by invoking the constructor that takes in the InputStream as
> argument and then sets the new reader's state.

Right. That's what I thought was probably going on after getting a bit further - the introspected
helpers are only used to create *real* filters.

I personally don't like the encapsulation of this: they're acting as factories, really - there's
no reason why the factory class should be the same one as the reader, and by forcing them
to be the same we end up with this dummy constructor.

I'm not suggesting refactoring it for 1.5, but maybe for Ant2 it would be worth doing.

> Hope this clarifies the code better.  The ctor in the
> abstract base class needs to stay...

Agreed. It should be documented though - I'll try to work something out, unless you want to?

Jon

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


Mime
View raw message