Boost logo

Boost :

Subject: Re: [boost] [review] string convert
From: Vicente BOTET (vicente.botet_at_[hidden])
Date: 2011-05-02 13:58:56


Hi,

> Message du 02/05/11 17:28
> De : "Gordon Woodhull"
> A : boost_at_[hidden]
> Copie à :
> Objet : [boost] [review] string convert
>
> Hi Vladimir, Ed, all,
>
> > - What is your evaluation of the design?
>
> That's the focus of my review, specifically, the design of the interface.
>
> Supplying a default value, a don't-throw option, and applying manipulators/locale, are the major features missing from lexical_cast. So I think there is no doubt that a library like this is needed, if lexical_cast can't be upgraded. And according to its author, lexical_cast can't be upgraded because it must look like a cast and all these features require more parameters (template or actual). So, fine, let's have a replacement that's not .*_cast.
>
> It is incredibly clear what lexical_cast does because it uses the standard meaning of the operators, function calls, and return values. There is a lot of value in that.
>
> I don't think it's always a bad idea to have lazily evaluated objects or creative uses of operators, but IMHO if it is possible to do something more clearly with the standard order of evaluation and standard use of operators and function calls, then that should be preferred.
>
> 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

the following

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

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

* Instead of

convert::result res = convert::from(str, up_dir);

the following

optional res = string_convert >(str);

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

* Instead of

string si = string_convert::from(i)
>> std::showbase >> std::uppercase >> std::hex;

the following

string si = string_convert(as_istream(i) >> std::showbase >> std::uppercase >> std::hex);

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

Of course the string_convert function must be overloaded when the source parameter is a defaults_to or an input stream and when the target parameter is optional and the function is extensible.

In this way the interface follows always the same schema. One source parameter convertible to one target.

Just my 2cts,
Vicente


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk