I forgot to answer the standard questions, so here they are: What is your evaluation of the design?
It is good: the converter concept has been adequately identified, considering performance. The responsibilities (the conversion itself an the usage conveniences) have been correctly separated. What is your evaluation of the implementation?
It is satisfactory. What is your evaluation of the documentation?
I must admit that had it not been for the email exchanges and discussions here in the group, I would have troubles figuring out what the library is capable of from the documentation. I mean, every piece of information is there, but the focus on the comparisons with lexical_cast is distracting. The first thing I want to know about the conversion library is: is it a ware of different locales and if I can easily (without performance implications) check the conversion failure. I would expect the initial page to be short(er) and give answers to my questions. Apart form the above remark, I believe that the documentation contains every piece of information that is needed. What is your evaluation of the potential usefulness of the library?
Very useful! There is no standard satisfactory way of converting a string to an int/float in C++ community.
Did you try to use the library? With what compiler? Did you have any problems?
I played with the pre-review versions on VC10. Basic examples worked fine. How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
I studied the documentation of this and the previous versions. Looked at the implementation of the facade of pre-review versions and had a short look at the current. Are you knowledgeable about the problem domain?
I often need to convert, and had occasionally need to write my own simple
conversion functions
Regards,
&rzej
2014-05-17 22:33 GMT+02:00 Andrzej Krzemienski
Hi, This is the review of the Convert library. My vote is a CONDITIONAL YES: this library should be accepted into Boost subject to conditions described at the end of this message.
This is a good and useful library. This is what I needed for a long while: a tool that allows me to convert a string to an int or float, but that would not treat the failure to convert as something unusual, and that would be locale-aware. Convert library offers this. It also offers other things that I fail to appreciate, but I accept that it is supposed to satisfy more expectations than just mine.
Your choice of the generic type converter: bool operator()(TypeIn const&, TypeOut&); I think it is the right choice: this is the signature that does not impose any run-time overhead. Unlike boost::convert<TypeOut>::result, which does impose some run-time overhead of copying (or at least moving) the TypeOut. But that's acceptable. If the conversion needs to be super fast, one can fall back to using the converter directly.
I appreciate the stringstream-based converter most. About the ability to plug other converters, I am just not convinced. Why would I use other converters? I know, lexical_cast is supposed to be faster in some cases, but if I was interested in this performance I would have used lexical_cast directly. The alleged ability to use this library to change the encoding of the strings or to convert between "related" types like ptime and time_t and Time -- I am not convinced: do you expect such conversions to fail and to report such failure the same way as you report the impossibility of interpreting a string as a float?
I personally do not like this trick to force me to type this "from" in "convert<int>::from". If I was only passing my string as an argument, this would be fine, but when I am also passing the converter object, it reads that I am converting from both the converter and the string. But I do not intend to argue over it. You wrote the (good) library, so you have the privilege to pick the name.
I am also not convinced about the potential of the 'facade' part. I can see that you can see the potential in it. But to me separating the library into the facade and the stringstream converter adds no practical value. But the stringstream converter itself is enough to stand for the usefulness of library.
My condition on the acceptance of the library is that its performance should be improved as indicated by others, in particular, provide move semantics for boost::convert<TypeOut>::result (on compilers with rvalue references). It is enough that you commit to doing it in the future.
Regards, &rzej