Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Matthew Chambers (matt.chambers42_at_[hidden])
Date: 2011-05-04 12:58:36

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.
>
> 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.

> 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. 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;
> }
>
> 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();
> }

OK, I agree that the optional return values do not conform to the cast semantics, i.e. it should 
only "cast" to what you tell it to. So either we should drop the _cast suffix for these variants or 
we specialize for optional<T>. The latter is probably disagreeable since some people might want to 
make their own specialization (someone mentioned optional<T> <-> string conversion earlier).

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. 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!).

string s = "4-2";

// Matt likes to throw:
int i = convert_cast<int>(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<int> i = optional_convert<int>(s);

Note that in several of these, target typename is no longer needed, right?

For non-defaultable types, I am now seeing the elegance of Vincente's default_value template 
parameter. It wouldn't be needed for the try_convert variant of course. But it would eliminate this 
distasteful overload:
optional<non_defaultable_type> i = optional_convert<non_defaultable_type>(s, 17);

With this setup, is there any reason that convert_cast and optional_convert couldn't just be thin 
wrappers around try_convert?

-Matt