On Wed, 2 Mar 2011 11:46:29 -0600 "Terry Golubiewski" <tjgolubi@netins.net> 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. * * *