Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2011-05-03 08:11:57

Gordon Woodhull wrote:
>
> Here's again the alternate "simpler" syntax which I put in my
> original review, which I think must have gotten buried from
> typing too much.  :-/
>
> optional<int> i = string_convert<int, dont_throw>(s);  // or
> string_convert<int>(s, _throw=false) if that's impossible
> int i = string_convert<int>(s, manip_ = std::hex);

That's only a portion of the functionality of the library under review.

> >> It is incredibly clear what lexical_cast does because it
> >> uses the standard meaning of the operators, function
> >> calls, and return values.
> >
> > int i = lexical_cast<int>(str);
> > int i = convert<int>::from(str);
> >
> > When I look at the above where lexical_cast and 'convert'
> > provide the same functionality, I am not sure if from the
> > user perspectives I immediately see how 'convert' differs
> > from lexical_cast complexity-wise. I am certainly biased.
>
> It's two more tokens, and I don't see the value to the user of
> separating the two parameters.  I admit it's partly a matter of
> taste, but I think following the lexical_cast syntax as far as
> possible has objective value.

Using the new-style cast interface is certainly good.  some_cast<int>(str) can actually call convert<int>::from(str), so the other aspects of the library can be retained (in whatever form may settle out of this review) without harm.  Finding the right prefix for "_cast" will be a challenge.

> Since you say it's there for technical reasons, let's take a
> look at how Vicente fixed it in his Conversion library.  I
> think this is the relevant section of his doc:
>
> "
> How to specialize the conversion functions
> You can add an overload of convert_to and assign_to functions
> as you will do for the swap function for example, but we have
> to use a trick to allow ADL and avoid infinite recursion and
> ambiguity. This trick consists in adding a unused parameter
> representing
> the target type. E.g. if you want to add an explicit conversion
> from a type A to a type B do the following:
> namespace my_own_namespace {
>     B convert_to(const A& from, boost::dummy::type_tag<B>
> const&);
> }
> "

Don't forget the other two use cases the library supports: no exception on failure (for types with and without sentinel values) and function objects.

> > The practical reason for "::from" is two-fold --
> > 1) it clearly shows the direction of the conversion (which's
> > been deemed important during original discussions);
>
> Again, I haven't reread those discussions.  I am surprised that
> people would find a function call confusing.  Usually it takes
> its input as an argument and returns its result.  :-p

One case that occurs to me is because of the need to specify both target and source types in some cases:

   int const i(convert<int,Foo>("example"));

Obviously, that can be written like this instead:

   int const i(convert<int>(Foo("example")));

However, int a generic context, that may not be appropriate.  What if Foo is noncopyable?  Is convert<>()'s argument passed by value or reference?

Maybe I'm just trying to create a problem, but there was certainly some context that led to that concern.

> So the intention is something like this?
>
> convert<int>::result i = convert<int>::from(s, 17);
> if(!i)
>   return i.value();  // returns 17 (!)
>
> That looks pretty weird to me.  I don't see why someone would
> want to specify the default value if they are then going to
> check if conversion succeeded.

Did you read the documentation?  There's an example for this use case.  The issue arises when a type has no appropriate sentinel value to indicate the failure and is noncopyable.  In that case, one must construct an instance with a real initial state.  That instance can be passed as the fallback value (17, above).  Getting the fallback value from the result object isn't proof that the conversion failed in that case.  Instead, one must query the result object and, if the conversion succeeded, retrieve the value.

> If that's really wanted, I think simply pair<bool,T> would be
> more clear.

That imposes Copyable on T.

> Is the following syntax possible?  I am not up on
> Boost.Parameter yet.
>
> int i = convert<int>::from(s)(_dothrow);
> optional<int> j = convert<int>::from(s)(_dontthrow);

Aren't you asking for the very thing you've been against?  convert<int>::from() returns two different things depending upon context, right?  I'd also argue against requiring _dothrow and _dontthrow.  One should be the default.

> > convert<int>::result res = convert<int>::from("blah", -1);
> > if (!res) message("conversion failed");
> > int value = res.value(); // proceed with the fallback anyway.
> >
> > The locality of 'convert' application (IMHO of course) does
> > not leave much to think how 'convert' is to behave.
>
> Clear enough in itself (although I'm still reeling from the
> idea of an object which converts to false having a value).

For non-DefaultConstructible types.

> It's just that the next line might read
>
> int res2 = convert<int>::from("blah");
>
> and throw, and there's no indication why one version throws and
> the other does not.

Of course there is.  One was given a fallback value and the other wasn't.

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