[boost] Re: Numeric Conversion Formal Review

From: Daryle Walker (darylew_at_[hidden])
Date: 2003-12-12 01:25:30

On 12/8/03 7:04 AM, "Thorsten Ottosen" <nesotto_at_[hidden]> wrote:

[SNIP]
> "The Boost Numeric Conversion library is a collection of tools to describe
> and perform conversions between values of different numeric types.
> 
> The library includes a special alternative for a subset of
> std::numeric_limits<>, the bounds<> traits class, which provides a
> consistent way to obtain the boundary values for the range of a numeric
> type.

This is OK.  (Your example in the docs may want to have also an "int"
example to show why "bounds" is needed.)

There are other holes in "std::numeric_limits", but those can be handled
later.

> It also includes a set of trait classes which describes the compile-time
> properties of a conversion from a source to a target numeric type. Both
> arithmetic and user-defined numeric types can be used."

These are OK.

For the tools that actually do conversions: some of the names are (still) in
InterCaps style; maybe they should be changed to match the_usual_style.
There is an improved "numeric_cast" that uses the new conversion helpers,
but only in the default mode.  Maybe there should be a variant of
"numeric_cast" that takes a conversion helper class that has some of the
template parameters overridden.

> If you intend to make a review, then please read all of this mail.
> 
> 1. The library can be downloaded in the files-section at yahoo (the file is
> called "numeric_conversions.zip")
> 2. Fernando and I are aware that the documentation could be more "tidy", but
> we don't think it will hinder any serious review.
> 3. Please try to work with Fernando if there arises compiler problems etc.
> Even of some compiler problems cannot be overcome,
>   please try to focus on a useful subset of the functionality
> 4. Please follow the guidelines described at
> http://www.boost.org/more/formal_review_process.htm#Comments, in particular,
>   remember to answer these (it makes it a lot easier to be a review
> manager)

> a.. What is your evaluation of the design?
> 
> b.. What is your evaluation of the implementation?

These are fine.

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

As you said, it needs some more work.  The gist of the content is done very
well.  (I should reject the submission for misspelling my name, but I'll be
nice.  -_^)

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

This should be useful.

> e.. Did you try to use the library?  With what compiler?  Did you have any
> problems?

I using a G4-equipped Mac with Mac OS X 10.2.8.  With the (Apple special)
GCC 3.1, the test programs ran fine.  With Metrowerks CodeWarrior Pro 8.3, I
couldn't compile.  The method the author used to sneak in the "floor",
"ceil", and "log10" screws stuff up.

For converter_test:

//=======================================================================
Error   : ambiguous access to overloaded function
'std::log10(float)'
'std::log10(float)'
unit_test_result.cpp line 146   tatic_cast<std::size_t>( std::log10(
(float)std::max( num_passed, num_failed ) ) ) + 1

Error   : ambiguous access to overloaded function
'std::floor(double)'
'std::floor(double)'
test_helpers.cpp line 75     return MyFloat ( std::floor(x.to_builtin()) ) ;

Error   : ambiguous access to overloaded function
'std::ceil(double)'
'std::ceil(double)'
test_helpers.cpp line 80     return MyFloat ( std::ceil(x.to_builtin()) ) ;

Error   : ambiguous access to overloaded function
'std::floor(double)'
'std::floor(double)'
converter_test.cpp line 498           double s1 = floor(dv1b + 0.5);

//=======================================================================

For udt_support_test, bounds_test, and traits_test: only the first three
errors given above

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

A quick reading and a try-out of the test programs.

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

Hopefully.  (It's basic C++ with a little math/logic thrown in.)

> 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.

> best regards

-- 
Daryle Walker
Mac, Internet, and Video Game Junkie
darylew AT hotmail DOT com