[boost] Re: Numeric Conversion Formal Review

From: Fernando Cacciola (fernando_cacciola_at_[hidden])
Date: 2003-12-11 18:08:20

Hi John,

Thanks for the review!

> ---------------------------------------------------
> 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'.
>
hmmm... the word "implementation" is intended to reflect the fact that this
is an implementation detail.
I think I better change it to something like:

traits class bounds<N>
template<class N>
struct bounds
{
    static N lowest  () { return <i>implementation_defined_value</i>; }
    static N highest () { return <i>implementation_defined_value</i>; }
    static N smallest() { return <i>implementation_defined_value</i>; }
 };

> 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.
>
Good point!

>
>
> * 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).
>
The order Target,Source is taken from the neccesary ordering of template
parameters of casting _functions_, in which the source type is resolved by
the compiler so only the target type is explicitely given. Even though this
is not a function but a class (so both types must be explicitely given), I
decided to follow the familiar ordering used by the casting functions found
elsewhere in boost.
That ordering is used througout all the library, including documentation,
for the sake of consistency.

> * (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.
>
Good point.

> * conversion_traits (docs)
>
> Not sure how you can define your own traits (no examples).
>
Noted. I'll add a section dedicated to this.

>
>
> * converter (code)
>
> I think the operator() functions should be static as well (because
basically it
> calls
> convert(), which is static).
>
Thorsten already commented on this (thanks Thorsten!)

>
> * 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.
> }
>
Hmmm... I couldn't quite follow... can you elaborate more? Thanks.

> * I miss examples (libs/numeric/conversion/examples)
>
Yes... I'm working on it, and also on introductory sections for the various
library compomnents.

Best regards,

Fernando Cacciola
SciSoft