Artyom <artyomtnk <at> yahoo.com> writes: Hello All, This is my review.
Artyom, Thank you for finding the time to evaluate the code and to submit your review. It's much appreciated.
I like the flexibility of this library, manipulators, locales, very-very-very convenient.
I am really glad to hear that. Especially given your evaluation runs well beyond of a quick glance over the documentation.
bool optimization.
Yes, indeed string-to-bool needs work. My only defence is that I was upfront about it as I always considered that to be more of an optimization *example* rather than a full-fledged converter. For my own purposes std::stream-based conversions are quite adequate. However, your suggestion of falling back to the std::stream-based conversion when quick string-to-bool fails makes a lot of sense. With that approach we seem to have the cake and eat it too. :-)
writing boost::convert<type>::result to the stream
There is a "strange" behavior when trying to use iostreams directly after convert:
// outputs 1 instead of 15000 boost::convert<int>::result r = boost::convert<int>::from("15000"); std::cout << r << std::endl;
// outputs 15000 as expected std::cout << boost::convert<int>::from("15000") << std::endl;
// does not compile... std::cout << boost::convert<std::string>::from(15000) << std::endl;
I admit that these use-cases never occured to me. Still, I find them quite legitimate. Curiously (or confusingly), #1 returns '1' because convert::result is implicitly converted to bool (well, safe bool). Not good. Seems simple to fix by providing template<class T> std::ostream& operator<<(std::ostream&, typename convert<T>::result const&) I'll need to look if #3 can be addressed in a similar manner. Putting that on my TODO/FIXME list (if the review outcome is positive).
Include guards ... So it is better to write BOOST_..._HPP
Makes sense. Another entry in my TODO/FIXME list.
What is your evaluation of the potential usefulness of the library?
Very useful. The conversion according to current locale of boost::lexical_cast and throwing behavior caused me to use std::stringstream as much more reliable.
I am really glad to hear that. I personally 've using the lib. quite extensively and I simply cannot imagine going back to lexical_cast... well, I just cannot as it lacks the needed functionality. I'd expect people with similar needs/tasks to face similar problems with conversions and lexical_cast.
std::string now = boost::convert<std::string>::from(std::time(0)) >> boost::locale::as::datetime; Or std::string now = boost::convert<std::string>::from(std::time(0)) >> boost::locale::as::ftime("%Y-%m-%d %H:%M:%S");
That's extra cool. I am currently working for the airline industry and they have/use UNIX (starts 1970) and industry-specific SSD (starts 1979) times. Then both UNIX and SSD are UTC and local. Or rather locals (depending on destination). Then different representations. Royal pain. That boost::locale seems to be able to help me great deal. That's the lib. that's been reviewed recently, right? I've gotta have it. Thanks for the pointer.
Optimization and specialization part.
This library optimizes "bool" case which is rarely used and also currently implemented wrong.
I would suggest to do something like
I snipped your suggestions for brevity only. They both look legitimate to me and I'll have to implement/try them both to see if there any clear advantages of one or the other. Another entry in my TODO/FIXME list. Best, V.