[
https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-2.patch
derby-3770-2.stat
Thanks for your attention, Mamta. I have receive your comments just now. Sorry
to reply late.
<<Junjie, the patch is commented pretty well and the code changes for those
comments look good.
<<One comment for the engine code change
<<1)The 2 new methods skipFully(InputStream is) and skipFully(InputStream is,
long skippedBytes) in their javadocs only talk about IOException and
EOFException for skipFully(InputStream is, long skippedBytes). Should we put
NullPointerException() also in the javadoc?
-----I have add the declaration for NullException.
<<Just couple comments for the new junit test
<<1)testNullStream has 2 test cases to check for null inputstream. For some
reason, if no NullPointerException is thrown, then we have following to catch
it
<<fail("Null InputStream is refused!");
<<The error message looks misleading. Should it be saying something like
<<fail("Null InputStream is accepted!");
-----I have correct it.
<<2)The 2 tests in testNullStream only check for NullPointerException.
Shouldn't we be catching other exceptions and make the test fail for those
exceptions.
-----I'm not clear about this. What other exceptions should be tested int
testNullStream()? For EOFException, I have tested it in testSkipFully(). As to
IOException, excluding EOFException, I don't know how to create or simulate it.
Could you give me more advices?
<<3)Don't have to address this but should we consider combining
testSkipUtilEOFWithOddLength and testSkipUtilEOF into one test fixutre.
-----testSkipUtilEOFWithOddLength() only tests EOF with special length, I think
it's better to seperate it from common length. Is the name of the method not
clear? Is testSkipUtilEOFWithSpecialLength() better?
<<Thanks for working on this jira entry.
Mamta, please give more suggestion to improve the patch. Thanks again!
Regards
Junjie
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat,
> derby-3770-2.patch, derby-3770-2.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say
> broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip
> procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes
> skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws
> EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes
> have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil,
> could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
