Overall, I like the interfaces, but I've got a few issues:
0) Update is mentioned in the Javadoc at the beginning of Archiver, but
is not implemented.
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
2) You can only add files to an archive, this makes it more difficult
to add generated entries into an archive. You need to actually write
them to disk before including them in an archive
3) UnpackDestination + Destination should be the same property (on the
both the interface as well as underlying implementation), or you should
split packing and unpacking into two different interfaces (or force
folks to pass a path to pack + unpack, instead of setting properties on
the object).
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);
5) In AbstractCompressor, I don't understand why you have a copy method
and don't just use IOUtils.copy()
6) In AbstractCompressor, I don't understand why you try to create your
own temp name, rather than just letting File.createTempFile do it's thing.
--Will
C. Grobmeier wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hello all,
here is a new draft for the compress interface:
* http://grobmeier.de/commons-compress-draft-5.zip
I have improved a lot of things, based on the comments of Sandy (thanks
for that).
This draft is not perfect, as you can see in the todo list. But imho we
have a quite good base for future work, everything compiles and works
and is documented. So i would like to propose that someone is comitting
this, except you have reasons not to do this.
If you agree with me, i will open a bugzilla issue and add the link to
the code.
- - Chris.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFEVKMikv8rKBUE/T4RAlxxAKCMYzova2roWDA/skRyoDvFcErE2gCfTjFw
bT2VrGdR8Byt+VjsRo7Cyhw=
=1GRF
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
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]
