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
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? 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
participants (1)
-
Matus Chochlik