Re: [boost] [review][constrained_value] Review of Constrained Value Library begins today

Subject: Re: [boost] [review][constrained_value] Review of Constrained Value Library begins today
From: Thorsten Ottosen (thorsten.ottosen_at_[hidden])
Date: 2008-12-04 17:30:33

Hi Robert,

Jeff Garland skrev:
> Hi all,
> 
> The review of the Robert Kawulak's Constrained Value library begins 
> today December 1, 2008, and will end on December 10th -- I will be the 
> review manager.  Please post reviews to the developer list.

I'm basing this review on reading the docs, mostly.

1. I don't like that the exeption object broken_contrait takes an 
std::string argument since that may throw std::bad_alloc. I'm fine with 
that in my own code, but I think libraries should try to avoid it. See also

http://www.boost.org/community/error_handling.html

2. I prefer functions like

   change_constraint(c, is_positive());

to be members. We had similar discussion for some function in the Bimap 
review. Basically, it is nicer for the user to press . to get a list of 
functions than to search the boost documentation for the right function.

This comment applies to all the free-standing functions.

3. In this code:

ignoring_even_int j(1); // exception! (cannot ignore construction with 
invalid value)

is there no way to get an assertion instead of the exception?

4. Is your library powerfull enough to implement something like

http://biology.nmsu.edu/software/probability/index.html

? If not, what would it take to make that possible?

5. The docs say

"Constrained objects can be used as a debugging tool to verify that a 
program operates only on an assumed subset of possible values. However, 
it is usually desirable to check the assumptions only in debug mode and 
avoid the checks in release code for performance reasons"

Have you made any benchmarks? In particular, I would be interested in 
those for bounded_int or bounded_float. 
(bounded_float/bounded_double,bounded_unsigned etc. are provided, no?)

6. My main use case would be to use constrained floating point values, 
but the docs state:

"Why C++'s floating point types shouldn't be used with bounded objects?
[...] there exist a number of myths of what IEEE-compliance really 
entails from the point of view of program semantics. We shall discuss 
the following myths, among others: [...] "If x < 1 tests true at one 
point, then x < 1 stays true later if I never modify x."

I don't fully understand this...AFAIK, this is not allowed for an IEEE 
complient implementation. How could we ever implement *anything* with 
respect to floats then?

Anyway, here's my review vote:
*************************'****

I think this library should be accepted into Boost provided that minor 
changes are done. The docs are nice and clear, and the library gives 
some quite interesting examples of how powerful the library is. Nice work!

I do think that the above issues should be thought really hard about, 
especially 1,2,3 and 6. If to is not possible, I change my vote to no.

best regards

-Thorsten