Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Vicente BOTET (vicente.botet_at_[hidden])
Date: 2011-05-02 16:36:42

> Message du 02/05/11 20:23
> De : "Stewart, Robert" 
> A : "'boost_at_[hidden]'" 
> Copie à : 
> Objet : Re: [boost] [review] string convert
> 
> Vicente BOTET wrote:
> > > De : "Gordon Woodhull"
> >
> > > The interface of the proposed Convert library is not simple
> > > enough. To start out with the most annoying thing, I don't
> > > like "::from" at all. The replacement should look as close
> > > to lexical_cast as possible.
> > >
> > > So, why the extra '::from'? Even though I disagree with the
> > > lazy evaluation (see below), even that doesn't seem to
> > > require this weird static member syntax. Is '::from' just
> > > so that '::result' will not feel lonely?
> > >
> > > For the rest of this review, I'm going to refer to
> > > convert::from(s), but please keep in mind that I really
> > > think it should be something like string_convert(s).
> >
> > I wanted to add more on the this direction. Vladimir, note that
> > maybe the syntax proposed below could be not correct in C++,
> > but I hope you will get the idea.
> >
> > What do you think of using
> >
> > * Instead of
> >
> > int i1 = convert::from(str); // Throws if the conversion fails
> 
> That should be convert::from(str).
> 
> > the following
> >
> > int i1 = string_convert(str); // Throws if the conversion fails
> 
> You haven't specified the target type. You'd need:

You are rigth. I forget the template parameter.

> int I = string_convert(str);
> 
> which doesn't read well.

Does 
int I = convert_to(str);
reads better?

> > * Instead of
> >
> > int i2 = convert::from(str, -1); // Returns -1 if the
> > conversion fails
> >
> > the following
> >
> > int i2 = string_convert(defaults_to(str, -1)); // Returns -1 if
> > the conversion fails
> >
> > defaultts_to(T,U) is a function that generates a wrapper to T
> > and default value U.
> 
> You have the same problems here, of course. Adding the wrapper really looks bad. I'd prefer something more like this:
> 
> int I = convert::from(str, default_(-1));
> 
> if you wish to make it explicit.

I would prefer

int I = convert_to(str, default_(-1));

> > * Instead of
> >
> > int i = convert::from(hex_str, -1) >> std::hex;
> >
> > the following
> >
> > int i = string_convert(defaults_to(as_istream(hex_str) >>
> > std::hex, -1);
> >
> > as_istream is a function that convert a type to an input stream
> > (provided the type is output streamable).
> 
> I see that you're trying to make "as_istream(hex_str) >> std::hex" look like a normal stream that plugs into your string_convert function, but it looks very ugly to me.

Yes, it is ugly. Maybe the following is less ugly

int i = -1;
as_istream(hex_str) >> std::hex >> i ;

> > * Instead of
> >
> > string d1 = string_convert::from(d)(locale_ = rus_locale)
> > >> std::setprecision(4)
> > >> std::scientific;
> >
> > the following
> >
> > string d1 = string_convert(as_istream(d) >>
> > set_locale(rus_locale) >> std::setprecision(4)
> > >> std::scientific;
> >
> > here set_locale can be a manipulator.
> 
> With that locale, you could also have:
> 
> string s = convert::from(d)
> >> set_local(rus_locale) >> std::setprecision(4)
> >> std::scientific;

or just

as_istream(d) >> set_locale(rus_locale) >> std::setprecision(4) >> std::scientific >> s;

> I understand that there are things hiding behind a façade:
> 
> - convert::from(d) returns a convert::result
> - convert::from(d) acts like a std::istream WRT manipulators
> - convert::from() returns a convert::converter
> 
> I also get that convert::result converts implicitly to a string but can also be queried via a safe-bool and directly for the string, but adding lots of explicit wrappers into the mix just obfuscates the desired conversion operation. It makes things more obvious, but syntactically obese.

You are right trying to force all around a single function gives non natural results. This is the main issue with this library. I'm sure we can find some more explicit, readable and acceptable syntax.

Best,
Vicente