Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Vicente BOTET (vicente.botet_at_[hidden])
Date: 2011-05-09 18:12:01

> Message du 09/05/11 23:34
> De : "Stewart, Robert" 
> A : "'boost_at_[hidden]'" 
> Copie à : 
> Objet : Re: [boost] [review] string convert
> 
> Vicente BOTET wrote:
> > De : "Stewart, Robert"
> >
> > > To summarize, here's the complete API as I see it now:
> > >
> > > 1. convert::default_value customization point
> > > * Default value, if needed, when not otherwise supplied
> >
> > I don't see this customization point exclusive for conversions.
> > I will move it to the boost namespace.
> 
> I don't know about that. If a type has a globally applicable default value, then it will have a default constructor (or be zero-initializable). I see default_value as specific to conversions.

You can have a 3pp class that has not defined a default constructor, but in your application you could find one. In any case, default_value is useful only when stream conversions are concerned.

> > > 2. convert::converter customization point
> > > * Non-stream-based main logic
> > > * Functor interface
> >
> > Do you mean that the default behavior is a call to the
> > conversion operator?
> 
> The default behavior is to rely on T(S) or T = S. Specializations of converter can do things differently.

OK. Boost.Conversion customization point is based on overload of the convert_to function. Are you proposing I change this? What are the advantages?

> > > 3. T convert::convert_cast(S)
> > > * Throws on failure unless T is convert::result
> > > * Uses convert::converter to do conversion
> >
> > I prefer conversion::convert_to.
> 
> That looks every bit like a cast. Why not name is so? Still, if there are to be parallel namespaces (see below), then convert_to() would be good.
> 
> > > 4. T convert::convert_cast(S, T _fallback)
> > > * Returns _fallback on failure unless T is convert::result
> > > * Uses convert::converter to do conversion
> >
> > I prefer conversion::convert_to_or.
> 
> Overloading is a lot cleaner to my eyes and, of course, I don't like your name as I've noted.

OK. I could remove the _or suffix and use overloading if we don't find a better name.

> > > 5. bool convert::try_converting_to(S, T & _value)
> > > * Returns false and leaves _value unchanged on failure
> > > * Uses convert::converter to do conversion
> >
> > I prefer conversion::try_convert_to.
> 
> It doesn't read properly, but I now see why you like it: you are adding "try_" to "convert_to" the name you want for #3. There is asymmetry in convert_cast/try_converting_to versus your convert_to/try_convert_to, which is good. However, convert_to_or simply has to go! ;-)

OK.

> > > 6. convert::stream_converter customization point
> > > * Stream-based main logic
> > > * Functor interface
> > > * Supports manipulators
> >
> > What about streamable::converter?
> 
> In my mind, there's one main namespace for the conversion library. Therefore, that would need to be conversion::streamable::converter and then I'd expect conversion::something_else::converter for the non-stream-based converter. I'd prefer conversion::converter and conversion::stream_converter, but that does require different names for the two sets of function templates.

Well, I don't think we need absolutely a specific namespace for the generic conversion (using the conversion operator by default) other than the conversion namespace, 

For the stream based conversion I will not use conversion::streaming::, but just streaming:: 

> BTW, I think "streaming" or even "stream" is better than "streamable."

yes, streaming could be more acceptable.

> > > Does that cover everything satisfactorily? Did I miss
> > > something? Except where I have suggested specific support
> > > for convert::result, I don't think it should be allowed.
> > > (That is, use SFINAE to ensure that convert::result isn't
> > > used with the other parts of the API.)
> >
> > If try_ will return bool, I think that we need also a function
> > that returns optional.
> 
> OK. There are those that dislike non-const reference arguments, so such an interface would satisfy those.
> 
> > Even if the function has the problem related to a template
> > parameter, the name of the function don't reflect that the
> > result will be the parameter T, and doesn't return a bloated
> > interface, but a well know and useful class that I believe is
> > proposed for TR2, or if it is not the case it should be.
> 
> I'm sorry. I couldn't parse that.

It is my bad. What I meant is that the problem was found in the context in of a single function behaving as an optional, the Target type itself, a bool. When we use two different names, one returning the target type (convert_to) and the other returning an optional (try_convert_to) is simple to avoid the ambiguity. It is enough to use convert_to. No need to add a .value() to qualify the requested result.

> > The names I have suggested allow to have two more or less
> > independent libraries, that follows the same interface on the
> > common needs, but having a different default behavior. For the
> > specific stream conversions there are surely a lot of variants
> > that can be provided.
> 
> I won't summarize the API this time because there are names to be ironed out, but there is the optional-based interface to add. I think that's best handled by making the use of optional explicit:
> 
> optional const o1(convert_cast>(s));
> optional const o2(as>(s));
> 
> When auto is added to the mix, it works very nicely.
> 
> auto const o1(convert_cast>(s));
> auto const o2(as>(s));

Ok. I agree with this overloading as it states clearly the returned type and avoid looking for a name. I will add the new prototypes for the part non related to streams in Boost.Conversion and request a pre-review to see what others think. If the namespace conversion is requested by others I will add it.

Waiting for what Vladimir will propose.

Best,
Vicente