On 13 May 2014 17:07, Edward Diener wrote:

On 5/13/2014 6:09 PM, Niall Douglas wrote:

...

On 13 May 2014 at 18:00, Edward Diener wrote:

The review of the Convert library started Monday, May 12 and goes

...

through Wednesday May 21. Many people may have missed the first announcement since the mailing lists were held back when the review started so I am announcing this again.

I think it is *deeply* unfortunate this was scheduled to occur during C++ Now. Many of us will still be recovering from jetlag when this review ends.

I will ask, because of C++ Now and the mailing list snafu, whether the review can be extended for 4 days through Sunday May 25.

I'd appreciate this as well, having had a strong opinion on the previous version of the library I'd love to have a look at what it evolved into. Jeroen

Jeroen, On 05/15/2014 03:47 AM, Jeroen Habraken wrote:

I'd appreciate this as well, having had a strong opinion on the previous version of the library I'd love to have a look at what it evolved into.

Jeroen

Thank you for your interest. I am very much looking forward to hearing your opinion. Without you holding back. Truly. The last time after dust settled and emotions subsided I was able to analyze all the input and to come to conclusion myself that the design was, well, convoluted and unworkable and expensive. On many levels it was a good learning experience. So, you see, after all you were right back then. :-) My only disappointment after the first failed review and all those strong opinions was that nobody picked it up and nothing has come out instead... maybe such component is not as important as I imagine it is. Anyway, I am ready to listen, to adjust, to learn, to fail... :-) No regrets.

Paul, Thank you for your input. I'll try and address your comments in the coming next days. On 05/14/2014 11:24 PM, Paul A. Bristow wrote:

...

-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Edward Diener Sent: 13 May 2014 23:01 To: boost@lists.boost.org Subject: [boost] Boost review of the Convert library is ongoing

The review of the Convert library started Monday, May 12 and goes through Wednesday May 21. Many people may have missed the first announcement since the mailing lists were held back when the review started so I am announcing this again. The Convert library builds on the boost::lexical_cast original design and experience and takes those conversion/transformation-related ideas further.

* to be applicable to a wider range of conversion-related use-cases, * to provide a more flexible, extendible and configurable type-conversion framework.

...

Please try to sum up your review by answering these questions:

What is your evaluation of the design? Looks as though most users requirements can be satisfied.

...

What is your evaluation of the implementation? Not qualified to judge. What is your evaluation of the documentation?

Good - but could be improved.

* By providing the worked examples in full in a folder /example. Examples are the first place that many users look at - before the reference docs. I believe that the files exist (used to provide the code snips in the docs), or could be easily written.

* Some use of fonts for code I thought unusual (and not an improvement ;-)

* Using Quickbook snippets system to ensure that the code in the documentation really compiles and runs.

* Adding Doxygen comments to describe briefly the files, functions etc (so users can find which include files may be needed?)

...

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

Essential tool.

...

Did you try to use the library? No - through lack of time. How much effort did you put into your evaluation? A quick reading. Are you knowledgeable about the problem domain? No

Comments: I don't understand the timing figures given in Performance section. Some items refer to MSVC express 10, other detailed tables in Boost 1.55 to 11? No Clang results?

...

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

Yes - it has been through enough iterations to 'hit the streets' - and get further feedback.

Paul

--- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 01539 561830

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

alex writes:

This is a review from an enthusiastic C++ and Boost user, but relative outsider.

Alex, You've made quite a few suggestions. It'll take me some time to get through them all and try and evaluate and address them. So it's just a quick acknowledgement reply to say thank-you for your interest and input. It's very much appreciated. I certainly agree that in many respect the documentation is not sufficient and the implementation is far from perfect. At this point I have to be realistic -- my overall effort is measured against (and tempered by) the real possibility of all of it wasted (if the review fails). Consequently, this submission is more of a proof-of-the-concept. If the community finds that the overarching design is sound and sufficiently flexible, then I'll have an incentive ;-) and an obligation to bring all the relevant components (the docs, the code) up to the standards... based on the agreed/approved design and behind the approved api (obviously, if the submission is successful). Please find my further replies below. V.

I would use his library because it provides a simple way to convert between strings and values. I particularly like that it returns an optional value giving the user a convenient way of dealing with failing conversions. Also, it is very useful to be able to apply the modifiers familiar from stringstream.

I found the documentation quite lacking from a user's perspective. The enormous emphasis on boost::lexical_cast is a distraction and a pain for new users. This should be a simple library and its understanding should not depend on being familiar with alternative libraries. You could have a separate page discussing the differences between Convert and boost::lexical_cast.

During our first (failed) "convert" review quite a few years ago many questions were "why not use lex_cast, why not do as lex_cast does?, etc". So to avoid answering the same questions over and over again I added lex_cast in the docs.

In my opinion the documentation should start by explaining how to convert between strings and numeric values. Next it could tell how to deal with failed conversions. Finally it could explain how to deal with stringstream modifiers.

The documentation doesn't seem to say anything about conversion to strings. I believe that is possible too? Conversions from strings to strings seems to be not supported, is that on purpose? For instance conversion from const char* to std::string would not work it seems to me.

String-to-type is obviously possible and there is quite a bit to be done improving the docs and the code... after/if it's decided it's worth doing.

The documentation doesn't seem to say anything about the from(converter) member function that seems to return a unary function object.

I believe you are referring the function which is used with algorithms. I'll see what I can say/add/do about it. Thank you.

I found it confusing that the function "from" is used for conversion; I would expect there to be an equivalent "to" that does the reverse of "from", but there is not.

I personally did not see the need for convert::to. So I do not have it. What would be a use-case where you might need convert::to and convert ::from would not do?

I would therefore prefer "get", "do","parse" or "convert".

Well, to me "convert<int>::from<string>" seems like a good "translation" from English "convert int to string" to C++. I do not immediately see mentioned alternatives doing it so succinctly. That said, I am all willing to listen and learn if you provide usage/deployment examples/comparisons.

The library says it will start using tr1::optional at some stage. Why not use boost::optional already now? It would make things a lot simpler and clearer.

Because right now boost::optional does not have all that is needed. Andrzej is working on extending boost::optional and I'll use it as soon as it's available.

The following usage pattern seems indirect and verbose:

boost::stringstream_converter cnv boost::converter<int>::result v1 = boost::converter<int>::from(str, cnv); int v2 = boost::convert<int>::from(str, cnv).value(); //may throw int v3 = boost::convert<int>::from(str, cnv).value_or(-1);

Would it not be easier to use: boost::stringstream_converter cnv; boost::optional<int> v1 = cnv.get<int>(str); int v2 = cnv.get<int>(str).get(); // doesn't throw int v3 = cnv.get<int>(str).get_value_or(-1);

Or, using Koenig Lookup to get rid of all the namespaces:

boost::stringstream_converter cnv; boost::optional<int> v1 = convert<int>(str, cnv); int v2 = convert<int>(str, cnv).get(); // doesn't throw int v3 = convert<int>(str, cnv).get_value_or(-1);

Well, here (I feel) you are suggesting quite a different design of using converters directly rather than via agreed interface. IMO that lacks the user-provider contract that 1) advertizes to the user what to expect (regardless of the converter used) and 2) forces the converter writer to follow the contract. Without these restrictions converter implementations are destined to diverge, provide varying api, etc. So, the pluggability and replaceability qualities (important to me and for a large-scale development) are no more.

Would it make sense to define the operator << for the boost::stringstream_converter? It would make it possible to pass modifiers in the same way as with stringstream.

Indeed, I had that interface during the first review (3 or 4 years ago). The interface kinda makes sense when the stringstream-based converter is looked at in isolation. If there is a strong push for this interface, I'll add it. No drama.

I think the effort to present Convert as a general conversion library is not helpful.

Here I have to respectfully disagree. My conversion needs do go beyond str-to-int and the like... not as often but still. So having a consistent API to do unknown-at-this-moment (i.e. generic) type-to-type conversions is important to me. My immediate application is MBCS strings to UCS strings.

A major strength and interest of this library seems to me that it responds to the same locale and formatting modifiers as stringstream.

IMO the major strength is that one can choose converter/functionality/performance on as-needed basis. stringstream-based converter is just one of many. Important for some, too slow and bloated for others.

This is not something offered generically by all proposed converters,

Correct... at this point. There is nothing stopping anyone to extend the existing (explicitly labeled as prof-of-concept) or write new converters supporting locale, etc.

and therefore by presenting Convert as a generic conversion library, you are reducing the expectations one can have of the library. It is not really clear to me what the attraction of a 'general conversion library' is: a generic interface for a unary function object where the user specifies the input and return type?

To me the importance of Convert is in providing one interface to varying conversion facilities... rather than any particular converter in isolation. IMO one *generic* interface (to do conversions or anything else) is essential if one writes any *generic* code: template do_something(type_in in, type_out out) { ... convert::from(in, cnv); }

What has that to do with string-to-string encryption as suggested on the first page of the documentation?

String-to-string encryption is just one example of generic type-to-type conversion/transformation that the library can be deployed for.

I vote for inclusion in Boost, conditional to using boost::optional as a return type and a simplification of the interface. I would recommend dropping the idea of a pluggable general conversion tool and providing only the stringstream based conversion.

Well, I appreciate your vote for inclusion but I feel you have to reconsider either your vote or your evaluation of the *existing* design. By "simplification of the interface" and "dropping ...a pluggable" you are effectively suggesting/requesting a totally different overall design and API. To me it kinda sounds like "I vote for an autobahn but only if we build a railroad instead". So, first, you vote the autobahn down and then provide the specs for and vote on the railroad.

...

What is your evaluation of the design?

Good. Can be a bit slimmer and should make use of boost::optional.

I feel like I am being a pain in the neck but I am honestly confused. A pluggable converter seems to be the center-piece of the design that you do not seem to agree with.

...

What is your evaluation of the implementation?

I can't really judge, but looking at the code it seems some unused parts need to be cut out. Also, the operator() in basic_stringstream_converter seems overly complicated. Would it be an option to completely get rid of detail/string.hpp and use plain overloads? See suggested code below.

...

What is your evaluation of the documentation?

Poor, preoccupied by lexical_cast. Fails to document key functionality.

...

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

Good

...

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

Yes, VS2013, no problems at all.

...

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

I read the documentation, and had to revert to the source where the documentation was lacking. Studied some code that looked overly complicated in more detail.

...

Are you knowledgeable about the problem domain?

Hardly, but have had to convert between numbers and strings in the past and also had to deal with locale related problems. So would have liked to use this library.

...

And finally:

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

Yes. Conditional to using boost::optional as a return type and a simplification of the interface.

The interface might be perceived as complex due to it trying to cater different needs and usage patterns. My hope is that if one has one particular usage pattern (say, interested only in std::sstream-based convertor), then that generic interface can be wrapped up in a simpler custom api.

...

As always whether you do or do not feel that the library should be accepted into Boost please specify any changes you would like to see for the library to be better in your estimation.

Suggested code to reduce complexity of overloads in basic_stringstream_converter

template bool from_string(const StringIn& value_in, ValueOut& result_out) const { stream_.clear(); // Clear the flags stream_.str(value_in); // Set the content of the stream

return !(stream_ >> result_out).fail(); }

template bool operator()(TypeIn const& value_in, StringOut& result_out) const { stream_.clear(); // Clear the flags stream_.str(StringOut()); // Clear/empty the content of the stream

return !(stream_ << value_in).fail() ? (result_out = stream_.str(), true) : false; }

template bool operator()(const std::basic_string& value_in, TypeOut& result_out) const { return from_string(value_in, result_out); }

template<typename TypeOut> bool operator()(const char_type* value_in, TypeOut& result_out) const { return from_string(value_in, result_out); }

I'll see if and how I can incorporate your suggestions. Thank you.

Edward Diener writes:

On 5/15/2014 9:42 PM, Vladimir Batov wrote:

...

alex writes: ... Well, to me "convert<int>::from<string>" seems like a good "translation" from English "convert int to string" to C++.

I think you just misspoke here Vladimir, and meant "convert to int from string".

Indeed, Edward. Thank you. I only noticed it after I posted. :-( I actually meant "convert int from string" analogous to "extract gold from ore"... but now I am not sure if that's acceptable usage of "convert" and 'to' as in "convert to int from string" is needed.

2014-05-16 3:42 GMT+02:00 Vladimir Batov :

alex writes:String-to-type is obviously possible and there is quite a bit to be done

...

The library says it will start using tr1::optional at some stage. Why not use boost::optional already now? It would make things a lot simpler and clearer.

Because right now boost::optional does not have all that is needed. Andrzej is working on extending boost::optional and I'll use it as soon as it's available.

Hi, I am a co-maintainer of boost::optional, I have already risen this point with Vladimir, and I think his decision to stick with his own utility for representing optional objects (rather than using boost::optional) for the time being is the right one. Also, Vladimir's library and its use cases reveals certain deficiencies in boost::optional: 1. Boost.Optional is not moveable. 2. Member function boost::optional<T>::get_value_or is slightly different than that of the proposed std::tr2::optional<T>::value_or. Vladimir opts to use the STD naming, which appears the right choice. 3. There is no getter function in boost::optional that treats the absence of the expected contained value as an exceptional situation (but not a bug) that causes an exception to be thrown (so that the user can choose whether it is a bug or an exceptional situation when a value is absent). This is a similar situation to functions at() and operator[] on std::vector. It is solved in std::tr2::optional. These things are addressed in tr2::optional; see here for details: https://github.com/cplusplus/fundamentals-ts/blob/master/fundamentals-ts.pdf. They should be fixed in boost::optional too, and I am working on them, but in my own slow pace. boost::optional is already moveable in develop and master branches. We are just waiting for the next Boost release. Until the above issues with boost::optional are addressed I would recommend for Vladimir to stick with his own implementation, so that his library does not suffer from the coupling with boost::optional. Regards, &rzej

alex writes:

...

Vladimir Batov writes alex writes:

... Alex, Thank you for your email, continuous involvement and suggestions. It's much appreciated. I took the liberty re-arranging our conversation... mostly for selfish reasons ;-) -- 1) the inlined replies were getting difficult to follow and 2) later it'll be easier for me to address a "bulleted" (as below) TODO list. Apologies if I managed to miss any of your points. Vladimir. RE#1: I vote for inclusion in Boost, but would strongly recommend that the library simplifies the interface AND / OR develops a concept of Converter that is rich enough to include the advanced functionality of the stringstream_based converter. REPLY: Thank you for voting for inclusion and for stating that you find the library useful (at least potentially). It's much appreciated. I whole-heartedly agree that we neeed a rich and advanced stringstream_based converter. In practice that happens to be my main converter as well (conversion performance is not a priority for me). Extending this particular converter is close to the top on my TODO list (the usual caveat: if the review successful). However, I'll need to gather user input/requirements what is the preferred interface, etc. Namely, I was leaning away from using the std manipulators (and was providing an alternative api) but you found it useful. So, now I feel we should be providing both interfaces. TODO: Provide "rich and advanced" stringstream_based converter. RE#2: Basic things that I would expect from a *generic* converter are not offered by the Convert library. REPLY-PART#1: Clearly there is no code that would satisfy every user's need. Far from it. *Currently* we are at the "consideration/evaluation" stage to see if that is the correct/valid/promising/sustainable *path* to go ahead and to implement all those bells and whistles. REPLY-PART#2: Well, the thing is -- there is no *generic* converter. More so, in my view there is no even Convert "library". :-) I see my proposal as a conversion/transformation *framework*. The difference between a library and a framework (in my view and in this particular case, anyway) is that the latter is merely the guidelines how to be able to develop code *later* that is to be deployed in and without disrupting the old/existing code. That's what convert::from API facade does. On its own it provides very little (if any) functionality. Its (sole?) purpose is to advertize and to force a *contract* between the user (service consumer) and the converter developer (service provider). That allows the user to write his domain-specific code *now* and to plug some converter later (when it becomes available) or to replace one converter with another (if/when the requirements change) without major disruption of the existing code-base. As you correctly pointed out one-particular-converter functionality can be called directly rather than via "meaningless" convert::from. The existence of Convert API facade is more for the purposes of *managing* a large development which includes *simultaneously* writing components which are meant to interact or rely on each other; adjusting to requirement changes without major existing code disruption (that includes major re-writes, major re-testing, etc.). That's why to this point I am not exactly sure if such an "administrative" proposal really belongs in Boost where it is overwhelmngly about clever code. RE#3: too much lexical_cast-related paranoia and too many lexical_cast references in the "convert" documentation. TODO: Focus documentation of "convert", create a separate lexical_cast-comparison-related chapter. RE#4: The following is not compiling because of the way it uses enable_if in sstream.hpp. boost::cstringstream_converter cnv; std::string str = boost::convert::from("hello", cnv).value(); JUSTIFICATION: It's not compiling because applying std::stringstream to such a trivial type transformation is wasteful. TODO: Add "char const*->std::string" transformation specialization to sstream-based converter. RE#5: I also think that for a concept this basic, the implementation should not pose unnecessary restrictions; The convert<>::from function can only convert to default-constructable types, which I think is not necessary for a *generic* converter. REPLY: The statement above is incorrect. The default-constructible requirement was actually the main reason I was forced to stop using lexical_cast and to develop "convert". Please read "The Default Constructible Type Requirement" chapter. RE#6: My feeling is that you didn't clarify the concept of a converter except that it is a function where something goes in and something goes out. So you might have just as well have called it "algorithm" or "function". ANSWER: In the documentation it is called "callable". I am sure it is a fairly standard name, isn't it? TODO: Extend/elaborate the "Converters" chapter. It purpose/requirements. RE#7: Did you mean:

template do_something(type_in in, type_out out) { ... convert::from(in, cnv); }

template void do_something(type_in in, converter cnv) { ... type_out out = convert::from(in, cnv); } REPLY: It was merely an example where one might need a generic interface to be able to plug in some functionality later. A converter can be passed in as in your example... or as a template parameter: type_out value_out = do_something(value_in) Clearly my chosen order of template-type arguments can be improved to allow for type deduction.

-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Vladimir Batov Sent: 17 May 2014 00:54 To: boost@lists.boost.org Subject: Re: [boost] Boost review of the Convert library is ongoing

Vladimir Batov writes:

...

alex writes: ... RE#4: The following is not compiling because of the way it uses enable_if in sstream.hpp.

boost::cstringstream_converter cnv; std::string str = boost::convert::from("hello", cnv).value();

JUSTIFICATION: It's not compiling because applying std::stringstream to such a trivial type transformation is wasteful.

TODO: Add "char const*->std::string" transformation specialization to sstream-based converter.

Hmm, now I am not sure if I agree with my own "justification". :-) What if I want to apply std::sstream-provided formatting facilities to convert from, say, a "0xFF" hex string to a decimal string? Is it one of "char const*->std::string" transformations that you had in mind? I've never thought of this and never tried... will it even work with std::sstream?

That's not what I had in mind, and in the example you give I would expect that the user (not the library) first converts the hex string to a number and then the number to a decimal string. I just expected the converter to be able to convert any Input Streamable value(including string) into a string, and a string into an Output Streamable value (including string). That is how I understood the type requirements. The type-requirements page also seems to suggest that you can use the stringstream convert for (for instance) from float to int, but I assumed that is not intentional?

On 05/20/2014 01:41 AM, alex wrote:

... This is about needing to specify what the "contract" for a converter is. At the moment all you specify is an object that is callable with an input and creates an output. To me that is too generic and I do not really see how this "contract" is helpful.

I think I see where our disconnect might be. I only apply the term "contract" to the formalization of how service-consumer (the user) and service-provider (the converter developer) "interact". That interaction is defined by boost::convert::from interface. So, that "contract" (in my interpretation) tells the user -- as long as you deploy various conversion facilities via agreed interface, you can expect consistent behavior and you can plug in other compatible converters. Then, to the service provider it states -- as long as you conform to the agreed "callable" interface for your converter, it can be used within boost::convert framework. That's it. You perceive it as "too generic". To this point to me it seems sufficient -- that allows me, the user, to replace one converter with another one if/when needed. Say, I started with strtol-based converter but later due to a requirement change needed additional formatting. So, I deployed std::sstream-based converter instead with very little impact on my application code. In my view there is no "contract for converter"... apart from the *requirement* it is a "callable" with a certain signature.

Conversion to/from string requires a different contract than conversion between types, encryption or conversion from Celsius to Fahrenheit.

I fail to see how/why these mentioned different type conversions/transformations "require different contracts". They indeed require different converters but the consumer-provider contract remains -- the user deploys converters in the same way and the behavior of boost::convert::from are the same. An every-day example. The electricity company provides the electricity and a socket. You get to use that power with whatever you plug into that socket as long the socket (interface) matches and the plugged-in device conforms to the requirements. It's short but it's still an agreement/contract. To the user -- you follow these rules, you get that... To the provider -- you provide your service a certain way, you get it used... That's how I see it anyway.

At the moment you are so generic to cover all these types of conversions, but with no (IMHO) added value. If you want to add "bells and whistles" at some point you need to be clear about what type of conversion you are supporting and what functionality comes with that.

Again, boost::convert::from by itself does not do and does not support any conversions. It only specifies *how* converters are to be deployed uniformly. "Bells and whistles" are provided (or not) by the respective converters. Some converters provide performance but no bells, some other the other way around. I am not trying to provide all imaginable conversions. I am only suggesting how to deploy those uniformly.

...

...

REPLY: The statement above is incorrect. The default-constructible requirement was actually the main reason I was forced to stop using lexical_cast and to develop "convert". Please read "The Default Constructible Type Requirement" chapter. The user still needs to supply a Default Construction function it is just not the default constructor. I can see that this requirement follows from the stringstream implementation.

No, it's not a requirement of std::sstream but rather imposed by boost::convert::from.

But it does not seem a reasonable requirement for a generic converter (nor is the requirement of Copy Constructible). All you can reasonably require is some function that creates a TypeOut from a TypeIn.

I feel you mix up requirements for converters and requirements for types. The only req. for converters is that they are a callable with a certain signature. As for requirements for TypeIn and TypeOut, then those listed requirements are needed for the framework to work. I was not able to come up with a better implementation that would not have those requirements. If you have such an implementation, I'll incorporate it gladly.

My issue is not that you require Default Constructible (through a function) or Copy Constructible, but that the value-to/from-string library is being sold as a generic conversion solution.

No, it is not claimed to be a "generic conversion solution". It is a framework, policy, guidelines, the interface to deploy actual conversion solutions (converters) uniformly.

These are reasonable requirements for this particular library, but not for a generic conversion contract.

How would you write an efficient converter for the following?

template<typename T> struct large_vector { large_vector(const T& v) : m_vector(v, 10000000) {} std::vector<T> m_vector; } ;

The proposal does not address how to write an efficient converter. It offers an interface how to deploy varying conversion facilities uniformly.

I am sorry to be sounding so critical. To remind you: I would use this library, because I like the functionality.

Thank you for the remainder. :-) As far as I am concerned, I do not perceive your comments as critical (in negative sense). I see it as a normal and healthy technical discussion.

template void boost::large_vector_converter operator() (const TypeIn& in, boost:: optional <TypeOut>& out) { try { out = large_vector< TypeIn >(in); } catch() { out = boost::none; } } }

This would be more correct: struct large_vector_converter { template<typename T> void operator()(const T& in, boost::optional& out) const { try { out = boost::in_place(in); } catch (...) {} } };

I am not arguing against interface/behavior specifications (a.k.a. "contracts") for converters. I only feel that "convert"'s purpose is far more modest -- to only specify how to deploy various converters and what to expect when we do. It seems far beyond my capabilities and expertise to come up with "contracts" you listed.

OK, then I would expect the contract for a highly-generic converter to pose minimal requirements on the Types to convert, preferably none.

...

...

...

The user still needs to supply a Default Construction function it is

just

...

...

...

not the default constructor.

I feel it's a generic converter requirement -- they all need a "place" to write the result to. We might say -- every converter for itself. On the other hand, that seemed like a generic enough requirement that the "framework" might care providing uniformly to every converter.

It is not a generic converter requirement IMO. Some converters might just create their own place. As the example that I gave.

Well, I feel you are not avoiding requirements but merely shifting them around from boost::convert() down to converters. A converter will still need to be able to create and TypeOut instance somehow and in a generic way. So, I feel, we'll be back where we are now with the burden of every converter having to do it itself.

The specification of requirements is shifted down because some converters have the requirement and others not.

...

...

I was not able to come up with a better implementation that would not

have

...

...

those requirements. If you have such an implementation, I'll incorporate it gladly.

That seems like a 0-gain game as we still have all the same requirements and all the same code, right?

No the requirements are not the same. Using the boost::optional from master the only requirement for the large_vector_converter TypeOut is that it is MoveConstructible. In other words it is possible to have the same interface without requiring DefaultConstructible (indirect). Moreover, even though it is required to be MoveConstructible in practice the MoveConstructor will not be invoked because of return value optimization: the large_vector is only constructed once. Still the requirement of MoveConstructible is not strictly necessary and you could write your result type in to not require it (something more like std::unique_ptr than boost::optional).

We still need to be able to create a TypeOut instance somehow generically.

Well, not generically, the actual converters can be very specific.

Although the code above forces every converter to do that by themselves, right? Although passing boost::optional into a container might have its benefits... I had it at one point... will have to re-visit again.

The benefit is that it defers initialization and thus passes that responsibility from the generic function to the specific converters.

That's something I'll have to think about. What jumps at me though is what I'd call "blur of responsibilities" -- now converter not only cares for conversion but for allocation as well. Secondly, in the current "convert" design converters know nothing about boost::convert "framework". The implementation above seems to require the converter to know/deploy boost::converter_default_maker which is either part of the "convert" infrastructure or converter's own code, i.e. the converter has to have either additional "knowledge"/coupling or additional code. In both cases I do not like the "additional" bit. :-)

Some converters require this additional code/knowledge and others not. For instance, your encryption converter would probably not need it. On a completely different note. In the cstringstream_converter you may want to check that the stream_.str() is empty after conversion. For instance, would you like the following to return 7 or 999? return boost::convert<int>::from("7 + 5", cnv).value_or(999); If you check(and clear) after conversion, you probably do not need to clear before conversion, so there may be no penalty at all?

2014-05-23 2:25 GMT+02:00 Vladimir Batov : [...] template

boost::optional<TypeOut> operator()(TypeIn const&); //#3

surely seems attractive and with that interface converters can be easily used on their own (if needed/preferred) bypassing "convert" api altogether.

Let's take a simple case of, say, the "direction" class from the docs (which has no default constructor) and try converting it to std::string. Say, we want that conversion possible using a few converters with std::sstream-based being one of them. In that setting I say #1 has the following advantages (no matter how ugly I personally find it):

[...]

template

boost::optional<TypeOut>

basic_stringstream_converter::operator()(const TypeIn& in) { boost::optional<TypeOut> out = boost::make_optional(boost:: convert_default_maker<TypeOut>::make());

stream_.str(std::basic_string< char_type>()); stream_ << in; stream_ >> *out;

return out = stream_.fail() ? boost::optional<TypeOut> : out; }

template boost::optional<TypeOut> lexical_cast_converter::operator()(const TypeIn& in) {

try { boost::optional<TypeOut> out_value = boost::make_optional(boost:: convert_default_maker<TypeOut>::make()); *out_value = boost::lexical_cast<TypeOut>(in); return out; } catch (...) { return out = boost::optional<TypeOut>; } }

Why would you make a default TypeOut to then assign to it, instead of: template boost::optional<TypeOut> lexical_cast_converter::operator()(const TypeIn& in) { try { return boost::lexical_cast<TypeOut>(in); } catch (...) { return boost::none; } } Regards, Kris

As for the

optional<TypeOut> operator()(TypeIn const&);

converter signature, then I am slowly warming up to it. If we declare something like

...

Alex, does it make you happy? Could anyone comment on that please?

It would make a lot of sense to me. However I am not sure about the following options: #2 void operator()(TypeIn const&, optional<TypeOut>&); #3 optional<TypeOut> operator()(TypeIn const&); Even though #3 looks neater, it requires explicit parameter declarations which I find pretty ugly for operator(): #2 optional<T> out; converter(in, out); #3 optional<T> out = converter.operator()<T>(in); That may not be a big issue, since users are expected to use the convert<T>::from() function. Another advantage (that seems more substantial) of the implicit template resolution of #2 is that the function can use template parameters other than TypeOut and it is therefore easier to provide specializations through overloads. template<class TypeIn> void operator(const TypeIn& in, optional<TypeIn>& out){...} template void operator(const TypeIn& in, optional >& out){...} template void operator(const TypeIn& in, optional<TypeOut>& out){...}

...

...

The benefit is that it defers initialization and thus passes that responsibility from the generic function to the specific converters.

Let's try staying with mainstream usage examples meaningful for the majority of users. I'll unlikely be persuaded by an esoteric academic (even a real but corner-case) example that will have negative impact on the real users. Thoughts?..

The convert contract you are proposing is quite abstract, so I think academic arguments are relevant. But your question can be translated to: are there real examples of classes that are expensive to construct but cheap to move? Moreover, these classes need to be potential candidates for being converted to. The example you have been using is encryption. Consider the following class for an encrypted password: struct encrypted_password { encrypted_password(const std::string& password) { // connect to random number device // do all kinds of arithmetic to encrypt // send email to NSA // m_encrypted will have a minimum size of 128. } encrypted_password (password_encryption&& rhs) : m_encrypted (std::move(rhs.m_encrypted)) { } bool check_password(const std::string& password){} private: std::string m_encrypted; }; Now I can write a converter: struct password_converter { void operator()(const std::string& in, boost::optional& out) { try { out = password_encryption(in); } catch(...) { out = boost::none; } } };

On 05/22/2014 01:54 AM, feverzsj wrote:

hi, list here are some personal opinions/thoughts on the convert lib:

1. in most cases, I found directly use of the converter would be more convenient:

int v = -1; // default value // no need to give the type of 'v' bool re = printf_converter()("123", v); // on failure, return false, and 'v' is untouched

so, I suggest add a default convert function using a default converter:

template bool convert(Src const& src, Dst& dst) { return default_converter()(src, dst); }

I am not the "list" ;-) but I'll take the liberty of replying. :-) I feel that one particular message that I am trying to make just does not make it. :-( It might be that my reasoning/idea behind that message is wrong or I might be a bad messenger. Still, I'll try again. Indeed, converters can be called directly. However, I advocate that calling various conversions via one(!) familiar/known boost::convert() interface ensures *known* behavior, better readability, better adaptivity to requirement changes (when one needs to upgrade/replace already deployed converter, etc.). string res = lexical_cast<string>(foo); string res = foo.str(); Above both are foo-to-string converters. Uhm, well, at least we *know* that the first one *is* and we *guess* that the second one *might* be also. IMO #1 is *instantly* recognizable and understood what it does and the purpose of the call. As for #2 we might *guess* what it does but in reality we do not know until we diverge and learn what foo.str() *actually* does, what its caveats, etc.

2. besides the generic ones, sprintf/sscanf converter still lacks some format support from sprintf/sscanf: for example: auto-determined integer string, plus sign, padding, float point format, etc. with most formats being supported, this converter would be the most powerful, precise and efficient one;

3. printf_converter() uses snprintf(), while on msvc the return value semantic is not standard conform;

4. printf_converter() uses fixed length char buf, while it is totaly possible to remove this restrict with a standard conform snprintf();

I have not tried to provide full-blown converters. They all marked as work-in-progress and proof-of-concept. For my every-day work I am content with std::sstream-based one and I do not feel I have the capacity to provide all imaginable converters... that was not the idea anyway. So, if you feel some converter(s) is/are lacking in functionality, please feel free to submit it/them. I'll gladly incorporate and mention your contribution... well, if the "convert" succeeds through this ongoing review... which you might have forgotten to vote on... :-)

On 05/22/2014 08:40 PM, alex wrote:

...

From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Vladimir

...

On 05/22/2014 01:54 AM, feverzsj wrote: hi, list here are some personal opinions/thoughts on the convert lib:

1. in most cases, I found directly use of the converter would be more convenient:

Indeed, converters can be called directly. However, I advocate that calling various conversions via one(!) familiar/known boost::convert() interface ensures *known* behavior, better readability, better adaptivity to requirement changes (when one needs to upgrade/replace already deployed converter, etc.). If you really want only one known convert interface, maybe your design should reflect that.

Indeed, and I feel the current design does just that -- one interface -- boost::convert::from() (for the version under the review).

At the moment you have one familiar / known interface for converter developers (success = cvr.from(in, out)) and one for converter users (out = convert<int>::from(in, cvr)).

Either I am confused or you misinterpret the meaning of "interface". Interface is for the users... that's why it's called "interface" in the first place... IMO anyway. "convert" does not have any interface that converter developers need to use. In fact, converters are not aware or coupled with "convert" infrastructure. What "convert" says with regard to converters is -- if you want to be incorporated into "convert" framework, you need a certain signature -- it's called a requirement.

Whereby the boost::convert<> acts as the middle man. Anybody looking for efficiency will consider eliminating the middle man.

1) With that way of thinking we need to eliminate all the human managers, team leaders, etc. from the process. Even though sometimes I feel that way ;-) it's not for the reasons you mention. Big projects won't survive without them. "convert" interface provides the same benefits what lexical_cast does. If you do not appreciate advantages of lexical_cast deployment over "direct" calls, then I won't be able to convince you of "convert" interface advantages. 2) Anybody looking for efficiency will run a profiler and will see that that mentioned "middleman" is nowhere on the list. More so, if you really care for efficiency, try compiling optimized and then try finding that "middleman" anywhere. :-)

I don't see the problem of having two familiar/known interfaces; I believe it is quite usual to have convenience functions for users.

My experience tells me otherwise. I find two interfaces unnecessary, confusing, complicating maintenance and readability. Why I need two when one does it?