Hi Chris,

On 8/27/20 9:20 AM, Chris Hegarty wrote:
Roger,

On 27 Aug 2020, at 02:34, Roger Riggs <roger.ri...@oracle.com> wrote:

Please review updates to the formatting and parsing API based on the last round 
of comments.
There are many changes, so it may be useful to read it as a fresh draft.

  - Rename classes: Encoder -> Formatter; Decoder -> Parser
  - Rename methods: encode -> format; decode -> parse, etc.
  - Rename factory methods to match
  - Added a factory method and re-arrange arguments to make it more convenient
    to create uppercase formatters based on the existing uses.
  - The implementation has been updated based on the suggestions and API changes

The webrev for applying the API to the security classes will be updated when 
the API settles down.

JavaDoc:
http://cr.openjdk.java.net/~rriggs/hex-formatter/java.base/java/util/Hex.html
1. "Hex.Formatter formats bytes to a hex string or appends to
     StringBuilder.”

I wonder if this could be a bit more descriptive. What about something
like:

“Hex.Formatter transforms sequences of bytes into a formatted
hexadecimal string.”  What is a formatted hexadecimal string?  “A
formatted hexadecimal string consists solely of; an optional prefix, a
pair of hexadecimal characters ( THESE ARE ALREADY DEFINED ELSEWHERE),
an optional delimiter, and an optional suffix.”  Or some such wording.
Thanks for the suggestion.

I was expecting that the brief description in the Hex class would lead
to the more complete desriptions in the Formatter and Parser classes
and avoid duplication.

* {@link Formatter} Hex.Formatter transforms sequences of bytes into a formatted * hexadecimal string. * A formatted hexadecimal string consists solely of an optional prefix, * a sequence of pairs of hexadecimal characters, an optional delimiter between each pair of * hexadecimal characters, and an optional suffix. * The {@link #formatter() canonical formatter} with an empty prefix, suffix, * and delimiter, provides unformatted (plain) encoder functionality.




2. I like the move to formatter/parser, but I now find myself looking
for an encoder/decoder ;-) It might be obvious, but could be worth
calling out explicitly, e.g. “A formatter with an empty prefix, suffix,
and delimiter, provides unformatted (plain) encoder functionality".
Similar for parser.   Maybe this is what “canonical” is referring to?
If so, I think it would be best to define it somewhere.
Yes, that's the canonical formatter.

3. "The Hex class consists solely of factory methods for hexadecimal
(hex) formatters and parsers.”

It could be worth generalising the class-level description now, so as
to allow space for additional convenience methods in the future. ( A
while back I suggested not adding convenience methods yet, but I see
that Mark has suggested a couple of key conveniences )
Fair enough.

Another thought though,  both the Formatter and Parser classes contain
only the parameter (and they the same parameters). With as few format methods and parse methods, it could collapse down to just the Hex instances with the parameters and having both the format and parse methods.  All of the behavior is in the methods,
not the class.  Though it does collapse the name space.

Thanks, Roger


-Chris.


Reply via email to