On Tue, Nov 26, 2013 at 4:32 PM, Gary Gregory <garydgreg...@gmail.com> wrote: > On Tue, Nov 26, 2013 at 12:31 AM, Damjan Jovanovic <dam...@apache.org>wrote: > >> On Mon, Nov 25, 2013 at 1:50 PM, Emmanuel Bourg <ebo...@apache.org> wrote: >> > Hi Damjan, >> >> Hi Emmanuel >> >> > I reviewed the API, here are my observations: >> >> Thank you. >> >> Firstly, we discussed several options before for the 1.0 release, and >> agreed that the contents of trunk would be quickly pushed out as 1.0 >> with minimal changes (many/most users are using 1.0-SNAPSHOT), and >> then the big API redesign would be 2.0. I've also been thinking of >> doing a complete rewrite for 2.0 and only pulling in some of the good >> bits we have now. So it's extremely discouraging to be pushed for more >> and more changes, many of which will have no post-1.x value, and don't >> even fit in with what was originally agreed on. >> >> > - Would that make sense to move the LZM implementation to >> commons-compress? >> >> I think you mean LZW, and let's discuss that on COMPRESS-243. >> >> > - What is the purpose of CachingInputStream and CachingOutputStream, >> > there is no Javadoc on these classes. Could they be merged into >> commons-io? >> >> It looks like CachingInputStream is used by IccProfileParser, and >> appears to be used to store data that has been read from the >> underlying stream so it can be re-read later. You can copy it to >> commons-io, but I'd prefer not having a runtime dependency on it. And >> it's ByteSourceInputStream you really want in commons-io and/or >> commons-compress, a gem that allows seeking over an InputStream. >> >> > - Why is ZLibUtils in org.apache.commons.imaging.common instead of >> > org.apache.commons.imaging.util ? >> >> The whole org.apache.commons.imaging.util package is a bit dated. >> >> > - FastByteArrayOutputStream is only used by PackBits in the same >> > package, it could be made package private. >> >> Yes. >> >> > - BitArrayOutputStream in org.apache.commons.imaging.common is only used >> > by classes in org.apache.commons.imaging.common.itu_t4. It could be >> > moved into this package and made package private. >> >> Yes. >> >> > - There are several unused public methods in BinaryInputStream >> >> Yes. >> >> > - org.apache.commons.imaging.common.Compression is never used >> >> I imagine the idea was to use it instead of using the different >> compression classes directly, but whoever started writing it never >> finished. >> >> > - RgbBufferedImageFactory is only used in RoundtripTest, should this be >> > moved with the test sources? >> >> Was probably meant to be the way to create all images in the API, but >> again never got finished, and is probably wrong now since some image >> formats produce paletted images. >> >> > - SimpleBufferedImageFactory is only used by ImageParser and could be >> > made package private. >> >> And bears suspiciously high resemblance to RgbBufferedImageFactory. >> >> > - What about replacing org.apache.commons.imaging.common.ByteOrder with >> > java.nio.ByteOrder? >> >> Enum vs public static final, hmm. >> > > I think the point is that ByteOrder is a JRE provided class, we should not > reinvent the wheel, unless we need to support middle-endian. Does Java run > on PDP-11? ;)
You laugh, but the leDOS JVM runs on MS-DOS, and JAmiga on Amigas, so it might be possible :). Also by the same analogy, org.omg.CORBA had IntHolder since Java SE 1.2, yet we still have MutableInt in commons-lang. But either way, Imaging's ByteOrder has already been replaced with java.nio's. Damjan --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
