Re: [boost] [review] string convert

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

Hi Vladimir,

I'm sorry to hear you've withdrawn Boost.Convert.

I hope you'll continue to participate in the conversation about replacing or improving lexical_cast.  

You brought a lot of interesting ideas and it's always helpful to have a concrete proposal for people to tear apart!  :-/

On May 3, 2011, at 8:36 AM, Vladimir Batov wrote:

>> Gordon Woodhull <gordon <at> woodhull.com> writes:
>> ... Anyway, I remain in favor of the
>> library with simplified semantics.  It only boils down to three things I find
>> distressing.
> 
> I am not sure it's not 'distressing'. More like something you are not
> immediately comfortable with, right? ;-) I certainly do not want anyone
> distressed over something I suggest.

:-)  

> From 
> 
> convert<int>(std::string)
> 
> it's not immediately clear if we convert int-to-str or str-to-int. So, more
> explicit direction was decided would be useful/helpful. Reading 
> 
> convert_to<int>(str)
> or
> convert<int>::from(str)
> 
> is more natural to read and easier-flowing and kind of self-documenting (I feel
> that way anyway).

Yes, something seems ambiguous about convert<> on its own.  But I'd prefer a longer name than these "prepositional phrases" - although I have to admit there is something neat about them!

> For better or worse my usual processing flow of configuration parameters:
> 
> // Try reading a configuration parameter.
> convert<int>::result i = convert<int>::from(s, 17);
> // Log error if parameter was bad.
> if(!i) message ("parameter ignored. default used");
> // Proceed with the default.
> return i.value();  // returns 17 (!)

Aha, I see now.  I still think that's a pair, see my next message.

> 
>> If that's really wanted, I think simply pair<bool,T> would be more clear.
> 
> Well, so far you suggest boost::optional in one place std::pair in another. I
> feel that providing one convert::result is more polished and easier to grasp. I
> personally is botherd by std::pair<iterator, bool> from std::map::insert().
> Equally I am not thrilled about pair<bool,T> in this context... more so because
> it'll probably be pair<bool, boost::optional<T> > -- something convert::result
> encapsulates.

Whoah!  No, there is only one failure to record, there should be no need for two bools.  

If conversion succeeded, then we have a value.  

If conversion failed, then the user has chosen to throw, to use a default, or use an optional.  Further, the user can choose when using a default, whether they want to know whether they're getting that value because of a conversion failure.

IIUC your convert<T>::result would be in the state {false, empty} if conversion failed and no failback was specified.  However, there is no need to remember whether a failback was specified inside the function which took it or didn't.  :-)

In other words, just return the appropriate structure based on the request type tag.  There is no need to have a one-size-fits-all result type.

Or, equivalently, have different names for different behaviors instead of overloads.  I'm not really opposed to that, but I guess I would prefer the overloads if there are going to be more behaviors and especially if behaviors could combine to produce yet more return 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);
> 
> Yes, it is possible. However, in #1 (_dothrow) is stating the obvious.

Yes _dothrow would be the default, unless there's a failback value.  It would explicitly be needed in one bizarre case, though.  Suppose the value is not defaultable, but you still want convert to throw.  So you have to supply a failback but don't want it returned...  Pretty uncommon, but it would be annoying if you needed it and it wasn't there.

> So, essentially we are talking about different deployment/API of the same
> functionality. 

Yes that is the essence of my review.

>> I think Boost.Lambda/Boost.Phoenix is pretty well-known syntax by now, but
>> perhaps I've been indoctrinated.
> 
> Nop. Tried both but was happier with boost::bind and boost::regex. Maybe due to
> my use-cases, maybe due to something else. ;-)

Boost.Bind is monomorphic.  I believe Phoenix Bind would work with the convert_cast in my next message.

>> I guess your syntax is a bit like currying - drop an argument and it becomes
>> a function object which takes that
>> argument.  Don't think I've seen that in C++ before.
> 
> Could that be a good thing? :-)

Yes! but Lambda & Bind are the normal way to do it.

> Thanks for all your input. It's much and truly appreciated. 

It's been fun!  
Gordon