RFR: 8170769 Provide a simple hexdump facility for binary data

[Vincent Ryan]

Thanks for your review Stuart.
Comments below.


> On 6 Dec 2018, at 02:18, Stuart Marks <stuart.marks at oracle.com> wrote:
> 
> Hi Vinnie,
> 
> Roger Riggs wrote:
>>> The 'forEachOrdered' should not be necessary and may raise questions about why.
>>> if there's no good reason, use 'forEach’.
> 
> Using forEachOrdered() is necessary. The dumpAsStream() method produces a stream; presumably it has a defined order that's the same as its input. The semantics of Stream.forEach() are extremely relaxed, and in particular it's not required to process stream elements in order, even if the stream is ordered. (In practice this is noticeable if the stream is run in parallel.)
> 
> I'd use forEachOrdered() both in the examples and also where you use it in implementations.

I’ve retained forEachOrdered() throughout.

> 
> The methods that return streams should specify the important characteristics. Probably the ones most important one are that the returned stream is ordered and sequential. For the overloads that take fixed-size input, the resulting stream might also be SIZED.

I’ve updated the javadoc @return tag for the Stream-returning methods.
Let me know if you’d prefer the stream characteristics to also appear in the method’s first sentence.

> 
> I'm not convinced that the overloads that send output to an OutputStream pull their weight. They basically wrap the OutputStream in a PrintStream, which conveniently doesn't declare IOException, making it easy to use from a lambda passed to forEachOrdered(). If an error writing the output occurs, this is recorded by the PrintStream wrapper; however, the wrapper is then thrown away, making it impossible for the caller to check its error status.

The intent is to support a trivial convenience method call that generates the well-known hexdump format.
Especially for users that are interested in the hexdump data rather than the low-level details of how to terminate a stream.
The dumpAsStream methods are available to support cases that differ from that format.

Have you a suggestion to improve the dump() methods, or you’d like to see them omitted?

> 
> The PrintStream wrapper also uses the platform default charset, and doesn't provide any way for the caller to override the charset.

Is there a need for that? Originally the requirement was driven by the hexdump format which is ASCII-only.
Recently the class has been enhanced to also support the printable characters from ISO 8859-1.
A custom formatter be supplied to dumpAsStream() to cater for all other cases?

> 
> Instead, you can just provide the Stream-returning methods, and let the user send the output to a PrintStream using forEachOrdered() as in your examples.
> 
> It might be nice to provide convenience APIs to send output elsewhere, but the problem is that it seems difficult to do so without losing control over things like error handling or charsets. In particular, since the hex formatter is producing strings, it seems like there should be an option to send the output to a Writer. Unfortunately it's difficult to do so from a Stream, because all the Writer methods throw IOException. However, solving this isn't hexdump's problem.

It’s a little more effort but those cases can always be handled by a custom formatter.


> 
> s'marks