Hi, My review: -- What is your evaluation of the design? The overall design is very flexible, there are a few points though: - Although this might be a matter of taste I find the choice of operator >> to add modifiers rather odd, it smells a bit like operator abuse. Also, it forces you to use braces in std::cout << (boost::convert<int>::from("12") >> std::hex) << "\n"; for example, maybe using a member function is an option. - In might be nice to add a way to return a default-constructed object to the non-throwing interface. This is useful in combination with boost::optional: boost::convert<boost::optional<int>
::from_default("XXX") returning an boost::optional<int>() instead of having to write boost::convert<boost::optional<int> >::from("XXX", boost::optional<int>());.
-- What is your evaluation of the implementation? Had a quick read through the code, it looks fine except that names containing "__" are reserved (used in the include guards). -- What is your evaluation of the documentation? The documentation covers all aspects of the library quite nicely. This in contrast to the reference which seems mostly empty, in my opinion it doesn't add much and can simply be removed. Since the behaviour of convert seems to be different to lexical_cast (see below) it might be useful to add a section listing these differences. Lastly the documentation contains a couple of examples, it would be nice to have these as separate files in libs/convert/example. -- What is your evaluation of the potential usefulness of the library? The library is very flexible and the extra features are nice, however there is a rather large problem, in my opinion. This is the domain of boost::lexical_cast, a library that has matured over the years and is in heavy use. It can not be deprecated or worse, removed. Even though it has problems (mostly missing functionality, which boost::convert has implemented) I do not believe adding a second library is a proper solution. Instead this functionality should be added to boost::lexical_cast itself (and patches to do so have been proposed over the years). -- Did you try to use the library? With what compiler? Did you have any problems? Yes, on Mac OS X with both gcc 4.2.1 (the default shipped by Apple) and 4.5.2. Problems: - boost::convert<int>::from("-123.2") behaves differently from boost::lexical_cast<int>("-123.2"), the former returns -123 whilst the latter throws. Whilst I'm not saying one is better than the other, the differences should be documented for people switching. - boost::convert<int>::from('1') results in a rather long error message (http://codepad.org/YgoR8H2t), if the conversion from X to Y isn't possibly this should be made clear in a more user-friendly way. -- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I've read the documentation, had a quick glance at the source code and tried the basic functionality of the library. -- Are you knowledgeable about the problem domain? As the author of boost::coerce I'm familiar with the conversion domain, however I'm less familiar with the locale domain. -- Do you think the library should be accepted as a Boost library? No it should not, instead the extra functionality should be added to boost::lexical_cast. Kind regards, Jeroen Habraken