Re: RFR 8251989: Hex formatter and parser utility

[Roger Riggs]

Hi Peter,

I made the same observation when exploring the API and ended up concluding
that the declared exceptions on Appendable made the API much harder to use.

Though the formatted characters can't be directly accumulated in the 
Appendable
it is possible to build the output in a StringBuilder and then append 
the StringBuilder

to the appendable with only a single copy since it is a CharSequence.
That would avoid the creation/allocation of a String as an intermediate 
step.


Thanks, Roger


On 8/27/20 1:21 PM, Peter Levart wrote:

Hi Roger,


About methods in Hex.Formatter that append to StringBuilder, like the 
following one:



    public StringBuilder format​(StringBuilder sb, byte[] bytes)


...I was thinking that such method could have more utility if it was 
specified as:



    public  A format(A appendable, byte[] bytes)


For example, you could also format directly to PrintStream, Writer, 
CharBuffer or any custom implementation. The only grief is that 
Appendable methods are specified to throw IOException, but 
Hex.Formatter.format(Appendable ... methods could be specified to wrap 
such exception with UncheckedIOException. For usage with StringBuilder 
this would not change anything as it never throws IOException.


What do you think?

Regards, Peter


On 8/27/20 3:34 AM, Roger Riggs 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 



Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-hex-formatter-8251989/

CSR:
https://bugs.openjdk.java.net/browse/JDK-8251991

p.s.
The previous (encoder/decoder) javadoc has been renamed to:
   http://cr.openjdk.java.net/~rriggs/hex-encoder-javadoc/

Re: RFR 8251989: Hex formatter and parser utility

[Roger Riggs]

Hi Douglas,

On 8/27/20 10:37 AM, Douglas Surber wrote:

The meaning of prefix and suffix is not specified in formatter​(boolean 
uppercase, String delimiter,String prefix, String suffix). It isn't specified 
whether they precede and follow the entire formatted value or each byte. The 
class comment clarifies but I shouldn't have to go there to discover this.

I was surprised at the meaning for prefix and suffix. The seem pointless to me. 
It is trivial to enclose the entire formatted value with a prefix and suffix 
without using these arguments. If they were prefix and suffix for each 
individual byte, that would be much more useful. For example, how can I format 
a byte sequence like this?

0x00 0x01 0x02 0x0d 0x0e 0x0f

format(false, " 0x", "0x", "")

doesn't work because an empty byte array would be

0x

instead of an empty string.


The prefix and suffix concepts first appeared in the StringJoiner.
In the Hex context, they can be used to construct a complete string 
efficiently.

For example a mac address.  [50:2b:7f:e8:6a:e2]

I have tried out the use of prefix and suffix as you did above and 
noticed the same limitation.


I did run across several examples in the OpenJDK code where an empty 
string had a different representation

and it might reasonable to have a replacement for an empty array.
Though the factory methods are about the limits for numbers of args.
A more fluent builder API has been suggested.

The javadoc can be expanded upon to make it clearer.

Thanks, Roger




Douglas


On Aug 27, 2020, at 4:55 AM, core-libs-dev-requ...@openjdk.java.net wrote:

Message: 1
Date: Wed, 26 Aug 2020 21:34:47 -0400
From: Roger Riggs mailto:roger.ri...@oracle.com>>
To: core-libs-dev mailto:core-libs-dev@openjdk.java.net>>
Subject: RFR 8251989: Hex formatter and parser utility
Message-ID: <6378b60b-7a45-d8b0-5ebd-3d3bf9144...@oracle.com 
>
Content-Type: text/plain; charset=utf-8; format=flowed

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 



Webrev:
? http://cr.openjdk.java.net/~rriggs/webrev-hex-formatter-8251989/ 


CSR:
https://bugs.openjdk.java.net/browse/JDK-8251991 


p.s.
The previous (encoder/decoder) javadoc has been renamed to:
?? http://cr.openjdk.java.net/~rriggs/hex-encoder-javadoc/ 

Re: RFR 8251989: Hex formatter and parser utility

[Roger Riggs]

Hi Chris,

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

Roger,


On 27 Aug 2020, at 02:34, Roger Riggs  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.

Re: RFR 8251989: Hex formatter and parser utility

[Peter Levart]

Hi Roger,


About methods in Hex.Formatter that append to StringBuilder, like the 
following one:



    public StringBuilder format​(StringBuilder sb, byte[] bytes)


...I was thinking that such method could have more utility if it was 
specified as:



    public  A format(A appendable, byte[] bytes)


For example, you could also format directly to PrintStream, Writer, 
CharBuffer or any custom implementation. The only grief is that 
Appendable methods are specified to throw IOException, but 
Hex.Formatter.format(Appendable ... methods could be specified to wrap 
such exception with UncheckedIOException. For usage with StringBuilder 
this would not change anything as it never throws IOException.


What do you think?

Regards, Peter


On 8/27/20 3:34 AM, Roger Riggs 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 



Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-hex-formatter-8251989/

CSR:
https://bugs.openjdk.java.net/browse/JDK-8251991

p.s.
The previous (encoder/decoder) javadoc has been renamed to:
   http://cr.openjdk.java.net/~rriggs/hex-encoder-javadoc/

Re: RFR 8251989: Hex formatter and parser utility

[Douglas Surber]

The meaning of prefix and suffix is not specified in formatter​(boolean 
uppercase, String delimiter,String prefix, String suffix). It isn't specified 
whether they precede and follow the entire formatted value or each byte. The 
class comment clarifies but I shouldn't have to go there to discover this.

I was surprised at the meaning for prefix and suffix. The seem pointless to me. 
It is trivial to enclose the entire formatted value with a prefix and suffix 
without using these arguments. If they were prefix and suffix for each 
individual byte, that would be much more useful. For example, how can I format 
a byte sequence like this?

0x00 0x01 0x02 0x0d 0x0e 0x0f

format(false, " 0x", "0x", "") 

doesn't work because an empty byte array would be

0x

instead of an empty string.

Douglas

> On Aug 27, 2020, at 4:55 AM, core-libs-dev-requ...@openjdk.java.net wrote:
> 
> Message: 1
> Date: Wed, 26 Aug 2020 21:34:47 -0400
> From: Roger Riggs mailto:roger.ri...@oracle.com>>
> To: core-libs-dev  >
> Subject: RFR 8251989: Hex formatter and parser utility
> Message-ID: <6378b60b-7a45-d8b0-5ebd-3d3bf9144...@oracle.com 
> >
> Content-Type: text/plain; charset=utf-8; format=flowed
> 
> 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 
> 
>  
> 
> 
> Webrev:
> ? http://cr.openjdk.java.net/~rriggs/webrev-hex-formatter-8251989/ 
> 
> 
> CSR:
> https://bugs.openjdk.java.net/browse/JDK-8251991 
> 
> 
> p.s.
> The previous (encoder/decoder) javadoc has been renamed to:
> ?? http://cr.openjdk.java.net/~rriggs/hex-encoder-javadoc/ 
> 
> 
> 

Re: RFR 8251989: Hex formatter and parser utility

[Chris Hegarty]

Roger,

> On 27 Aug 2020, at 02:34, Roger Riggs  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.

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.

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 )

-Chris.