On 11/15/13 1:01 PM, Alan Bateman wrote:
On 15/11/2013 17:12, Xueming Shen wrote:
I'm happy to put the "IAE with advanced positions" change back, the
webrev has been updated
according.
http://cr.openjdk.java.net/~sherman/8028397/webrev/
That looks much better.
One case that is still a bit awkward is where there are insufficient
bits remaining in the source buffer, say when you are consuming from a
channel and reading a chunk at a time. The decode(ByteBuffer,
ByteBuffer) method really needs a boolean to indicate to the decoder
if it capable of providing more bits. I guess it's too late to add
that now.
We really need CodeResult here to have a full control of the decoding
life circle. But the original goal
is to have a "simple" API, without getting into this decoding state
management. That said, to return a
native value (either -1 or the native value of the bytes written) is
better than to throw a IAE if you expect
app to use this method to take care "under_flow" (charset term)
management. It might look better to
check the return value < 0, instead of catching the IAE and then recovery.
As regards the javadoc updates then a small typo on L660 (=> "is a
padding character"). On L849, I think I would drop "current". Do you
think decode(ByteBuffer) needs a sentence to make it clear that the
input buffer's position may be advanced even if an exception is thrown?
Did you mean "to make it clear that the input buffer's position will NOT
be advanced"? the implementation and
the intent of the spec is not to advance, since when we have a malformed
input, we don't get an output buffer
to collect the bytes decoded. So the case here is "there is malformed
input, I can't do nothing for it, so I
will not touch the src position..."
On the test then it looks like there isn't any coverage to check the
output buffer's position has been advanced. Otherwise looks fine.
The output buffer's position is "checked" by checking the its available
(decoded) bytes against
the expected.
-Sherman
