Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2011-05-06 07:51:45

Vladimir Batov wrote:
> > Stewart, Robert <Robert.Stewart <at> sig.com> writes:
> > ...
> > OK, that means we're down to the following now:
> >
> >  1 - default_value<T> customization point
> >  2 - converter<T,S> customization point, main logic, functor
> >  3 - T convert_cast<T,S>(S, formatting = none); can throw
> >  4 - T convert_cast<T,S>(S, T, formatting = none)
> >  5 - bool try_convert_to<T,S>(S, T &, formatting = none)
> > ...
>
> #3, #4. I do not think these can cause any controversy
> (although people who refuse reading documentation might be
> surprised by #2 throwing or #3 not throwing ;-) ).

I don't care too much if they can't read the documentation far enough to learn that much.

> #5 IMO can. It deploys the Pascal-style parameter passing and
> modifications. I remember reading Stroustrup (I think) long
> time ago advising against passing non-const references and I
> personally agree. That's due to potential confusion and wrong
> expectations. I am not aware of any function in std and boost
> doing that. Introducing such a precedent might be a hard-sell.
> Especially confusing might be the fact that #4 does not modify
> the second parameter when #5 does.

I know some folks don't care for output parameters but they are useful.  As for #4 not modifying and #5 modifying the second parameter, the names are distinct, so distinct behavior should not be surprising.

> std::pair<T, bool> try_convert_to<T,S>(S, T, formatting = none)
>
> is far less-controversial and even familiar

That cannot be used as straightforwardly:

int i(3);
if (!try_convert_to<int>("1", i))
{
   // report failure
}
// i == 3

versus:

std::pair<int,bool> maybe(attempt_to_convert_to<int>("1", i));
if (!maybe.second)
{
   // report failure
}
else
{
   // what's i's value?
}

Note that I changed the name.  The "try" prefix, as noted by Vicente, implies returning a bool, not a pair.

> (even though I personally dislike std::pair due to
> unreadability of the resulting code sprinkled with faceless
> first and second).

I agree that a dedicated type, with names for the two fields, would be better.  Furthermore, that avoids the chance for reversing the parameterizing types:

std::pair<bool,int> bad(attempt_to_convert_to...); // Mismatch!

> Could I suggest convert_result as a more palatable
> alternative? :-)

That would have to be convert_result<int>.

> #1. I am personally not thrilled with default_value because it
> looks to me like additional/avoidable/arbitrary piece of
> machinery that I have to implement to incorporate my class into
> the framework. I've been managing without it. I'd expect a new
> design not to force me to work harder.

That's based upon the idea that default constructors and zero-initialization are available for most types one would use this library to convert.  However, I note that you replied elsewhere that relatively few classes in your environment have default constructors.  That very clearly is a sore spot in this design.

Vicente's desire for uniformity, through a generic interface, is laudable.  Using a CP like default_value permits that.  Having to specialize default_value for most of your classes would be annoying, I agree, but consider that the alternative is to explicitly create an initial value for every conversion call you make.

> Additional note. While you are on the interface, it probably
> needs to be kept in mind that the current (as I understand it
> anyway) Boost policy is to avoid introducing content directly
> in the boost namespace.

That's an excellent point.

> Therefore, I presume fully qualified it'll be something like
>
>     int i = boost::conversion::convert_cast<int>(str);

A namespace alias or using declaration or directive may be used to overcome such a verbose name, of course.

> So, maybe you might consider shortening it to something like
>
>     int i = boost::convert::cast<int>(str);

With a using directive, that would end up just being cast<int>(str), which is unhelpful.

>     int i = boost::convert::to<int>(str);

That's good.

>     int i = boost::convert<int>::from(str); // Just kidding.

I've seen that somewhere before.  Now, where was that?

> because otherwise the user will be implicitly forced to
>
>     namespace cvt = boost::conversion;
>
> or worse
>
>     using namespace boost::conversion;

A user may prefer to avoid the verboseness of "boost::conversion::convert_cast," but there's no implicit force.  Given that "convert_cast" is a nice, descriptive name, I'd be likely to use a using declaration.

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