2014-05-18 15:16 GMT+02:00 Vladimir Batov
Andrzej Krzemienski
writes: 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. ... 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.
Yes, I remember you suggesting defaulting the converter parameter to the std::sstream-based and my initial intention to do just that. I then backed off as I realized the create-converter-once-use it-many-times can be easily lost. If we could come up with something like
boost::convert::use(some_converter);
int v = convert<int>::from(str); // some_converter used
That might address your concern, right? Any crazy tricks to achieve that without inheritance?
No; no globals, please. My remark is only about the choice of the name ("::from"): not the interface. In the other place you are suggesting to drop the "::from". This would satisfy my taste. The converter should be passed as the argument to the facade, or at least be the member of the state-full facade. Your choice is the right one.
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.
My intention, in fact, is to ditch convert<>::result for boost::optional given that you'll be pushing it into 1.56, right?
Well, at this point there are chances that all the necessary changes to Boost.Optional will not make their way to 1.56.0 release. Currently move semantics are ready, but function value() and value_or() are still missing and my velocity has shrunk since my daughter was born. It is also not certain when 1.56 is scheduled to be released. So I recommend that for 1.56 you have some plan B: either stick to your own type, or accept Boost.Optional with the limited (for the time being) interface. Regards, &rzej