[Boost-announce] [boost] [Review] Boost.Convert, review manager analysis

Subject: [Boost-announce] [boost] [Review] Boost.Convert, review manager analysis
From: Edward Diener (eldiener_at_[hidden])
Date: 2011-05-06 11:01:06

As previously stated Vladimir Batov decided to withdraw his library at the end of the review period from consideration for inclusion into Boost and I had announced this and the fact that because of this the library would not be accepted.

Nonetheless I want to give my analysis of the reviews and the issues both pro and con of the reviewers. I also want to give some of my own thinking on these issues.

There were few reviews of the library but much late discussion in the final day of the review period and just afterward.

There were 2 reviews which voted to accept Convert ( Artyom Bellis, Alfredo Correa ), 3 reviews which voted not to accept Convert ( Jeroen Habraken, Thomas Heller, Hartmut Kaiser ), 1 review which would have accepted Convert but qualified that only if lexical_cast could not be extended instead ( Rob Stewart ), 1 review which would have accepted Convert with large scale changes ( Gordon Woodhull ), and one mention, not a formal review, of acceptance until something else came along but with perhaps a later change of mind ( Paul Bristow ). There was also one review sent to me personally by Barend Gehrels, after Vladimir Batov withdrew his library, which voted not to accept Convert. There were also a number of people who raised doubts and objections to Convert in various ways without giving a formal review.

It is all these issues raised by reviewers and commenters I want to discuss next. Vladimir Batov was responsive to nearly all of those issues but I want to enumerate them and give my own thoughts about them as I read reviews and comments.

A number of people wanted lexical_cast to be extended instead of a new library. This certainly is a valid viewpoint. But I believe that the lexical_cast library is set as it is until someone takes it over from its original author and is willing to change it. I think it is better to review the library under discussion rather than to hope that something else can be changed in the future.

There was an issue of the basic syntax which many people did not like. Instead of the syntax of 'convert<dest>::from(source)' there were a number of people who wanted to see 'convert<dest>(source)' instead, largely becaused it matched the 'lexical_cast<dest>(source)' syntax. Vladimir Batov explained why he used the syntax he did. I think in this case it is reasonable. Again this is an issue in relation to lexical_cast, and I saw no real significant difference between the syntaxes on a basis of practical use.

The main issue brought out by many people is whether a single function can encompass successfully all of the functionality which Convert wished to encompass and whether this functionality both worked smoothly with everything others wished to do with the functionality. Here there were many valid objections and I will attempt to enumerate them:

1) The return value of the convert function is really many different returns, and this is done through implicit conversions of the actual return type. This was mentioned most eloquently by Thomas Heller. It is quite natural to assume that the return value of a successful call to 'convert<dest>::from(source)' would be a 'dest' object but instead it is a value which has an implicit conversion to 'dest'. This works when passing the value to ordinary functions but does not when passing the value to template functions. As Vladimir Batov mentioned one can use the 'value()' member function of the return value, and presumably also one can use a 'static_cast<dest>' also, to force the exact 'dest' object to be passed, but I also viewed this as a negative way to work, and a very valid objection to the way Convert was designed. To me it was not just a matter of confusion to the end-user but of a naturalness of use. A convert-type interface should have some function where, if the convert is successful, the actual result of the conversion is returned, and not some intermediate value with an implicit conversion.

2) The Convert library uses a stream-based method to do conversions but wanted to leave the door open for other methods. Part of the extended syntax of the library works with stream manipulators and locales. A very valid point was brought up, again by Thomas Heller, that it is not clear how the extended syntax is supposed to work with and affect non-stream based specializations of the basic convert template. Even the method of extending the library in this way, rather than just providing stream-based operators for a given type, is open to much doubt as valid C++ and largely left unexplained in the documentation.

3) The syntax for the stream-based and locale additions to the library's functionality was found odd by many people. The term 'wacky' was often used and Vladimir Batov did defend his choice about it well.
Nonetheless it is a valid issue to consider whether the extended syntax is easy for a person to use. I found the syntax for using iostream manipulators quite natural, and the syntax for locales and other functionality such as throwing behavior less natural. Again I do not want to get too caught up in syntactical issues but I did understand the issues brought up by others regarding this.

4) The method of using the convert function in algorithms was criticized. Some people expressed their wish that a placeholder syntax, as used by other Boost libraries, be used. I actually found the way Convert does this as pretty elegant, there were two syntaxes which worked, but could understand others liking for the placeholder syntax.

5) The use of a default value, the issue of a default constructor, and the differing results when a conversion fails or not, received much discussion, especially at the official end of the review period and afterward, and I appreciate reading all of the discussions. I do not want to specifically go into each one, other than to say that I think this has basically to do with the issue of having a single function do everything which could not easily cover all the different scenarios mentioned for actual use.

There were other valid concerns which are related to just a need for more complete documentation. Vladimir Batov responded to most of them. Here I will enumerate them:

1) Rob Stewart in his review mentioned many documentation corrections related to grammar, spelling etc.
2) What locale is being used by default is not explained.
3) The reference is very incomplete.
4) The section for extending the library is unclear and incomplete.

Finally there were valid concerns relating to getting Convert to work properly. Some of these related Convert and lexical_cast and noted the differences. A few others mentioned things which just did not work properly in the actual conversion and again Vladimir Batov said he would look at these issues. Barend Gehrels sent me a late review, which he did not want to post to the mailing list because Vladimir Batov had already withdrawn is library, in which he said that he had some technical problems adapting a Boost.Geometry type to work with Convert. I think these are all technical issues which can be resolved.

Many other people had specific incisive remarks during the review period but I can not list them all. That does not mean those remarks were ignored by me in any way, as I read and attempted to digest what everyone had to say.

Final remarks:

I would like to bring some clarity to the issues which may have forced Vladimir Batov to essentially withdraw his library at the end of the review period from consideration and which led to much comment and criticism about the library. This is my purely personal take on the library, but it is based on my technical understanding of the issues involved.

The main issue with the library is that the 'single function fits all scenarios' works much less well in practical use than the author supposed, even as I understood the reason for simplicity which caused him to pursue this path.

There really needs to be a function that simple returns the converted value upon success. Overloaded versions of that function, or differently named functions, can return other specific things. Where there is a great attraction to a return value which can be implicitly converted to other values, I think this attraction given practical use is somewhat illusory.

While I did not enjoy some of the harsher remarks made by Thomas Heller in his review of the library about the design of Convert ( and I do not think he really meant them as harshly as they may have sounded ) he did rightly point out that the 'overloaded' return value is an impediment to a cleaner design which would allow one to use the right convert function given the possibilities which the library wants to accomplish.

I know there has been many subsequent discussions about a set of functions that can replace the single function in Convert depending on how Convert can be used, and I do not myself want to get in the middle of this debate, but once there is an understanding of what needs to be done, I really think the library can be designed to appeal to end-users in a better way.

The second issue in my mind is that the library attempts to do too much given the way it was designed. If the focus is simply a library which does stream-based conversion between strings and objects ( both ways ) I personally think it would be much better. Once you look for non-stream based conversions you have to consider more carefully how the stream-based syntax and locales should be integrated with the library. Once you look for type to type conversions rather than string to type and type to string conversions you have to design a good general facility for specifying how this is done within the functions you provide. This is a much bigger job of thinking about design, unless you are simply providing a very generalized shell with little real functionality built-in. I am not saying the author of the library has not thought about these issues, but my impression is that they have not been addressed carefully.

Just in case anyone wanted to know, and also so that Vladimir Batov does not feel so bad about withdrawing the library from consideration at the end of the review period, I would have rejected the library for inclusion into Boost given the reviews and comments of others and my own perceptions of the issues involved. But I think the author has still done a fine job and is close to having a library which could be included as a part of Boost.

Although it is hard work, it has been a pleasure to be a review manager of this library and I thank everyone involved and especially the author of the Convert library himself.

Edward Diener

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost