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: Stjepan Rajko (stjepan.rajko_at_[hidden])
Date: 2008-12-07 19:20:03

On Sat, Dec 6, 2008 at 10:31 AM, Stjepan Rajko <stjepan.rajko_at_[hidden]> wrote:
>
> One thing I want to mention is that there is a slightly higher
> abstraction that this library almost addresses, what can maybe be
> called "monitored values" - values where you need to do something when
> the value changes (or is first constructed).  In the case of this
> library, "something" is checking a constraint and calling a policy if
> the constraint is violated.  In other cases, it might be calling a
> boost.signal, in another, writing to a log.
>
> The author might consider (eventually) altering the design of the
> library to offer this higher level of abstraction, as he already
> solves the problem of hooking into a value change reasonably well (so,
> in addition to constrained and bounded classes, offer a monitored
> class using which constrained is implemented).  This would allow much
> wider use of the library.  (I don't know if there are requirements for
> a "MonitoredValue" library that are significantly different / in
> addition to what the current libraries, that I'm not thinking of at
> the moment)
>

I am rethinking this suggestion.  If the library ends up separating
the invariant from the test, then the following would provide support
for a purely monitored value with the implementation the library
already offers:
* "set" the invariant to none (i.e., guarantee nothing additional to
the invariant offered by the underlying type)
* use a test that always returns false
* use a policy class that does whatever the task of the monitoring is
(e.g., send a boost::signal, write to a log, ...)

I think I like that better than my original suggestion, because you
can now also have conditionally monitored values (i.e., you only want
to send a signal or log in certain circumstances).

This is convincing me even more that the current design of the library
is spot-on, with the exception of separating the current "constraint"
into separately considered "invariant" and "test" (or whatever terms
might fit better).  I strongly propose the following structure (a
reiteration of what I've already posted):

* there is an invariant, which is only documented
* there is a test which is implemented - a passing test guarantees the
invariant holds (and will remain holding until the object is
explicitly changed), and a failing test causes the policy to get
invoked
* the policy guarantees that the invariant will hold when/if it
returns (and can change the test / invariant if it wants to, where a
change in the invariant again only occurs in the documentation of the
behavior)

The above treatment covers the current behavior of the library, allows
dealing with issues such as the floating point case, *and* support for
purely monitored values.  AFAICT, It requires no changes to the
implementation.

Stjepan

>> - What is your evaluation of the implementation?
>
> I looked at the implementation of the constrained class, and found it
> very satisfactory.  Well implemented, well documented code that was a
> pleasure to review.
>
>> - What is your evaluation of the documentation?
>
> Great as a tutorial, and great as class / function reference.
>
> The only part I found lacking is the discussion of exact requirements
> on the underlying value type / constraint (my previous posts elaborate
> on this).  Robert - I think the notion of "spontaneous" changes of the
> constraint extends beyond the floating point case and perhaps be used
> to discus other "spontaneous" changes or changes beyond the control of
> whatever the library has control over (like the use_count of a
> shared_ptr).
>
>> - What is your evaluation of the potential usefulness of the library?
>
> Very useful.  Beyond the constrained and wrapping built-in types, the
> documentation suggests some very creative uses of the library, and the
> flexibility of the library promises a lot more.  When it can take
> advantage of declspec, the usefulness will further increase even more.
>  Lambda might kick it up a notch as well (Robert, in my suggestion to
> provide syntactic sugar support for the calling of a f(value_type &)
> function, my unstated point was that f could be any callable -
> including a lambda expression).
>
>> - Did you try to use the library?  With what compiler?  Did you have any
>> problems?
>
> Nope.  No problems :-)
>
>> - How much effort did you put into your evaluation? A glance? A quick
>> reading? In-depth study?
>
> I read through the documentation a few times, followed the discussion
> on the list, and looked at the implementation of the constrained
> class.
>
>> - Are you knowledgeable about the problem domain?
>>
>
> My experience in using functionality related to the library consists
> of  inserting asserts here and there, and manually wrapping values
> when they need to wrap. I recall plenty of situations where having a
> library like this would have been wonderful.
>
>> Please state in your review, whether you think the library should be
>> accepted as a Boost library.
>
> I believe the library should be accepted.  I found the library quite
> feature-complete for its domain (in reading the docs, every time I
> found myself thinking "this library should support x", a few
> paragraphs down I would find out it does), while preserving simplicity
> and elegance.
>
> Some of the things I would definitely like to see in the final version are:
>
> * document exact requirements on the value, constraint and policy
> types, and how they relate to library's guarantees
> * the library should *legally* allow for a reasonable floating point
> solution (it would be great if it actually provided one, but allowing
> one would be good enough for me)
> * (seconding suggestions by other reviewers): provide tests (including
> sizeof tests)
>
>
> Nice work, Robert!
>
> Stjepan
>