Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Vicente BOTET (vicente.botet_at_[hidden])
Date: 2011-05-04 12:05:59

> Message du 04/05/11 17:05
> De : "Barend Gehrels" 
> A : boost_at_[hidden]
> Copie à : 
> Objet : Re: [boost] [review] string convert
> 
> Hi Robert,
> 
> On 4-5-2011 16:33, Stewart, Robert wrote:
> > Barend Gehrels wrote:
> >
> >> int i = convert_cast(s, 17);
> >>
> >> // template parameter as optional one there
> >> int i = convert_cast
> >> <
> >> int
> >> , throw_even_though_i_specified_a_failback_
> >> >(s, 17);
> >>
> >> optional i ... (similar as other one with optional)
> > I presume you mean the following, but why?
> >
> > optional i = convert_cast
> > <
> > optional
> > , throw_even_though_i_specified_a_failback_
> > >(s, 17);
> >
> Good question - I included it because it was in the original list of 
> Gordon, but I actually also wonder what is the specific usage of this 
> function.
> 
> Gordon?
>
> > convert_cast(s, 17) works pretty well (maybe "throw_on_failure").

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.

> > Even better:
> >
> > auto r(try_convert_cast(s));
> > if (r)
> > {
> > i = r.get();
> > }
> >
> > Notice also that the precedent, std::map::insert()'s return type, puts bool second, not first. I never remember the order and you used the reverse. That leaves too much room for confusion. The optional version is much better.
> 
> I agree completely. And indeed this usage with auto is convenient. One 
> thing, users using it like this will not notice anymore that "auto" is 
> (behind the screens now) an "optional", and might be surprised by the 
> .get() call.

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

Best,
Vicente