Re: [boost] [xint] Boost.XInt formal review

Subject: Re: [boost] [xint] Boost.XInt formal review
From: Chad Nelson (chad.thecomfychair_at_[hidden])
Date: 2011-03-02 21:12:32

On Wed, 2 Mar 2011 11:46:29 -0600
"Terry Golubiewski" <tjgolubi_at_[hidden]> wrote:

> This is definitely a useful library. Thanks.
> I have a few some comments/suggestions to contribute.

I'm always happy to hear them. :-)

> I would prefer than highestbit just returned bitsize_t(-1) or some 
> predefined constant, similar to string::npos.

With the current design, you can tell it what you want it to return if
called on a zero. There's no truly correct answer in that case, a
default of zero with a user-provided override was the best I could come
up with.

> Conversion to/from dynamic_bitset would be nice.

Added to the to-do list.

> Consider adding a function returning the number of ones in the
> integer. This could be used to determine parity.

Is there a common use-case that requires this ability? I ask because
I've never found a need of it myself, or seen any code that made use of
it.

> Consider adding a function to reverse the order of the bits.
> "reflect" might be a good name.

Given that XInt is intended primarily as a math library, would this
really fit?

> Consider adding conversions to/from floating point types.

Please see the rationale section of the documentation, under the
heading "Why didn't you provide conversions to and from float/double
types?"

> Should integer_traits and numeric_limits be specialized for xint?

numeric_limits already is -- please look for the string
"numeric_limits<boost::xint::integer_t" (sans quotes) in
boost/xint/integer.hpp. integer_traits probably should not be, because
it only adds const_min and const_max, and there aren't any fixed
minimum or maximum values for most integer_t objects.

> I assume that xint is not thread-safe and just gives the user a way to
> force a "real" copy for multi-threaded applications (I like it!).

It can be made entirely thread-safe, using the Threadsafe option, but
this is less efficient. It's mostly thread-safe even without it, you
just can's safely use integer_t objects from threads other than the
one that created them.

The forced-copy parameter on the integer_t constructor is there if you
don't want to use the Threadsafe option, but still need to move
integer_t objects between threads.

> Consider adding an endian argument to to_binary.  Note that this
> could affect both the ordering of bits within each byte as well as the
> ordering of the bytes themselves.  (I actually prefer the libary the
> way it is, assuming little-endian.)

Sorry, but I don't entirely understand. to_binary writes the information
out one byte at a time, so there aren't any byte-endian issues with it.
Or do you mean to provide an option to write it out highest-byte-first
too?

The bit-endian problem is pretty specialized. According to Wikipedia
(<https://secure.wikimedia.org/wikipedia/en/wiki/Endianness#.22Bit_endianness.22>),
it's generally handled transparently. Do you see a use-case that would
require such an option?

> I like that xint includes move support.

Thanks! I intend to improve that once Boost.Move is official.

Thanks for your comments.
-- 
Chad Nelson
Oak Circle Software, Inc.
*
*
*