commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From will pugh <>
Subject Re: [compress] New draft 5
Date Mon, 01 May 2006 00:35:04 GMT
It occured to me later on, that I think this interface would be better 
if it were focused on describing an Archive, rather than trying to 
describe two processes (ie describing the noun rather than all the 
verbs).  The fall out from this, is that you don't have a member for an 
unpack directory, since that is only relevent to the unpacking process.  
and I think it makes the interface I went to mock this up and came up 
with a couple other issues with the interface.

  1)  Adding an entry or a file to an archive, you need to be able to 
set the entry name, otherwise you can run into problems with getting 
just the right relative path in the archive for the files.
  2)  It is pretty common to want to recursively add directories, we 
should provide this in the archive API

I've attached my mock-up so folks can see if they think these thoughts 
make sense.  I tried to borrow design and parameter conventions from 
some of the commons-io methods I use alot.


will pugh wrote:

> 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,, 
>, 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:
>> Hash: SHA1
>> Hello all,
>> here is a new draft for the compress interface:
>> *
>> 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.
>> Version: GnuPG v1.4.2.1 (MingW32)
>> Comment: Using GnuPG with Mozilla -
>> iD8DBQFEVKMikv8rKBUE/T4RAlxxAKCMYzova2roWDA/skRyoDvFcErE2gCfTjFw
>> bT2VrGdR8Byt+VjsRo7Cyhw=
>> =1GRF
>> -----END PGP SIGNATURE-----
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail:
>> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

View raw message