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. :-)
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
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
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?
Did you try to use the library? With what compiler? Did you have any
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. 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