From: Guillaume Melquiond (guillaume.melquiond_at_[hidden])
Date: 2003-12-12 12:47:40
Le lun 08/12/2003 à 13:04, Thorsten Ottosen a écrit :
[...]
> a.. What is your evaluation of the design?
I think the design is good. The number of parameters of the converter
template seems a bit high. In particular, was it really necessary to
provide both a Float2IntRounder and a RawConverter parameters?
I also don't really understand the interest of having both an
OverflowHandler and a UserRangeChecker::validate_range. I would simply
remove this member since its only purpose is to use OverflowHandler (the
converter could do that itself, no need for the user to provide it).
And bounds::smallest could return 1 rather than 0 for integers.
> b.. What is your evaluation of the implementation?
As I explained in other mails, there are a few shortcomings with the
rounding from a floating-point value to an integer when rounding to
nearest.
Other than that it seems alright to me. But it is bit hard to verify a
code with a design thought to be fully meta-programing-compliant :-).
Could it be possible for the library to use another directory tree? The
root directory boost/ is already a dumping ground, it's not a reason for
the subdirectory boost/numeric/ to become one also :-). I would let only
cast.hpp in this directory (so we can #include <boost/numeric/cast.hpp>)
but the other files should be put in their own subdirectory.
> c.. What is your evaluation of the documentation?
The documentation may benefit from a bit of tidying, in particular in
the definitions. However, it is great to have such a big documentation
for such a "small" library. That's a good thing.
> d.. What is your evaluation of the potential usefulness of the library?
This library is really useful. A few languages (e.g. Ada) always check
that conversions are meaningful but C++ doesn't. Now it will be possible
in a "standardized" way. I'm a bit surprised that lexical_cast was born
before numeric_cast.
> e.. Did you try to use the library? With what compiler? Did you have any
> problems?
I only tried with GCC 3.3. I didn't encounter any problems.
On an architecture with an extended precision floating-point unit
(namely x86), the converter_test will fail on d->f because of compiler
optimizations (the use of a 10-byte floating-point temporary). It may be
better to add a volatile keyword to neutralize it (I only tested with
GCC but I suppose other compilers would probably behave similarly).
There was also a problem with udt_support_test at runtime.
> f.. How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
A few hours. The study was in-depth concerning floating-point behavior
since it's my field.
> g.. Are you knowledgeable about the problem domain?
I'm knowledgeable about computer arithmetic. I'm not that knowledgeable
when it comes to advanced C++ design.
> 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.
I think the library should be accepted: the design seems good to me and
it will be a really useful addition. However the implementation needs to
be corrected first (or the documentation should explain its
shortcomings).
Regards,
Guillaume