[boost] [review] convert

Subject: [boost] [review] convert
From: Matus Chochlik (chochlik_at_[hidden])
Date: 2014-05-25 02:50:06

Hi,

Here follows my review of the convert library:

- What is your evaluation of the design?
----------------------------------------------------------

The design of the interface with the post-review changes (the one returning
optional<T>) is sound and flexible even if little too verbose for certain
cases. The reason why I still write my own converters in applications is
that I don't like the interface of lexical cast that does not provide a
nothrow option. If Convert is accepted to Boost this would make my life
easier.

In regard to the verbosity, I understand that it is so for the sake of
flexibility, but if possible I would like if one of the available
converters was labelled as default (the stringstream-based one?) and a new
variant of convert accepting only the input value and internally
constructing and using the default converter was added. This is however not
a condition for accepting the library.

- What is your evaluation of the implementation?

>From the quick glance I've had at the sources it looks acceptable.
[nitpicking]
Some of the code is indented using 4 spaces and the rest using tabs. This
looks awkward and is difficult to read on editors with tab width other than
4.
[/nitpicking]

- What is your evaluation of the documentation?
----------------------------------------------------------

Why is the motivation section placed towards the back? This section IMHO
belongs to the front. Otherwise acceptable.

- What is your evaluation of the potential usefulness of the library?
----------------------------------------------------------

Very useful, even to the point that I'd like to have it in the standard
(one day soon).

- Did you try to use the library? With what compiler? Did you have any
problems?

Yes, with gcc 4.8.1.

I've built and run the `test_convert.cpp` test case. It build without
problems, but I had to change the hardcoded locale for it to run without
errors. Also, the makefile in `test/` references files outside of the
library source tree.

I've also tried it on some of my own very simple applications which all
ultimately built and ran without errors.

- How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?

In total somewhere between 2 and 3 hours. I've read the docs and some of
the reviews here, I've at least scrolled through all source files, and had
a look at some parts that I was interested in.

- Are you knowledgeable about the problem domain?

As knowledgeable as anyone who had to write his own converters for argument
values or text input many times.

- Do you think the library should be accepted as a Boost library?

Yes, if the new interface returning `optional<T>` is used.

Best regards,

Matus Chochlik