Re: [boost] Numeric Conversion Formal Review

From: John Torjo (john.lists_at_[hidden])
Date: 2003-12-10 20:38:30

>   a.. What is your evaluation of the design?
good

>   b.. What is your evaluation of the implementation?
good

>   c.. What is your evaluation of the documentation?
almost good

>   d.. What is your evaluation of the potential usefulness of the library?
useful

> 
>   e.. Did you try to use the library?  With what compiler?  Did you have any
> problems?
tried with VC6 (SP5), failed.
successfully with comeau 4.3.2.
(small problem with converter_test.cpp/ line 399 - needs using std::back_inserter; )

> 
>   f.. How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
aprox 2 hours

> 
>   g.. Are you knowledgeable about the problem domain?
intermediate

> 
>   h.. Do you think the library should be accepted as a Boost library?  Be
> sure to say this explicitly so that your other comments don't obscure your
> overall opinion.
yes.

---------------------------------------------------
Details

* bounds

In the docs you say something like:

traits class bounds<N>
template<class N>
struct bounds
{
   typedef {impl-def} implementation ;

   static N lowest  () { return implementation::lowest  (); }
   static N highest () { return implementation::highest (); }
   static N smallest() { return implementation::smallest(); }
};

In the code, there's nothing called implementation, it's actually something 
called 'limits'.
Also, I'm not sure what's the benefit of having the 'limits' typedef public. I 
think it should be made

private, or else, it could be misused in client code.

* requirements.html (doc)/ UDT's special semantics

you present a list, like this: 'Target... and Source...'. IMO it should be the 
other way around

(Source... and Target...), since it's more natural this way.

Actually, I think this probably goes throughout the whole library (for instance, 
the converter class).

* (docs)

I think the converter class should be presented right after "Definitions" (and 
not being the 4th item,

as is now). This is because I think the 2nd and 3rd items are related more to 
the implementation rather

than the interface.

* conversion_traits (docs)

Not sure how you can define your own traits (no examples).

* converter (code)

I think the operator() functions should be static as well (because basically it 
calls
convert(), which is static).

* numeric_cast (code)

I love numeric_cast, but I would like something even better.
A generic numeric caster.

Something like:
template<class Traits_func,
          class OverflowHandler
          class Float2IntRounder
          class RawConverter
          class UserRangeChecker
         >
struct caster {
        template< class T, class S>
        struct convert_type { typedef ... type; }

        
        template<typename Target, typename Source>
        typename convert_type<Traget,Source>::type::result_type
        numeric_cast ( Source arg )
        {
                    return convert_type<Traget,Source>::type::convert(arg);
        }
}

This way I can have a
typedef .... custom_converter;

and in code use:
int i = custom_converter::numeric_cast<double>(d);

Also, I can use it in generic programming

template< class caster, class to, class from>
void f( from val) {
        to result = caster::numeric_cast<to>(val);
        //etc.
}

* I miss examples (libs/numeric/conversion/examples)

Best,
John