Message du 04/05/11 20:19 De : "Stewart, Robert" A : "'boost@lists.boost.org'" Copie à : Objet : Re: [boost] [review] string convert
Matthew Chambers wrote:
On 5/4/2011 11:05 AM, Vicente BOTET wrote:
I think we need to clarify one thing. Vladimir library uses values for two purposes: * as a default value when the type is not default constructible * as a fail-back in case of the conversion fails
And I think we should mix them. To cover the first case we can use as I said in another post a default_value metafunction that can be specialized for the non default constructible type.
Presumably you meant we should *not* mix the two uses.
Yes.
I agree that when a fail-back is given the user is not interested in knowing if the conversion succeeded or not, so in this case the return value should be T and not optional T. The question now is what function should be used, convert_cast or try_convert_cast. As the function doesn't throw I will use try_convert_cast, but as the function returns type T I will use convert_cast.
I disagree. I think the fallback with conversion success is a reasonable use case. Vladimir's case of notifying the user when a fallback value is being used is reasonable. It's difficult to leave that logic outside the conversion because only the conversion knows if and why the input is invalid.
I agree with Matt.
I don't see the added value then. If the user needs to check if the conversion succeeded, why not setting herself the fail-back int i; if (! try_convert_to(s,i)) { i = fail_back; }
Let me comment a little more on the function try_convert_cast returning optional T. The function can not be used directly where the target type T was expected so we can not consider it to follow the cast pattern.
You're right.
Other try_ functions return just bool. If we follow this pattern the preceding code could be written as
int i; if (try_convert(s,i)) { // do whatever you want with i; }
Yeah, I think that's right, too: a "try_" function should return bool. However, the name should be "try_convert_to" if you go that route.
OK.
If you want to preserve the convert_cast that returns a optional T, I will prefer to name it optional_convert_cast, so the user that will read it will be advertised that the result is an optional T.
auto r(optional_convert_cast(s)); if (r) { i = r.get(); }
I'd much rather see convert_cast>(S) than optional_convert_cast(). It indicates what is happening better.
Yes, this can be equivalent and doable.
If others agree, I could also go with Vincente's version of try_convert. It doesn't bother me that passing in the variable by reference makes it a two-liner, since the expression itself can go inside the if statement and it implicitly supports non-default-constructable types.
It is certainly non-surprising.
Using Vicente's default_value customization point still makes convert_cast> a viable candidate:
int i; if (try_convert_to(s)) { // use i }
You missed the i parameter the call to try_convert_to.
auto const c(convert_cast>(s)); if (c) { // use c.get() }
Note that try_convert_to() makes for simpler, shorter, and more direct code and that both are two-liners.
And we can also provide optional_convert, which CAN be one-lined (if the user doesn't care about conversion success, and if they DO, then they should use the try_ version!).
Offering both is possible, and depends upon the other functions in the set, though generally, offering two ways to do something can be a source of confusion.
string s = "4-2";
// Matt likes to throw: int i = convert_cast(s);
// Vladimir cares about success: int i = 17; if (!try_convert(s,i)) { /* log that fallback value is being used */ }
// except when he doesn't (but he never throws) int i = convert_cast(s, 17);
// Vincente thinks success is optional: optional i = optional_convert(s);
Note that in several of these, target typename is no longer needed, right?
try_convert(s, i) doesn't tell me whether s is being converted to i's type or i to s's type.
Is try_convert_to(s,i) better, or you see the same problem. Do you think it is better to use named parameters _from_ and _to?
convert_cast(s, 17) has the same problem and breaks with the new-style cast pattern.
would you prefer to use a _failback named parameter? convert_cast(s, _failback=17) or rename the function if_fail_conversion_cast(s,17); I don't really like the incidence on the interface/design of the failback feature. As no option satisfy completely I will just do int i; if (! try_convert_to(s,i)) i = 17; If try_convert_to doesn't updates the target variable if conversion fails we can even write int i=17; try_convert_to(s,i);
With this setup, is there any reason that convert_cast and optional_convert couldn't just be thin wrappers around try_convert?
Yes, this seems a good implementation option.
Perhaps, but there's still too much churn to think about that yet.
Yes, we need first to try to fix the interface. Best, Vicente