First of all, there is already a Boost.Conversion library (released in 1.20.0). Users will be confused, because two libraries have same names. * What is your evaluation of the design? It is simple and looks good. But consider the following situation: lexical_cast<char>(1) and numeric_cast<char>(1) will produce output. It must be documented, which cast will your library use in which cases. There must be some tags, to allow user to choose, which conversion is required. There are some dummy::type_tag tags in source code, but looks like they are not used, and not documented! * What is your evaluation of the implementation? Did not look very carefully trough the sources. * What is your evaluation of the documentation? There is no documentation about which conversion when used. convert_to<char>(1) will call numeric_cast or lexical_cast? * What is your evaluation of the potential usefulness of the library? Usually user just want to use lexical_cast or numeric_cast. Situations, when generic conversion is required are less often. * Did you try to use the library? With what compiler? Did you have any problems? I did not used it. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? A quick reading. * Are you knowledgeable about the problem domain? I am knowledgeable about lexical conversions domain. Generic conversions are close enough to that domain. * Do you think the library should be accepted as a Boost library? Yes, it should. But only after the documentation will be updated and tags added. Until then, it is not clear which cast is when used and library is not as generic as possible. But such big changes to library design will require some new short review.
First of all, there is already a Boost.Conversion library (released in 1.20.0). Users will be confused, because two libraries have same names. I pretended to include the 'generic' conversions into the conversion
Le 28/08/11 09:43, Antony Polukhin a écrit : library.
* What is your evaluation of the design?
It is simple and looks good. But consider the following situation: lexical_cast<char>(1) and numeric_cast<char>(1) will produce output. It must be documented, which cast will your library use in which cases. The documentation of header <boost/conversion/std/string.hpp> includes "Include this file when using conversions from/to |std::string| via |lexical_cast|. "
There must be some tags, to allow user to choose, which conversion is required. There are some dummy::type_tag tags in source code, but looks like they are not used, and not documented! They are used to overload the conversion functions. The class is documented here https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h.... This https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h... tutorial section explains how to use it.
* What is your evaluation of the documentation?
There is no documentation about which conversion when used. convert_to<char>(1) will call numeric_cast or lexical_cast? The library doesn't provide a conversion from int to string if you don't include the specific file mentioned above.
* Do you think the library should be accepted as a Boost library?
Yes, it should. But only after the documentation will be updated and tags added. Until then, it is not clear which cast is when used and library is not as generic as possible. But such big changes to library design will require some new short review.
Could you elaborate on how you see the library can use these tags? Thanks for your review, Vicente
2011/8/28 Vicente J. Botet Escriba <vicente.botet@wanadoo.fr>:
Le 28/08/11 09:43, Antony Polukhin a écrit :
* What is your evaluation of the design?
It is simple and looks good. But consider the following situation: lexical_cast<char>(1) and numeric_cast<char>(1) will produce output. It must be documented, which cast will your library use in which cases.
The documentation of header <boost/conversion/std/string.hpp> includes "Include this file when using conversions from/to |std::string| via |lexical_cast|. "
And that`s were the problems begin! If user included this header, all his conversion will convert convert_to<char>(1) as lexical cast does? And what if the user whants conversions, like numeric cast does? More examples: Two developers are working on different parts of project. Developer #1 uses convert_to<char>(1) in his code and wants behavior like numeric_cast. He also uses some headers, which is maintained by Developer #2. At one day Developer #2 includes <boost/conversion/std/string.hpp> in one of his headers. And we get really hard detectable errors!
There must be some tags, to allow user to choose, which conversion is required. There are some dummy::type_tag tags in source code, but looks like they are not used, and not documented!
They are used to overload the conversion functions. The class is documented here https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h.... This https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h... tutorial section explains how to use it.
I could not determinate, for what are this tags used from the documentation. Can we use them, to call numeric_cast instead of lexical_cast? Can we use them, to call Boost.Coerce instead of lexical_cast? can we use them, to support some parameters for Boost.Coerce?
Could you elaborate on how you see the library can use these tags?
Some thing like this: convert_to<char>(1, numeric_tag); convert_to<char>(1, coerence_base<16>); convert_to<pair<char, char> >(make_pair(1,1), lexical_tag); convert_to<double>(1); // We can also convert without tags Best regards, Antony Polukhin
Le 29/08/11 17:42, Antony Polukhin a écrit :
2011/8/28 Vicente J. Botet Escriba<vicente.botet@wanadoo.fr>:
Le 28/08/11 09:43, Antony Polukhin a écrit :
* What is your evaluation of the design?
It is simple and looks good. But consider the following situation: lexical_cast<char>(1) and numeric_cast<char>(1) will produce output. It must be documented, which cast will your library use in which cases. The documentation of header<boost/conversion/std/string.hpp> includes "Include this file when using conversions from/to |std::string| via |lexical_cast|. " And that`s were the problems begin! If user included this header, all his conversion will convert convert_to<char>(1) as lexical cast does? And what if the user whants conversions, like numeric cast does?
More examples: Two developers are working on different parts of project. Developer #1 uses convert_to<char>(1) in his code and wants behavior like numeric_cast. He also uses some headers, which is maintained by Developer #2. At one day Developer #2 includes <boost/conversion/std/string.hpp> in one of his headers. And we get really hard detectable errors!
There must be some tags, to allow user to choose, which conversion is required. There are some dummy::type_tag tags in source code, but looks like they are not used, and not documented! They are used to overload the conversion functions. The class is documented here https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h.... This https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h... tutorial section explains how to use it. I could not determinate, for what are this tags used from the documentation. As said in the documentation they are used to overload the customization
I will remove the string conversion are required by others as there is no consensus on which conversion must be used. The library works for unrelated types for which there is a clear and normal conversion. There are a lot of cases where the conversion should be unique, but the library can not ensure that that two developpers would define the same conversion. This section https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h... gives some hints on how the libraries could organize conversion so only one definition is included in a program. points.
Can we use them, to call numeric_cast instead of lexical_cast? Can we use them, to call Boost.Coerce instead of lexical_cast? can we use them, to support some parameters for Boost.Coerce? No. I don't know from where you can get this impression. As said in the documentation the library is not intendeed to manage with string conversions neither with specificities in numeric conversions. This features are already covered by other libraries.
Could you elaborate on how you see the library can use these tags? Some thing like this:
convert_to<char>(1, numeric_tag); convert_to<char>(1, coerence_base<16>); convert_to<pair<char, char> >(make_pair(1,1), lexical_tag); convert_to<double>(1); // We can also convert without tags
This could be a possibility, but IMO tags don't scale well in this case. You will need to tag each one of the leaves otherwise you will find cases that would not be covered by the interface. convert_to<pair<char, char> >(make_pair(1,1), make_pair(numeric_tag, lexical_tag)); Instead of this the user can create wrapper classes that convey the intendeed conversion. convert_to<pair<char, char> >(make_pair(numeric_tag(1), lexical_tag(1))); We can now define that lexical_tag<T> convert to a type U using lexical_cast, and that numeric_tag<T> uses numeric_cast. The library is open to these kind of customizations, but this works only if the users stors the wrappers and don't need to build them explicitly before doing the conversion. Best, Vicente
On 29 August 2011 18:57, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Le 29/08/11 17:42, Antony Polukhin a écrit :
2011/8/28 Vicente J. Botet Escriba<vicente.botet@wanadoo.fr>:
Le 28/08/11 09:43, Antony Polukhin a écrit :
* What is your evaluation of the design?
It is simple and looks good. But consider the following situation: lexical_cast<char>(1) and numeric_cast<char>(1) will produce output. It must be documented, which cast will your library use in which cases.
The documentation of header<boost/conversion/std/string.hpp> includes "Include this file when using conversions from/to |std::string| via |lexical_cast|. "
And that`s were the problems begin! If user included this header, all his conversion will convert convert_to<char>(1) as lexical cast does? And what if the user whants conversions, like numeric cast does?
More examples: Two developers are working on different parts of project. Developer #1 uses convert_to<char>(1) in his code and wants behavior like numeric_cast. He also uses some headers, which is maintained by Developer #2. At one day Developer #2 includes <boost/conversion/std/string.hpp> in one of his headers. And we get really hard detectable errors!
I will remove the string conversion are required by others as there is no consensus on which conversion must be used. The library works for unrelated types for which there is a clear and normal conversion. There are a lot of cases where the conversion should be unique, but the library can not ensure that that two developpers would define the same conversion.
If the traits are moved to Boost.TypeTraits, Boost conversions are moved to the respective libraries (both of which I support and believe should be done regardless of the review results) and string conversions are removed, then what's left of this library?
This section https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h... gives some hints on how the libraries could organize conversion so only one definition is included in a program.
There must be some tags, to allow user to choose, which conversion is required. There are some dummy::type_tag tags in source code, but looks like they are not used, and not documented!
They are used to overload the conversion functions. The class is documented here
https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h.... This
https://svn.boost.org/svn/boost/sandbox/conversion/libs/conversion_ext/doc/h... tutorial section explains how to use it.
I could not determinate, for what are this tags used from the documentation.
As said in the documentation they are used to overload the customization points.
Can we use them, to call numeric_cast instead of lexical_cast? Can we use them, to call Boost.Coerce instead of lexical_cast? can we use them, to support some parameters for Boost.Coerce?
No. I don't know from where you can get this impression. As said in the documentation the library is not intendeed to manage with string conversions neither with specificities in numeric conversions. This features are already covered by other libraries.
Could you elaborate on how you see the library can use these tags?
Some thing like this:
convert_to<char>(1, numeric_tag); convert_to<char>(1, coerence_base<16>); convert_to<pair<char, char> >(make_pair(1,1), lexical_tag); convert_to<double>(1); // We can also convert without tags
This could be a possibility, but IMO tags don't scale well in this case. You will need to tag each one of the leaves otherwise you will find cases that would not be covered by the interface.
convert_to<pair<char, char> >(make_pair(1,1), make_pair(numeric_tag, lexical_tag));
Instead of this the user can create wrapper classes that convey the intendeed conversion.
convert_to<pair<char, char> >(make_pair(numeric_tag(1), lexical_tag(1)));
We can now define that lexical_tag<T> convert to a type U using lexical_cast, and that numeric_tag<T> uses numeric_cast.
The library is open to these kind of customizations, but this works only if the users stors the wrappers and don't need to build them explicitly before doing the conversion.
Best, Vicente
Jeroen
Le 30/08/11 21:36, Jeroen Habraken a écrit :
On 29 August 2011 18:57, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Le 29/08/11 17:42, Antony Polukhin a écrit :
2011/8/28 Vicente J. Botet Escriba<vicente.botet@wanadoo.fr>:
I will remove the string conversion are required by others as there is no consensus on which conversion must be used. The library works for unrelated types for which there is a clear and normal conversion. There are a lot of cases where the conversion should be unique, but the library can not ensure that that two developpers would define the same conversion. If the traits are moved to Boost.TypeTraits, Boost conversions are moved to the respective libraries (both of which I support and believe should be done regardless of the review results) and string conversions are removed, then what's left of this library?
Recall that the intent of Boost.Conversion was to manage with conversion of unrelated types. While we could (if the authors accept it) add some of the provided extrisic conversion intrinsically to some of the Boost libraries, we can not do the same for the STL classes, neither for the 3rd party libraries. Best, Vicente
participants (3)
-
Antony Polukhin -
Jeroen Habraken -
Vicente J. Botet Escriba