Re: [io] thoughts on FileUtils methods while improving test coverage

[Ch...@dlr.de]

Hi,

__matthewHawthorne wrote:
> Most of these comments are directed at Jeremias... but if anyone else is
> interested, please let me know your thoughts.

I'm speking up, since many methods stem from my original code...

> 
> 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.
> 
> 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.

I use this for syncronizing with external code and I use it to create file
locks.

Testing could be done with threads and sleep calls..

> 
>    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 utility method comes handy when making directory listings from
within scripting tools (velocity, jelly).

I do agree that using FileFilters seems a more broader use, but is also
more dificult to use. The "String[] extensions" form may be better at
home in a FilenameUtils class.

> 
>        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.

My opinion is that traversing subdirectories should be optional.
Then the CVS-ignoring should be coded in some type of FileFilter - for
which another utility method would come handy...

> 
>    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 guess the java.lang classes have some less error prone ways of doing
such a conversion... This method must be rewritten or deleted.
e.g. File.getCanonicalPath()

> 
> 
>    I think that the following methods don't add a lot of value, and 
> should be deleted:

or moved to FilenameUtils...

> 
>        void fileDelete(String fileName)

Deprecate in favour of forceDelete.
Consider a wrapper to it in FilenameUtils...

>        boolean fileExists(String fileName)

Practical for scripting and other applications.

>        String     fileRead(String fileName)
>        void fileWrite(String fileName, String data)

This is a widely used functionality. It ensures avoiding
dangling file handles upon errors in scripting environments...

Must consider adding an encoding parameter explicitly.

>        boolean isFileNewer(File file, Date date)

Also practical, but maybe some of the overloaded signatures could
be reduced.

> 
> 
> 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 java.io.File import from their classes, we 
> should be trying to replace all of the things that they do with the File objects.

Unless they do a better error handling or generalize the usage
as in scripting environments (whre this code emerged...).

> 
> 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 java.io.File as 
> a parameter should be removed.  A lot of the parsing methods are useful and 
> can be put in the new FilenameUtils class.

Yup! +1

> 
> Let me know what you think.
> 

Cheers,
Christoph Reck