Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2011-05-02 14:21:36

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<int>::from(str).

> the following
>
> int i1 = string_convert(str); // Throws if the conversion fails

You haven't specified the target type.  You'd need:

   int I = string_convert<int>(str);

which doesn't read well.

> * 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<int>::from(str, default_(-1));

if you wish to make it explicit.

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

> * 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<string>::from(d)
   >> set_local(rus_locale) >> std::setprecision(4)
   >> std::scientific;

I understand that there are things hiding behind a façade:

 - convert<string>::from(d) returns a convert<string>::result
 - convert<string>::from(d) acts like a std::istream WRT manipulators
 - convert<string>::from() returns a convert<string>::converter

I also get that convert<string>::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.

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