Boost logo

Boost :

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


> Message du 04/05/11 20:19
> De : "Stewart, Robert"
> A : "'boost_at_[hidden]'"
> 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


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk