commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sandy McArthur" <sandy...@apache.org>
Subject Re: [compress] Interface is ready
Date Wed, 05 Apr 2006 16:47:20 GMT
On 4/5/06, Hariharasudhan R <hari.developer@gmail.com> wrote:
> Hi Sandy,
>
> Does this work with Solaris tar, since Solaris tar is different from GNU tar
> ?
> If not , is there a plan to do so ?
> Won't making TarEntry an interface with Solaris/GNU implementations be a
> good idea ?
> This is just an idea off the top of my head.. I've not delved too much into
> the code..

No clue. I don't really have access Solaris. I personally have access
to Linux, Mac OS X, Win32 and at work I can access AIX but that is
becoming less and less available as we move to linux.

> On 4/4/06, C. Grobmeier <grobmeier@possessed.de> wrote:
> >
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Hello Sandy,
> >
> > thousand thanks for your comments and the time you have spent for this.
> > I will work through this all and prepare another draft soon.
> >
> > Thank you very much,
> > Chris
> >
> > > The following is a stream of things I don't think are right in the
> > > order I observe them while skimming the code. I've never used the
> > > compress code so excuse any suggestions that miss the point.
> > >
> > > * Archiver: too many overloaded methods. Especially with setters.
> > > Having setFoo(File) and setFoo(String) will breaks the JavaBean
> > > property conventions and could cause problems with scripting tools. If
> > > you're going to do something like this then use different names eg:
> > > setDestinationFile(File) and setDestinationFilename(String) and let
> > > the latter be a virtual property that really just creates a new File
> > > and calls setDestinationFile(File).
> > >
> > > * Archive: doesn't do much, only contains one static method:
> > > getInstnace(ArchiverType). Because it's static you cannot subclass it
> > > so there is no way to extend it's behavior for when someone else want
> > > to implement CabArchiver. The type-safe enum example I gave in the
> > > other email makes this class unnecessary.
> > >
> > > * ArchiveType/CompressorType: make them type safe enums. Actually I
> > > think it's a mistake to distinguish between Archive types and
> > > Compression types. Making them separate you have one library that does
> > > two things and I think you should treat them as one thing and do that
> > > one thing well or have two libs that each do one thing.
> > >
> > >
> > > * TarConstants: is package scoped and, with the exception of one line
> > > in TarOutputStream, is only used by TarEntry. It looks to me the logic
> > > in TarOutputStream that uses TarConstansts could be easily pushed into
> > > TarEnrty and then the constants in TarConstants should be moved to
> > > where they are used.
> > >
> > > * TarInputStream: skimming it I don't it. It looks like it's taking
> > > the InputStream concept and corrupting it with the notion of many
> > > separate streams for each file in one stream. This is confusing
> > > because it doesn't fit the expectations of an InputStream. IMO it
> > > should be it's something similar to an Iterator that takes a raw
> > > InputStream and provides a way to get at metadata sequentially from
> > > the raw InputStream. From the meta data you should be able to get an
> > > InputStream which is just for that file in the archive.
> > >
> > > * TarOutputStream: Same problem as TarInputStream but with an
> > > OutputStream. Because TarOutputStream subclasses OutputStream it makes
> > > write methods available. But using these methods are dangerous because
> > > if you write a different number of bytes than what was passed when
> > > putNextEntry(TarEntry) was called you corrupt the archive. A good API
> > > doesn't let the programmer make mistakes. I'd change this to be it's
> > > own object type that accepts TarEntrys with all the needed to add a
> > > file in one step that either succeeds or fails.
> > >
> > > * TarInputStream/TarOutputStream: I don't see why these classes should
> > > be public. Making them package private would make me care less what
> > > they are because they aren't part of the public API and can be changed
> > > at will.
> > >
> > >
> > > * ZipLong, ZipShort: these are both public, and I don't see why they
> > > need to be. They are only used inside the package with the exception
> > > of 3 places ZipShort is used as a parameter type in a public method. I
> > > don't see why those ZipShorts cannot be converted to shorts for the
> > > public API and both of them made package private.
> > >
> > > * ZipOutputStream: has the same problems as TarOutputStream.
> > >
> > > * compress.archivers.zip: every class in this package is public, I
> > > don't think they all need to be.
> > >
> > >
> > > * compress.compressors.bzip2: This package seems fine. I don't get why
> > > CBZip2InputStream and CBZip2OutputStream start with the letter "C"
> > > though.
> > >
> > >
> > > * compress.exceptions: I don't get why the exceptions are segregated
> > > to their own package. Javadoc already keeps them separated so there
> > > isn't a need to put them in their own package. I'd put them in the
> > > compress package or a package closer to where they are used.
> > >
> > > --
> > > 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
> > >
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1.4.2.1 (MingW32)
> > Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
> >
> > iD8DBQFEMi31kv8rKBUE/T4RAhe3AJ9HCrNQf8dvHAyJnW20dWoF4tpnxwCgi7U8
> > Hu/4efxAre76d8mbK/mqPrw=
> > =4wUL
> > -----END PGP SIGNATURE-----
> >
> > ---------------------------------------------------------------------
> > 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


Mime
View raw message