Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2011-05-03 08:10:04

Vicente BOTET wrote:
> > De : "Stewart, Robert"
> > Vicente BOTET wrote:
> > > > De : "Gordon Woodhull"
>
> > 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?

You forgot the target type again.  Let's compare:

   convert<int>::from(str);
   convert_to<int>(str);

Your version reads pretty nicely and is shorter.  For that particular use case, I'd call it better.  Indeed, you might as well make it a cast:

   convert_cast<int>(str);

That would make it completely self-documenting as it follows the new-style cast interface.  However, "convert" isn't quite as useful as a cast descriptor and then there's the question of the other behaviors.

> > > * Instead of
> > >
> > > int i2 = convert::from(str, -1); // Returns -1 if the
> > > conversion fails
> > >
> > > the following
> > >
> > > int i2 = string_convert(defaults_to(str, -1));
> > >
> > > 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));

convert<int>::from(str, default_(-1));

> > if you wish to make it explicit.
>
> I would prefer
>
> int I = convert_to(str, default_(-1));

convert_to<int>(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 ;

That's pretty reasonable.  Streams are known to set an error state, possibly throwing an exception when doing so, and so the default -1 would be intact should hex_str not be parsable as a number.

There is one thing missing in that approach, however: how does one determine if the conversion fails for a type without a useful default value?  convert<T>::result provided a safe-bool interface to determine whether the conversion succeeded.  That permits initializing the value to hold the conversion without having to use that initial value as the sentinel for failure.

This is why I asked Gordon for a complete interface.  Only when all of the use cases are addressed can one make a reasonable comparison with the interface submitted for review.

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

OK

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

Many things were discussed some years ago.  You have some interesting ideas, but you're missing the function object part of the interface (or did we lose it through snipping context?) and the safe-bool/value part.  The ability to reuse the converters and pass them to algorithms is a nice feature.  The ability to use types without default constructors or usable sentinel values is necessary.  Valdimir's attempt to resolve all of the forces is what is being reviewed.  However, he seems open to making something better, so keep at it.

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