Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Gordon Woodhull (gordon_at_[hidden])
Date: 2011-05-04 09:01:05

Hi Robert,

Ok, let me try again with what I've learned in the past 31 hours.

Thanks for challenging me to make specific proposals!  I think I was emptily complaining a bit there.  Here's another revision of my idea of a good interface: Vladimir's interface squished.

On May 3, 2011, at 8:11 AM, Stewart, Robert wrote:
> 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.

So, here's something complete (?) for people to rip apart:

int i = convert_cast<int>(s); // default behavior: might throw
optional<int> i = convert_cast<int>(s, use_optional_instead_of_throwing_);
int i = convert_cast<int>(s, 17); // won't throw; can't tell if conversion successful
int i = convert_cast<int>(s, 17, throw_even_though_i_specified_a_failback_);
optional<int> i = convert_cast<int>(s, 17, use_optional_instead_of_throwing_);
pair<bool,int> i = convert_cast<int>(s, 17, fallback_and_report_success_);

I am not concerned with names, just with syntax.  So I'm using "convert_cast" although I actually don't care what it's called, and I have some very ugly very descriptive tag values with distinct types to trigger different overloads, which might be abbreviated dont_throw_, do_throw_, and, um, tell_fallback_ or something.

All would also accept extra manip_/format_= and locale_ arguments as in Vladimir's proposal, except they have to be appended to the first parameter list because of the "no proxies" requirement. 

Doesn't this now cover all the functionality we've been talking about?

The 5th and 6th forms cover nondefaultable types / types with no sentinel value, and Vladimir's "print a warning if failing back" use-case.

You mention noncopyable types but I'm not able to find any reference to that in the documentation or in previous discussion.  I don't see how the value can be non-copyable since it's being returned by the function.

I'd actually prefer the behavior flags were put in the template parameters, because they affect the return type, but that would be tricky because of the defaulted Destination type.  (I don't think it's impossible, but it might be messy.)

Maybe just having more functions would be simpler; I'm just taking an idea to its conclusion. 

Since there are now a bunch of overloads with different behavior, and no converter<> object, I figure these would dispatch to a customization point function that looks a lot like Vicente's, but probably with a default an extra bool parameter indicating whether to throw.

> Finding the right prefix for "_cast" will be a challenge.

Sigh, another naming debate.  (Naming is important but so difficult!)  It does seem like the direction that *_cast<>() is clearer to people than convert<>()

I also like as<>()  :-)  but somehow I don't think that'll generate consensus.

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

Right, I should have reread.  I got it now, thanks.

I think I've addressed this use-case now.

I'm still not sure what you mean by noncopyable and hope that you mean nondefaultable.

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

Again, I'm confused by this requirement for support of noncopyable types.

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

No, it's not about the context, it depends just on the parameters.  Different overloads.

But as people have convincingly argued, the whole proxy object has to go.  So those argument lists get combined, and there would have to be some tricky overloading.  I think Boost.Parameter can handle this, if I understand it correctly.  It might still be pretty messy, but the customization point would not have to be.

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

Okay my example should have read

convert<int>::result i = convert<int>::from(s); // doesn't throw
int i                  = convert<int>::from(s); // could throw

Changing behavior based on arguments is okay; depending on context is too EDSL and not compatible with the many, many places this function will get used.

Note: I don't think I'm volunteering to write convert_cast, just putting something out for people to tear apart.  In fact, I should really be working on my BoostCon talk.  :-D

Cheers,
Gordon