Re: [boost] [convert] Review

Subject: Re: [boost] [convert] Review
From: Vladimir Batov (vbatov_at_[hidden])
Date: 2011-04-26 21:13:23

> Thomas Heller <thom.heller <at> googlemail.com> writes:
> Hi,
> Following is my review of the convert library ...

Thomas,

Thank you for your review. It's much appreciated. 

> What is your evaluation of the design?
> I think the design is not good.

Well, that's not good. :-) Care to elaborate? For clarity we'll need to
distinguish design from API and/or implementation. I only bring it up because
your review is largely/exclusively concerned with API.

> The use of operator>> feels misplaced. 

I hope I covered it in my prev. reply to Jeroen Habraken.

>... the rational behind the non-throwing version isn't very clear
> to me. It feels like a step backwards: Weren't explicitly returned
> error codes obsoleted by exceptions?

I tried covering that topic throughout the documentation. In a nutshell as I
indicated in the "Getting Started" section the rationale behind not-throwing is
that quite often "a conversion failure is not exceptional enough to justify
throwing an exception". The std::fstream::open() behavior might be another such
example. But I am sure you've already read that "Getting Started" section. So,
I am not sure if I managed to answer your question... unless it was a rhetorical
question.

Stroustrup, chapter 14.1 stated that the fundamental idea behind exceptions "is
that a function that finds a problem it *cannot cope with* throws an exception".
So, my reading of it (and other sources) is that exceptions are a somewhat "big
gun" more appropriate applied to the high-level design.

> I don't get how the design really is superior to boost::lexical_cast, 

I am not sure if the underlying convert *design* is any superior compared to
the lexical_cast design. Although, I'd note that convert provides considerably
more room for specialization and optimization. However, as far as the
functionality is concerned (something that users might be interested in), then
I have a feeling that 'convert' provides quite a bit more. From the
"Introduction" section: 

* throwing and non-throwing behavior when conversion fails;
* support for the default/fallback value to be returned when conversion fails;
* two types of the conversion-failure check - basic/simple and better/safe;
* formatting support based on the standard std::streams and
std::stream-based manipulators (like std::hex, std::scientific, etc.);
* support for different locales;
* support for boost::range-compliant char and wchar_t-based string
containers (std::string, std::wstring, char const*, wchar_t const*, char
array[], etc.);
* no DefaultConstructibility requirement for the Target/Destination type;
* extendibility and additional room to grow.

But I am sure you've already read that section. So, I am a bit at a loss what
to add to it.

> ... why it couldn't been implemented as an extension to that library.

I tried covering that in my prev. reply to Jeroen Habraken.

> I don't think that having everything hidden behind this static from
> function is wise and should be rethought. Different functions, for
> different purposes would be better suited.

I am not sure if I follow. I thought you indicated that "the API is too
convoluted". Now "everything hidden behind this static from() function" does not
exactly sound convoluted. Indeed, you find it too small instead. Surely you are
not saying that making API bigger will be less convoluted and easier for the
user? I thought we had one purpose -- conversion -- served by one function --
convert::from. What other purposes? How are you suggesting to extend the
existing API? Unfortunately, I am not sure I am quite following what you are
actually suggesting as you seem to be at peace with one-function lexical_cast
API.

> What is std::u8string?

You've got me. Unfortunately, there is no such a thing. Wishful thinking on my
part. I'd like to have one though. :-)

> After reading through the documentation I got the feeling that the 
> library is incomplete and not really finished yet. 

Well, I happen to disagree with your feeling. I suspect it is hard/impossible
to qualify this or any library as complete. 'Convert' is more of a framework to
incorporate more type conversions if/when needed and to explore other (like
string-to-string) conversions if such a need arises. It does not mean the
library is not usable now. In fact, I've been using it quite extensively and
instead of lexical_cast for a few years.

> Almost any feature is experimental. 

Well, that's certainly wrong (IMHO of course).

> Do you think the library should be accepted as a Boost library?
> No. I second that it feels more like a extension to boost::lexical_cast
> than anything else.

Please see my reply to Jeroen Habraken.

Best,
V.