You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "C. Grobmeier" <gr...@possessed.de> on 2006/05/12 10:01:46 UTC

[compress] Draft 6

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

here is a new draft:
http://www.grobmeier.de/commons-compress-draft-6.zip

I have changed a lot of things discussed with draft 5.
The API is easier now. Everything what seems to be unnecessary has been
deleted. Compress is working with Files and InputStreams only, no more
String-params are allowed. New is also ArchiveEntry, which represents
the entry of an archive ;-).

For example, a zip file can be created like this:

Archiver archiver = ArchiverFactory.ZIP.getInstance();
	archiver.add(	
		new File("C:\\Temp\\1.html"));
	archiver.add(	
		new File("C:\\Temp\\1.html.bz2"));
	archiver.save(	
		new File("C:\\Temp\\ZIPTEST.zip"));

and unpacked like this:

Archiver archiver = ArchiverFactory.ZIP.getInstance(
			new File("C:\\Temp\\ZIPTEST.zip"));
archiver.unpack( new File("C:\\Temp\\unpacked\\"));


There a lots TODO, like handling archive-manipulating, overwriting
files, checking out the solaris issue or how to set special flags.
But imho with this interface we have done a step into the right
direction and now we can think about the more difficult things.

Regards,
Chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEZEDqkv8rKBUE/T4RAlP+AJ9wYDnXZoPK3/oAfutGSKfw690C6ACeNQHZ
Qg9hnd0w6IqMLBV47pqzOXM=
=zgEA
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Torsten Curdt <tc...@apache.org>.
> i have read your suggestion but i was not sure why i should accept
> strings for choosing an archive.

This can also be convenient if the method comes from a configuration.
Common practise for factories.

> I like the idea ArchiveFactory
> identifys archives by it's header. So, thanks for making this clear to me.

No worries :)

cheers
--
Torsten

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by "C. Grobmeier" <gr...@possessed.de>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> ...so what's the benefit over the usual factory + constructor
> instantiation then?
> 
> Archive arch = new TarArchive(new File("my.tar"));
> Archive arch = ArchiveFactory.newInstance(new File("my.tar");
> Archive arch = ArchiveFactory.newInstance("tar");

i have read your suggestion but i was not sure why i should accept
strings for choosing an archive. I like the idea ArchiveFactory
identifys archives by it's header. So, thanks for making this clear to me.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEZaN4kv8rKBUE/T4RAiV3AJ9WcG4Sfl4b7Hlv1pj/DDbojqbV9gCeJaFd
lZAWKvyFjba5F7LEVYQqKV4=
=9Ojn
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Torsten Curdt <tc...@apache.org>.
> > if (".zip".equals(extension)) -> ArchiveType.ZIP.newInstance();
> > if (".rar".equals(extension)) -> ArchiveType.RAR.newInstance();
> > ...
> >
> > in you code - which is cumbersome IMO.
>
> In that situation you'd use: ArchiveType.valueOf(extension).newInstance()

...so what's the benefit over the usual factory + constructor
instantiation then?

 Archive arch = new TarArchive(new File("my.tar"));
 Archive arch = ArchiveFactory.newInstance(new File("my.tar");
 Archive arch = ArchiveFactory.newInstance("tar");

cheers
--
Torsten

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Sandy McArthur <sa...@apache.org>.
On 5/12/06, Torsten Curdt <tc...@apache.org> wrote:
> > the small problem with that or any factory is the use of a String to
> > request a behavior means the compiler cannot know for sure if that
> > code will work.
>
> And this is a problem why?
>
> I think it would be great to be able to just pass in a file object
> into the factory that will look e.g. at the file extension (or even
> header) and figure out what archiver to use. Otherwise you *always*
> have to implement that part yourself.
>
> Checking the result of a factory for null or throwing an exception is
> common practise and I cannot really see real benefit of the
> compile-time check here.
> There are probably always going to be archivers that compress is not
> going to support. So the factory *cannot* always return an instance
> ...unless you do all the checking
>
> if (".zip".equals(extension)) -> ArchiveType.ZIP.newInstance();
> if (".rar".equals(extension)) -> ArchiveType.RAR.newInstance();
> ...
>
> in you code - which is cumbersome IMO.

In that situation you'd use: ArchiveType.valueOf(extension).newInstance()

-- 
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Torsten Curdt <tc...@apache.org>.
> the small problem with that or any factory is the use of a String to
> request a behavior means the compiler cannot know for sure if that
> code will work.

And this is a problem why?

I think it would be great to be able to just pass in a file object
into the factory that will look e.g. at the file extension (or even
header) and figure out what archiver to use. Otherwise you *always*
have to implement that part yourself.

Checking the result of a factory for null or throwing an exception is
common practise and I cannot really see real benefit of the
compile-time check here.
There are probably always going to be archivers that compress is not
going to support. So the factory *cannot* always return an instance
...unless you do all the checking

if (".zip".equals(extension)) -> ArchiveType.ZIP.newInstance();
if (".rar".equals(extension)) -> ArchiveType.RAR.newInstance();
...

in you code - which is cumbersome IMO.

cheers
--
Torsten

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by "C. Grobmeier" <gr...@possessed.de>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

will pugh wrote:
> Yeah.  I agree valueOf would work just fine, but unless I'm missing
> something, the current implementation doesn't have this.  The purpose of
> my comments was to say that I thought it should have something along
> those lines.

The valueOf is on the TODO.
Cheers,
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEZaMjkv8rKBUE/T4RAhidAJ9+bj2TDSwjYa38MeFn7POKhFY2cgCfUgu7
ar6ChVFXGvEGc3u2CygbDUI=
=/zrx
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by will pugh <wi...@sourcelabs.com>.
Yeah.  I agree valueOf would work just fine, but unless I'm missing 
something, the current implementation doesn't have this.  The purpose of 
my comments was to say that I thought it should have something along 
those lines.

I'm unsure of the value of having the static Factories, e.g. 
ArchiverFactory.ZIP

    --Will

Sandy McArthur wrote:

> On 5/12/06, will pugh <wi...@sourcelabs.com> wrote:
>
>> Hmm.
>>
>> Factories have always seemed most useful to me when they are
>> encapsulating the creation of some configurable resource.  In practice,
>> it seems like "configurable" most often means pulling a string from some
>> attribute or properties file, and using that to choose implementation.
>>
>> If the type of Archiver I am using is static and would never change, I
>> would simply the Archiver's constructor.  If it's not static and will
>> change, you are almost always going to have to deal with a string at
>> some point.  Passing that string into a factory is easier than getting
>> this ZIP member via reflection (and cleaner than special casing every
>> Archiver type)
>
>
> FWIW: Reflection isn't needed for the valueOf method. the method just
> looks in an internal Map or Set for the requested type.
>
>> Sandy McArthur wrote:
>>
>> > On 5/12/06, Torsten Curdt <tc...@apache.org> wrote:
>> >
>> >> > here is a new draft:
>> >> > http://www.grobmeier.de/commons-compress-draft-6.zip
>> >> >
>> >> > I have changed a lot of things discussed with draft 5.
>> >> > The API is easier now. Everything what seems to be unnecessary has
>> >> been
>> >> > deleted. Compress is working with Files and InputStreams only, 
>> no more
>> >> > String-params are allowed. New is also ArchiveEntry, which 
>> represents
>> >> > the entry of an archive ;-).
>> >> >
>> >> > For example, a zip file can be created like this:
>> >> >
>> >> > Archiver archiver = ArchiverFactory.ZIP.getInstance();
>> >>
>> >> Why not use a proper factory? Did you consider my suggestions?
>> >>
>> >> 
>> http://www.mail-archive.com/commons-dev@jakarta.apache.org/msg79370.html
>> >
>> >
>> > At Draft 3 or 4 I suggested he use a type safe enum for an
>> > "ArchiveType" which fully implemented would have implicitly had the
>> > factory feature built in and can remain extendable. eg:
>> >
>> > ArchiveType someType = ArchiveType.valueOf("zip")
>> > Archive someArchive = someType.newInstance();
>> >
>> > the small problem with that or any factory is the use of a String to
>> > request a behavior means the compiler cannot know for sure if that
>> > code will work. The enum solves this if the programmer wants by having
>> > the constants. If the compiler allows the following you know it work:
>> >
>> > Archive zip = ArchiveType.ZIP.newInstance();
>> >
>> > That said, it's better to commit to one idiom. Chris, as long as you
>> > have the ball on this you gotta pick the suggestions that work well
>> > together from all the feedback. On the surface the "ArchiveFactory"
>> > Factory/Enum hybrid seems to be less good than just using either a
>> > factory pattern or an enum pattern alone.
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>>
>>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Sandy McArthur <sa...@gmail.com>.
On 5/12/06, will pugh <wi...@sourcelabs.com> wrote:
> Hmm.
>
> Factories have always seemed most useful to me when they are
> encapsulating the creation of some configurable resource.  In practice,
> it seems like "configurable" most often means pulling a string from some
> attribute or properties file, and using that to choose implementation.
>
> If the type of Archiver I am using is static and would never change, I
> would simply the Archiver's constructor.  If it's not static and will
> change, you are almost always going to have to deal with a string at
> some point.  Passing that string into a factory is easier than getting
> this ZIP member via reflection (and cleaner than special casing every
> Archiver type)

FWIW: Reflection isn't needed for the valueOf method. the method just
looks in an internal Map or Set for the requested type.

> Sandy McArthur wrote:
>
> > On 5/12/06, Torsten Curdt <tc...@apache.org> wrote:
> >
> >> > here is a new draft:
> >> > http://www.grobmeier.de/commons-compress-draft-6.zip
> >> >
> >> > I have changed a lot of things discussed with draft 5.
> >> > The API is easier now. Everything what seems to be unnecessary has
> >> been
> >> > deleted. Compress is working with Files and InputStreams only, no more
> >> > String-params are allowed. New is also ArchiveEntry, which represents
> >> > the entry of an archive ;-).
> >> >
> >> > For example, a zip file can be created like this:
> >> >
> >> > Archiver archiver = ArchiverFactory.ZIP.getInstance();
> >>
> >> Why not use a proper factory? Did you consider my suggestions?
> >>
> >> http://www.mail-archive.com/commons-dev@jakarta.apache.org/msg79370.html
> >
> >
> > At Draft 3 or 4 I suggested he use a type safe enum for an
> > "ArchiveType" which fully implemented would have implicitly had the
> > factory feature built in and can remain extendable. eg:
> >
> > ArchiveType someType = ArchiveType.valueOf("zip")
> > Archive someArchive = someType.newInstance();
> >
> > the small problem with that or any factory is the use of a String to
> > request a behavior means the compiler cannot know for sure if that
> > code will work. The enum solves this if the programmer wants by having
> > the constants. If the compiler allows the following you know it work:
> >
> > Archive zip = ArchiveType.ZIP.newInstance();
> >
> > That said, it's better to commit to one idiom. Chris, as long as you
> > have the ball on this you gotta pick the suggestions that work well
> > together from all the feedback. On the surface the "ArchiveFactory"
> > Factory/Enum hybrid seems to be less good than just using either a
> > factory pattern or an enum pattern alone.
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>


-- 
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by will pugh <wi...@sourcelabs.com>.
Hmm.

Factories have always seemed most useful to me when they are 
encapsulating the creation of some configurable resource.  In practice, 
it seems like "configurable" most often means pulling a string from some 
attribute or properties file, and using that to choose implementation.

If the type of Archiver I am using is static and would never change, I 
would simply the Archiver's constructor.  If it's not static and will 
change, you are almost always going to have to deal with a string at 
some point.  Passing that string into a factory is easier than getting 
this ZIP member via reflection (and cleaner than special casing every 
Archiver type)

       --Will

Sandy McArthur wrote:

> On 5/12/06, Torsten Curdt <tc...@apache.org> wrote:
>
>> > here is a new draft:
>> > http://www.grobmeier.de/commons-compress-draft-6.zip
>> >
>> > I have changed a lot of things discussed with draft 5.
>> > The API is easier now. Everything what seems to be unnecessary has 
>> been
>> > deleted. Compress is working with Files and InputStreams only, no more
>> > String-params are allowed. New is also ArchiveEntry, which represents
>> > the entry of an archive ;-).
>> >
>> > For example, a zip file can be created like this:
>> >
>> > Archiver archiver = ArchiverFactory.ZIP.getInstance();
>>
>> Why not use a proper factory? Did you consider my suggestions?
>>
>> http://www.mail-archive.com/commons-dev@jakarta.apache.org/msg79370.html
>
>
> At Draft 3 or 4 I suggested he use a type safe enum for an
> "ArchiveType" which fully implemented would have implicitly had the
> factory feature built in and can remain extendable. eg:
>
> ArchiveType someType = ArchiveType.valueOf("zip")
> Archive someArchive = someType.newInstance();
>
> the small problem with that or any factory is the use of a String to
> request a behavior means the compiler cannot know for sure if that
> code will work. The enum solves this if the programmer wants by having
> the constants. If the compiler allows the following you know it work:
>
> Archive zip = ArchiveType.ZIP.newInstance();
>
> That said, it's better to commit to one idiom. Chris, as long as you
> have the ball on this you gotta pick the suggestions that work well
> together from all the feedback. On the surface the "ArchiveFactory"
> Factory/Enum hybrid seems to be less good than just using either a
> factory pattern or an enum pattern alone.
>

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Sandy McArthur <sa...@apache.org>.
On 5/12/06, Torsten Curdt <tc...@apache.org> wrote:
> > here is a new draft:
> > http://www.grobmeier.de/commons-compress-draft-6.zip
> >
> > I have changed a lot of things discussed with draft 5.
> > The API is easier now. Everything what seems to be unnecessary has been
> > deleted. Compress is working with Files and InputStreams only, no more
> > String-params are allowed. New is also ArchiveEntry, which represents
> > the entry of an archive ;-).
> >
> > For example, a zip file can be created like this:
> >
> > Archiver archiver = ArchiverFactory.ZIP.getInstance();
>
> Why not use a proper factory? Did you consider my suggestions?
>
> http://www.mail-archive.com/commons-dev@jakarta.apache.org/msg79370.html

At Draft 3 or 4 I suggested he use a type safe enum for an
"ArchiveType" which fully implemented would have implicitly had the
factory feature built in and can remain extendable. eg:

ArchiveType someType = ArchiveType.valueOf("zip")
Archive someArchive = someType.newInstance();

the small problem with that or any factory is the use of a String to
request a behavior means the compiler cannot know for sure if that
code will work. The enum solves this if the programmer wants by having
the constants. If the compiler allows the following you know it work:

Archive zip = ArchiveType.ZIP.newInstance();

That said, it's better to commit to one idiom. Chris, as long as you
have the ball on this you gotta pick the suggestions that work well
together from all the feedback. On the surface the "ArchiveFactory"
Factory/Enum hybrid seems to be less good than just using either a
factory pattern or an enum pattern alone.

-- 
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [compress] Draft 6

Posted by Torsten Curdt <tc...@apache.org>.
> here is a new draft:
> http://www.grobmeier.de/commons-compress-draft-6.zip
>
> I have changed a lot of things discussed with draft 5.
> The API is easier now. Everything what seems to be unnecessary has been
> deleted. Compress is working with Files and InputStreams only, no more
> String-params are allowed. New is also ArchiveEntry, which represents
> the entry of an archive ;-).
>
> For example, a zip file can be created like this:
>
> Archiver archiver = ArchiverFactory.ZIP.getInstance();

Why not use a proper factory? Did you consider my suggestions?

http://www.mail-archive.com/commons-dev@jakarta.apache.org/msg79370.html

cheers
--
Torsten

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org