Subject: Re: [boost] [review][constrained_value] Review of ConstrainedValueLibrary begins today
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2008-12-10 14:04:51
----- Original Message -----
From: "Robert Kawulak" <robert.kawulak_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Wednesday, December 10, 2008 4:22 AM
Subject: Re: [boost] [review][constrained_value] Review of ConstrainedValueLibrary begins today
>> * I don't know if you will add monitored values to your
>> library. IMO it will be better to have them in a separated
>> class.
>
> Or, if it's reasonable, implement constrained objects in terms of monitored
> objects.
Yes, this seam OK. A constrained object can be seen as a monitored one.
>> * The operators have not always a sens for the application.
>> For example,
>> constrained<int, is_even> x;
>> ++x;
>>
>> I would prefer to have a compile error in this particular
>> case instead of an exception. It will be quite interesting to
>> show how a user can catch this error at compile time and
>> avoid this runtime error.
>
> Passing over presumable complexity of the implementation, this would be quite
> inconsistent behaviour. Why "++x" should behave in a different way than "x +=
> 1"?
I'm not said that.
> What if the user wants this to be a runtime error or wants to use the object
> in a generic function (wich would contain the "++x" expression, but not
> necessarily invoke it)?
I'm asking that the user must be able to choose if he wants arithmetic operators or not with a parameter with_arithmetic_operators.
>> template<typename V , typename C , typename E >
>> constrained< V, C, E , with_arithmetic_operators> &
>> operator++ (constrained< V, C, E,with_arithmetic_operators> &c)
>>
>> the bounded hierarchy should have with_arithmetic_operators.
>
> Then for the even object we should also ban, e.g., *=, although it perfectly
> makes sense?
It should be up to the user to choose if he want arithmetic operator for his
constrained<int, is_even, throw_exception<>, with_arithmetic_operators>
>> * constrained must be default constructible. The library
>> should provide a mean to specify a default value. E. g.
>> constrained<default<int, 1>, is_odd >
>
> I think this is too general utility to belong to this library (it's similar to
> value_initialized in Boost.Utility).
Maybe, this could be a general utility, but your library make some constrained values not default constructibles. What the user can do waiting for this utility? I would like to see a class odd that is default constructible in the documentation.
>> * The default value for excluding the bound is false, which
>> mean that the bound is included. There is no example of
>> excluding bounds. Here is one
>
> What do you exactly mean by saying "there is no example"? The tutorial contains
> a section titled "Bounded objects with open ranges".
>
>> I think that it will more clear if instead of stating
>> exclusion we state inclusion, and the default be true,
>>
>> So we can write
>> typedef static_bounded<int, 0, 100, throw_exception<>,
>> false, true>::type t;
>
> A value-initialised bool has the value of false. Therefore, bounds exclusion is
> used rather than bounds inclusion, so the default is always "bounds included"
> when the bounds inclusion indicators are default-constructed.
Why do you talk about value initializetion. The parmeter have already a default value. It is enough to change the meaning of the boolean parameter.
>> * I would like the library manage constraints between several
>> variables, e.g the range of valid days values depends on the
>> month. We can need to set/get each varaible independently or
>> all at once. The concept of tuple seems interesting.
>
> Indeed, sounds interesting, but I wonder if it would be needed frequently enough
> to make this part of the library.
>
>> The current interface could also be used as
>>
>> typedef constrained<tuple<day, month>, adapt_day_to
>> _month_constraint > date;
>>
>> but we can not assign the value independently.
>
> That's right. What I'd rather do is to define a simple class containing the
> constrained day and month, and providing functions to get/set the members. The
> setter for the month would additionally adjust the upper bound of the day.
It is OK for me. Could you add this example please?
>> * The number of parameters of the bounded class is already to high.
>
> Most of them (ordered from most to least used) have default values, so I don't
> think this is a serious problem. Your solution reduces them from 7 to 5, which
> may be seen as not much... However, it looks interesting.
>
>> Even in this case the bool values are not enough declarative.
>> It would even be beter to define two templates, open, close
>> (see below)
>> typedef static_bounded<int, open<0>, close<100> >::type t;
>
> I don't know if this is an optimal solution if for the most common case, when
> the bounds are included, you have to write more than:
>
> typedef static_bounded<int, 0, 100>::type t;
When I read static_bounded<int, 0, 100> I need to know which are the defaults to know if the bound are in or not. When a read static_bounded<int, open<0>, close<100> > there is no issue. It is explicit. What is more readable |1,100| or [1,100], (1,100)?
>> Adding more examples will be welcome as:
>> * Application domain classes using (by inheritance and by
>> containement) a constrained class to see what the user needs
>> to do when it needs to add some methods, and don't want some
>> inherited methods.
>
> I don't know if such example is indeed needed, sounds a bit like showing how to
> derive from std::string...
Perhaps. But your library adds a lot of arithmetic operators and I would like to see in the documentation how a user can use the constrained class when he don't want these operators to be defined.
>> *Date type would show the change upper bound at runtime, and
>> see how the values of a variable (month) impact the
>> constraints on another variable (day).
>
> Might be a nice example, let me consider this.
I think this example is a must. Every one think on the Date data type when talking abount constrained values.
Regards,
Vicente