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