commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeremias Maerki <>
Subject Re: [io] thoughts on FileUtils methods while improving test coverage
Date Mon, 08 Sep 2003 16:51:06 GMT
First of call, congrats for your committer hat. Too bad I was away
during the vote.

On 27.08.2003 01:21:36 __matthewHawthorne wrote:
> Most of these comments are directed at Jeremias... but if anyone else is
> interested, please let me know your thoughts.
> I ran the clover report in Maven to get a better idea of test coverage.  
> There
> is a lot of code that is not being executed, and I plan on writing a lot of
> tests to help fix that.

Yes, yes, yes, yes!

> The first class I'm working on is FileUtils, and I have a few notes and
> questions:
>     void waitFor(String, int)
>         - You made a note asking if this method makes sense.  I think 
> this could be
>         useful in some circumstances.  It's difficult to test though.

Christoph cleared that up, I guess. But the method does need a test case.

>     String[] getFilesFromExtension(String directory, String[] extensions)
>         - You made a note saying that this should be rewritten using the 
> filefilter
>         package.  I agree, but also believe that this method should be 
> simplified a little.
>         how about something like:
>             File[] getFiles(File directory, FileFilter[] filters)
>         If necessary, we could write a small wrapper around this method 
> which could
>         provide the functionality of the original.


>         This method also provides some undocumented behavior, such as 
> excluding all
>         "CVS" directories.  This is useful, but I think that the method 
> is trying to
>         do too much... it seems too specific for a *Utils class.  I 
> would think
>         that something like that should be in a CvsFileUtils class.

I don't think we'll put enough together to form a CvsFileUtils class. Or
am i wrong? Anyway, I like Christoph's idea of providing a FileFilter
for that functionality.

>     File toFile(URL)
>         - We've talked a little about this.  There seems to be such a 
> large room for
>         error in these types of conversions, that I don't like providing 
> them.
>         My vote is to eliminate this method.

I agree about the large room for error but still think this method can
sometimes come handy. I've written one myself a couple of years ago.
Documentation is everything: We could simply provide a warning.

>     I think that the following methods don't add a lot of value, and 
> should be
>     deleted:
>         void fileDelete(String fileName)
>         boolean fileExists(String fileName)
>         String     fileRead(String fileName)
>         void fileWrite(String fileName, String data)
>         boolean isFileNewer(File file, Date date)

No strong opinion here. The methods with a string parameter should go to
FilenameUtils and their naming checked.

fileRead and fileWrite have their own special problems (using the
default encoding, see my comment in the code). IMO they can stay but
should get a string parameter for the encoding.

> Here is my rationale for deleting some of the aforementioned methods: in a
> package like [io], there seem to be endless convenience methods that we can
> provide, so I think we should choose our battles wisely.  Methods that 
> save a
> user 1 line of code aren't worth it.  We shouldn't be providing methods 
> that will
> allow users to remove the import from their classes, we 
> should be
> trying to replace all of the things that they do with the File objects.
> Redundant stream reading/writing, filename parsing, cleaning 
> directories, these
> are useful things that we are providing.
> I'm starting to think that any methods that don't take a as 
> a parameter
> should be removed.  A lot of the parsing methods are useful and can be 
> put in
> the new FilenameUtils class.

+1 in theory. But I'm sure people will keep asking for them. So many
devs still believe a filename is a string. What I want is a big sign
telling them that best practice is to use when working with

Jeremias Maerki (who's so happy he doesn't have to apply Matthew's
great patches anymore)

View raw message