Subject: Re: [boost] [Review] Boost.Convert library is ongoing
From: Vladimir Batov (vbatov_at_[hidden])
Date: 2011-04-27 17:48:47
> Artyom <artyomtnk <at> yahoo.com> writes:
> Hello All,
> This is my review.
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
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
> 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)) >>
> 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.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk