Subject: Re: [boost] [review] Convert library
From: Roland Bock (rbock_at_[hidden])
Date: 2014-05-25 08:47:35
Hi,
On 2014-05-25 03:40, Vladimir Batov wrote:
>> The most trivial use of boost::convert, and boost::lexical_cast and
>> > boost::coerce simply don't compare in my opinion.
>> >
>> > boost::cstringstream_converter cnv;
>> > int i = boost::convert
>> > <int>
>> > ::from("23", cnv).value();
>> >
>> > versus
>> >
>> > int i = boost::lexical_cast
>> > <int>
>> > ("23");
>> > int j = boost::coerce::as
>> > <int>
>> > ("23");
> Here we go again... :-) lexical_cast will be haunting me to my grave. :-)
>
No need for that :-)
First of all, I think I would not start each and every section of the
documentation with how to do something with lexical_cast...
Apart from that:
I think the main objection here is the amount of code you have to write
in order to get the most simple case: I have a value of type X and want
to convert it to a value of type Y.
I think it would be relatively simple to get to a much leaner interface.
For instance,
* convert could have a converter template parameter, defaulting to the
stringstream converter
* convert::from() could have an overload that just takes the "from"
parameter and provides the default-constructed converter itself.
* convert could have a conversion operator for the OutType, which does
the same as convert::value()
Then I would have
int i = boost::convert<int>::from("17");
As far as I can see, conversion errors are handled by
1. throwing exception (and letting the developer handle that where and
how appropriate)
2. using a default value
3. calling a function (e.g. report the problem and provide a default
value or throw an exception)
With 1) being the default, I could have overloads for 2) and 3), e.g.
int i = boost::convert<int>::from("17", 0);
int i = boost::convert<int>::from("17", []{
std::cerr << "conversion problem, using default" << std::endl;
return 0;} );
And if I want to use a specific converter that, I could use something like
MyConverter cnv{some args};
int i = boost::convert<int>::with(cnv).from("17", 0);
Or, if the converter can be default constructed:
int i = boost::convert<int, MyConverter>::from("17", 0);
In a similar way you could create the functors, for instance to be used
by algorithms:
auto f = boost::convert<int>::functor();
// no need for the <string> template argument if the call operator
// is a template, I'd say
Without having looked at the code that, I assume that an API like that
would be rather easy to accomplish.
I am not sure if my opinion is strong enough to be counted as a review,
but here it comes anyway:
> What is your evaluation of the design?/
I am not too fond of the frontend API as is. There are several
suggestions for a leaner API in the reviews. I second Julian's opinion
that it would be beneficial to mention parameters in the order [from,
to] instead of [to, from].
> What is your evaluation of the implementation?
The few files I looked at seemed ok, but I did not really do a code review.
> What is your evaluation of the documentation?
Not being familiar with boost::lexical cast, I had difficulties finding
the things which are really about boost::convert (the note in "getting
started") does not help...
The documentation also leaves a lot to guesswork, IMHO. For instance,
the examples suggest, that convert<int>::result has a conversion
operator to bool which returns true, if the conversion was successful,
and false otherwise. If references to lexical_cast were omitted, there
would be enough room for explanations.
> What is your evaluation of the potential usefulness of the library?
Personally, I have to admit, that I do not really see myself using the
library as is. I would write wrappers around it to get an interface
closer to what I want. But then, I have to wonder why I don't use the
converters directly? And since the converters are mere wrappers to
existing stuff afaict, I would probably use the underlying things directly.
So unless the API get's cleaned up in a way that does not make me want
to wrap it, I see no real use.
> Did you try to use the library? With what compiler? Did you have any
problems?
No.
> How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
About 3hours including writing this mail. Feel free not to count this as
a full review.
> Are you knowledgeable about the problem domain?
I'd say yes, having written a few converters myself, having used quite a
few more.
And finally:
Do you think the library should be accepted as a Boost library?
I try to avoid conversions like cats try to avoid water. The library
does not make we want to go swimming, so to speak.
Yes (since I need to swim sometimes), under the condition that the API
and the documentation get cleaned up, especially
* parameters to -> from
* default parameter for the converter
* ability for the user of the library to provide methods to be called
in case of conversion error
o non-throwing methods would return a default value
* Consider convert to be of a value on its own, not just in comparison
to lexical_cast. If I need be annoyed by lexical_cast in order to
understand the value of convert, there is something wrong...
* Actually explain the library instead of just giving examples and
letting the reader guess.
Regards,
Roland