Alexei Fedotov wrote:
Tim, thank you for reading and understanding. I like when my code
passes a review and I am eager to try arguments I've created for
myself. I thought of your variant and liked it because it would be
possible to JIT to inline your version of ByteStream.readFully()
method.
There is some complication caused with a public API exposed by
Manifest class: the constructor and read methods accept a general
InputStream as a source for Manifest. Other applications, such as
Eclipse, read a file themselves and then call Manifest API directly to
get few main attributes. There is a better chance that external
developers would use use documented ByteArrayInputStream to wrap their
bytes than "our possibly optimized internal child" of that class.
I also believe that I decrease overall project astonishment by
encapsulating more complex technique in one place instead of spreading
less conventional things around. Reflection is hidden in a class with
conventional API, and no one would question when reading
ZipFile#getInputStream(ZipEntry) code why he should wrap a resulted
byte buffer in "our possibly optimized internal child" instead of
ByteArrayInputStream.
Finally two these methods may be combined: in addition to
ByteArrayInputStream one may check in readFully(InputStream) if "our
possibly optimized internal child" is supplied an use a quick path
without reflection. I wonder what you and other java gurus think about
this.
Alexei,
I wrote a version of the class that I was thinking about (without
reflection) [1].
It has the same #readFully behavior, but if we allow the
ZipFile#getInputStream method to return a ByteStream then we will get
the optimizations you talk about.
Take a look and let me know what you think -- then we can take the other
parts a step at a time.
[1] http://issues.apache.org/jira/secure/attachment/12378007/ByteStream.java
Regards,
Tim
