On 18/04/16 15:30, Pavel Rappo wrote:
On 13 Apr 2016, at 10:10, Chris Hegarty <chris.hega...@oracle.com> wrote:
A general comment; Quite a number of classes, especially Encoder and Decoder,
would benefit from a few relatively lightweight comments, e.g. IntegerReader
I would like to address it after the initial push.
That's fine.
> If possible. Other than that,
I have updated the implementation:
http://cr.openjdk.java.net/~prappo/8153353/webrev.01/
I'm happy with this version.
-Chris.
There's also a javadoc now:
http://cr.openjdk.java.net/~prappo/8153353/javadoc.01/
Removed some FIXMEs and TODOs, updated tests.
StringReader.java
reset() can probably just reset its readers, regardless of huffman, since the
readers are always guaranteed to be present ( and reset is minimal ).
General comment; in many cases it would be useful to capture, at least, the
‘state’
when throwing InternalError.
Done.
IntegerReader.java
reset() assumes that there is no bugs in the code. Should is assign 0 to r,
value, and maxValue as well.
It will be done in the next invocation of 'configure'.
Q: is it necessary to call configure after reset?
Yes.
ISO_8859_1.java
Sorry, I don’ get 'outstanding'. Are you catching RuntimeException somewhere,
or do you expect a user of the Decoder to do some special exception handling ?
Removed. It was a leftover from a failure-atomic behaviour, which is not needed
here.
HpackException.java
Do you need this ??? ( FIXME comment )
Removed, thanks. Instead, a sandwich is thrown:
UncheckedIOException(ProtocolException).
