Re: [boost] Review Request: Endian

Subject: Re: [boost] Review Request: Endian
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2012-08-01 07:56:18

David Stone wrote:
>
> A few months back I posted a proposal for an endian
> conversion library:
> http://lists.boost.org/Archives/boost/2012/04/191931.php
>
> I believe my library is now in a finished state, and I would
> like to have it considered for inclusion into Boost. A zip
> file containing everything can be found here:
> https://bitbucket.org/davidstone/endian/get/tip.zip or you
> can just browse the repository here:
> https://bitbucket.org/davidstone/endian/

Boost.Endian was conditionally accepted last year.  You cannot use the same name.  Indeed, you need to see whether anything you've done can be added to Beman's library.  However, in the interest of helping your efforts in that direction, I have some comments on your library.

I dislike the names be_to_le, le_to_be, etc.  "be" is an English word and "le" is used for "less-or-equal-to" in many contexts.  Furthermore, the e's for "endian" are redundant given the boost::endian namespace.  For big-to-little and little-to-big endian conversions, the most common cases, clearer names are possible.  For example, to_big and to_little would imply the source endianness.  That doesn't help with the PDP byte ordering function names, but why not big_to_pdp, pdp_to_little, etc.?  I realize that those names are longer, but they are less cryptic.

Mixing identifier case is odd.  I realize the CamelCase names are in your detail namespace, but they are not The Boost Way(tm).

I'd prefer that you use the platform-supplied hton-style functions when applicable.  That is, when the type to swap has the same size as the sizes supported by those functions, it would be better to use the platform-supplied functions to ensure an optimal implementation.

I also dislike the names h_to_be, le_to_n, etc., for similar reasons.  I'd prefer to_network and to_host instead of your h_to_n and n_to_h?  Then, of course, big_to_host, network_to_little, etc.

The floating point conversion functions should be in a different namespace; they should not be accessible by default.  This is because such conversions are fraught with danger due to the possibility of creating special forms, including signaling values.

> I included the documentation in index.html, but I'm not quite
> sure on how to correctly format things.

Your documentation should start with a description of the library and then provide motivation for its use.  Rationale should be a late section.

Some platforms now supply htonll and ntohll, so your claim about no 64b support is not entirely accurate.

s/template functions/function templates/

s/a run-time determination as a back up/runtime logic selection as a fallback/

It is helpful to indicate headers as follows so users can copy and paste directly into their code:

   #include <boost/endian.hpp>

"Boost.Endian provides several functions that constructed in a fairly straightforward way" provides nothing useful as written, let alone the missing "are".  Indeed, the entire synopsis is lacking:

 - Every public function should be listed in the synopsis.  Each should be a link to its reference documentation.

 - "If other conversions are desired" doesn't give the reader any idea why such conversions would be desirable.

 - "They" is not the appropriate pronoun for a user.  It is plural.

You should provide more information about the problems of floating point conversions and why that support is experimental.

_____
Rob Stewart                           robert.stewart_at_[hidden]
Software Engineer                     using std::disclaimer;
Dev Tools & Components
Susquehanna International Group, LLP  http://www.sig.com

________________________________

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.