Re: [boost] [Review] of binary_int utility starts

From: Cromwell Enage (sponage_at_[hidden])
Date: 2005-10-22 17:35:25

> ===Open questions in Scott Schurs's library===
> 
> * How does it work on 64 bit systems? Could someone
> try?

Sorry, I'm not that well off yet.

> * Naming: perhaps shorter names could be used (e.g.
> bits/nibble)

The current name is fine.

> * BOOST_WORKAROUND in the code, is it really needed?

I don't know.  I use GCC 3.4.2 (MinGW) and MSVC 7.1,
which are treated as class-A compilers.

> === Questions for the reviewers===
> 
> * Which one of the alternatives fits in Boost
> better?

First, assuming binary_int becomes MPL-friendly, I
believe the following code illustrates a more common
usage:

template <typename IntegralConstant>
struct color_argb64
{
    typedef typename bitand_<
                IntegralConstant
              , binary_int<0000,0000,0000,1111>
            >::type
            blue;
    typedef typename bitand_<
                shift_right<IntegralConstant,int_<16>
>
              , binary_int<0000,0000,0000,1111>
            >::type
            green;
    typedef typename bitand_<
                shift_right<IntegralConstant,int_<32>
>
              , binary_int<0000,0000,0000,1111>
            >::type
            red;
    typedef typename bitand_<
                shift_right<IntegralConstant,int_<48>
>
              , binary_int<0000,0000,0000,1111>
            >::type
            alpha;
};

The corresponding macro usage is:

    #define ARGB64_BLUE(color_int) \
    (color_int & BOOST_BINARY_LITERAL(1111))

    #define ARGB64_GREEN(color_int) \
    ((color_int >> 16) & BOOST_BINARY_LITERAL(1111))

    #define ARGB64_RED(color_int) \
    ((color_int >> 32) & BOOST_BINARY_LITERAL(1111))

    #define ARGB64_ALPHA(color_int) \
    ((color_int >> 48) & BOOST_BINARY_LITERAL(1111))

I see ample room in Boost for both libraries.  (At
least they can check each other for correctness!) 
Furthermore, the small code size and memory footprint
of the macro implementation does not mitigate the
weaknesses that macros have.  BOOST_BINARY_LITERAL may
be more efficient, but for beginners and intermediate
users at least, binary_int will be safer.

> * What is your evaluation of the implementation(s)?

As limited as binary_int is right now, it's no match
for BOOST_BINARY_LITERAL.

> * Do you think the utility should be accepted as a
> Boost library?

Not yet.  It isn't Boostified; the documentation is
not in the current format; making it MPL-friendly will
take more time than is available during this review
period--it certainly won't be as trivial as inheriting
from integral_c.  However, I see enough interest in
this utility that once these issues are resolved, I'll
probably vote for it in a second review.

                              Cromwell D. Enage

                
__________________________________ 
Yahoo! FareChase: Search multiple travel sites in one click.
http://farechase.yahoo.com