Sandy McArthur wrote:
On 4/30/06, will pugh <[EMAIL PROTECTED]> wrote:
1) You often change method names based on the parameter types, e.g.
Archiver.addFile + Archiver.addFileName, setUnpackDestinationName +
setUnpackDestinationFile, etc. It seems more conventional and less
chaotic to give all the methods the same name, and have them only differ
based on parameter. Examples of this style are constructors for
java.utils.zip.ZipFile, java.io.FileInputStream,
java.io.FileOutputStream, org.apache.commons.io.IOUtils.IOUtils.copy,
org.apache.commons.io.FileUtils.isFileNewer, etc
This is intentional, in a previous version the method names were the
same. The problem with using the same name but different param types
breaks the JavaBean property getter/setter rules and the classes will
not be as usable in at least some scripting environments.
Hmm. What scripting envirionments don't allow you to call this kind of
Java method? I looked at the documentation for Jythong, JRuby and
Groovy and they all seemed to be O.K. with dealing with methods that
differ by parameter types.
Also, it doesn't seem like the current interface adheres to the Java
Bean specification anyways (unless I'm missing somethign). If you
wanted to adhere to the Java Beans Spec, I would suggest having only one
getter and one setter for each property, i.e. not having a
setUnpackDestinationFile and setUnpackDestinationFileName, but only
having setUnpackDestination(File f).
As I see it, here is the list of areas I think the archiver interface
offers odd behaviour relative to the JavaBean spec:
1) The sourceFile property is recieved by getSourceFile(), but is set
by loadFile and loadFilebyPath. Following the JavaBeans rules, this
means there is a readonly property called sourceFile, rather than this
being a property that is gettable and settable.
In addition to load* are bad names because they don't actually load
anything.
2) The unpackDestination property has a getter, but no setter.
The unpackDestinationFile property has a setter but no getter.
The unpackDestinationName property has a setter but no getter
4) None of your interfaces deal with the case of what to do if the
destination file is already there. Choices are either defining it in
the interface, or adding a property defining what to do. I would
suggest the latter, and would suggest that for Archiver this method
should take a FileFilter (since the unpack behaviour can be non-trivial)
and should default to FalseFileFilter.INSTANCE. For the Compresser
interface a simple boolean is probably sufficient. e.g.
Archiver.setOverwriteFilter(TrueFileFilter.INSTANCE)
Compresser.setOverwrite(true);
I'll disagree that this should be configurable. IMO it should just do
whatever FileOutputStream or whatever is used does. It's the calling
code's responsibility to handle any name collisions.
How can the calling code handle name collisions? It takes significantly
more work for it to walk through all the entries in the archive and then
walking through all the files in the file ssytem. Are you suggesting a
caller should only unpack into an empty directory?
I'm not really sure I understand what your saying about
FileOutputStream. The apis for the Archiver don't allow you to set an
OutputStream, and it's not really applicable to upack (which is writing
many files).
FileOutputStream allows you to set a boolean for whether the stream
should overwrite or append. This is made more difficult in the unpack
scenario where there are more than one file. From personal experience,
I liked the solution used in FileUtils.iterateFiles and
FileUtils.listFiles for doing yeah/nay on a recursive operation like this.
Cheers,
--Will
--
Sandy McArthur
"He who dares not offend cannot be honest."
- Thomas Paine
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
