Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2011-05-04 14:18:03

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.

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

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

> > 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<optional<T>>(S) than optional_convert_cast<T>().  It indicates what is happening better.

> 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<optional<T>> a viable candidate:

int i;
if (try_convert_to<int>(s))
{
   // use i
}

auto const c(convert_cast<optional<int>>(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<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?

try_convert(s, i) doesn't tell me whether s is being converted to i's type or i to s's type.

convert_cast(s, 17) has the same problem and breaks with the new-style cast pattern.

The target type should be required in all cases, I suspect, to make things clearer:

try_convert_to<int>(s, i) clearly indicates that int is the target type.

convert_cast<int>(s, 17) is likewise better.

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

Perhaps, but there's still too much churn to think about that yet.

_____
Rob Stewart                           robert.stewart_at_[hidden]
Software Engineer                     using std::disclaimer;
Dev Tools & Components
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.