Boost review of the Convert library is ongoing

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
39 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Boost review of the Convert library is ongoing

Edward Diener-3
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.

The Convert library can be cloned from GitHub at
https://github.com/yet-another-user/boost.convert. The library follows
the modular-boost format. Just clone it to modular-boost/libs in the
'convert' subdirectory and run 'b2 headers' in order to create a link to
its header file directory in the modular-boost/boost subdirectory.

The library comes with documentation in its top-level index.html or
doc/html/index.html file. You can also view the documentation online at
http://yet-another-user.github.io/boost.convert/doc/html/index.html.
The library is by Vladimir Batov and is a second reiteration with a
greater focus of his library that was reviewed in the past. I am Edward
Diener and I am again serving as the review manager of the library.

If you have used lexical_cast or, like many C++ programmers, have used
stringstream to do string-to-type, type-to-string conversions please
look at this library. We need reviews of whatever point of view before a
library can even be considered a Boost library.

Comments, questions, and reviews will all be welcome for the library.
Please try to sum up your review by answering these questions:

     What is your evaluation of the design?
     What is your evaluation of the implementation?
     What is your evaluation of the documentation?
     What is your evaluation of the potential usefulness of the library?
     Did you try to use the library? With what compiler? Did you have
any problems?
     How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
     Are you knowledgeable about the problem domain?

And finally:

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

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.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Niall Douglas
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.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ 
http://ie.linkedin.com/in/nialldouglas/




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

SMime.p7s (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Edward Diener-3
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.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

pabristow
In reply to this post by Edward Diener-3


> -----Original Message-----
> From: Boost [mailto:[hidden email]] On Behalf Of Edward Diener
> Sent: 13 May 2014 23:01
> To: [hidden email]
> 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
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Jeroen Habraken
In reply to this post by Edward Diener-3
On 13 May 2014 17:07, Edward Diener <[hidden email]> 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

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Edward Diener-3
On 5/14/2014 1:47 PM, Jeroen Habraken wrote:

> On 13 May 2014 17:07, Edward Diener <[hidden email]> 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.

The review has been extended through Sunday. May 25.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov-5
In reply to this post by Jeroen Habraken
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.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov-5
In reply to this post by pabristow
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:[hidden email]] On Behalf Of Edward Diener
>> Sent: 13 May 2014 23:01
>> To: [hidden email]
>> 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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

alex-2
In reply to this post by Edward Diener-3
This is a review from an enthusiastic C++ and Boost user, but relative
outsider.

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.

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.

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

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 would therefore prefer "get", "do","parse" or "convert".

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.

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::converter<int>::from(str, cnv),value(); //may throw
    int v3 = boost::converter<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);

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.

I think the effort to present Convert as a general conversion library is not
helpful. A major strength and interest of this library seems to me that it
responds to the same locale and formatting modifiers as stringstream. This
is not something offered generically by all proposed converters, 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? What has that to do with string-to-string encryption
as suggested on the first page of the documentation?

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.


>     What is your evaluation of the design?

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

>     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.

>
>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<typename StringIn, typename ValueOut>
    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<typename StringOut, typename TypeIn>
    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<typename TypeOut, typename T, typename A>
    bool operator()(const std::basic_string<Char, T, A>& 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);
    }


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov
alex <alexhighviz <at> hotmail.com> 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<class type_in, class type_out, class cnv>
do_something(type_in in, type_out out)
{ ...
    convert<type_out>::from<type_in>(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<typename StringIn, typename ValueOut>
>     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<typename StringOut, typename TypeIn>
>     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<typename TypeOut, typename T, typename A>
>     bool operator()(const std::basic_string<Char, T, A>& 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.




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Edward Diener-3
On 5/15/2014 9:42 PM, Vladimir Batov wrote:

> alex <alexhighviz <at> hotmail.com> 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 think you just misspoke here Vladimir, and meant "convert to int from
string".

> 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<class type_in, class type_out, class cnv>
> do_something(type_in in, type_out out)
> { ...
>      convert<type_out>::from<type_in>(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<typename StringIn, typename ValueOut>
>>      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<typename StringOut, typename TypeIn>
>>      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<typename TypeOut, typename T, typename A>
>>      bool operator()(const std::basic_string<Char, T, A>& 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.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov
Edward Diener <eldiener <at> tropicsoft.com> writes:

> On 5/15/2014 9:42 PM, Vladimir Batov wrote:
>> alex <alexhighviz <at> hotmail.com> 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.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Rob Stewart-6
In reply to this post by Edward Diener-3
On May 15, 2014 11:28:59 PM EDT, Edward Diener <[hidden email]> wrote:

>On 5/15/2014 9:42 PM, Vladimir Batov wrote:
>> alex <alexhighviz <at> hotmail.com> writes:
>>
>>> 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 think you just misspoke here Vladimir, and meant "convert to int from
>string".

I noticed that and was going to say that he was arguing for "to" rather than "for".

The alternatives do not read well and having the result type on the left, plus the input type near the argument justifies the use of "from".

[snip entire remainder of Vladimir's message, including the ML footer]

Please don't over-quote.

___
Rob

(Sent from my portable computation engine)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Andrzej Krzemienski
In reply to this post by Vladimir Batov
2014-05-16 3:42 GMT+02:00 Vladimir Batov <[hidden email]>:

> alex <alexhighviz <at> hotmail.com> 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

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

alex-2
In reply to this post by Vladimir Batov
----
> Vladimir Batov writes
>
>alex <alexhighviz <at> hotmail.com> writes:
>
>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.

I think you were right to add the comparisons, and I should have mentioned
that you make the case very succinctly and clear. I just think that by
interspersing this with the actual documentation of the library you are
addressing the (past) reviewers and not the future users.

>>...
>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.

I understand but it is the existing library that is being reviewed. 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<std::string >::from("hello",
cnv).value();



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

You are right and I now agree you should keep it that way. I would not have
gotten confused if the documentation also covered conversion to string. My
first reading of the code was that the reverse of "i =
boost::convert<int>::from(str, cnv)" would be "str =
boost::convert<int>::to(i,cnv)", but I certainly don't mean to suggest this
would be better. Just that I got confused.



>> I think the effort to present Convert as a general conversion library is
>> not helpful.
>
>Here I have to respectfully disagree.

...

>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:
>


But your *generic* interface only captures part of the features that make
the specific cstringstream_converter stand out from for instance
lexical_cast.


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


Did you mean:

template<class type_in, class type_out, class converter>
void do_something(type_in in, converter cnv)
{ ...
    type_out out = convert<type_out>::from<type_in>(in, cnv);
}

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".

Basic things that I would expect from a *generic* converter are not offered
by the Convert library. For instance in the function above I cannot check
whether the converter is able to convert from type_in to type_out. Also some
advanced functionality that would make a *generic* converter library very
useful is not made generic in your library. For instance, In the function
above I cannot use something like cnv(std::hex) because you haven't made the
concept of applying modifiers on a converter object generic.

For instance some of the following would be useful:

converter<int>::apply_modifier(cnv, mdf)

traits<converter>::can_convert<In,Out>
traits<converter>::supports_std_modifiers

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.  

...
>
>> 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.
>

I voted for inclusion because I would use the library, and would prefer it
over lexical_cast in most applications. However I would strictly use it for
the stringstream based converter. I am not sure about the need for a
*generic* conversion library, but if there is need for one it should be
better attuned to the specific nature of conversion tasks and not be a
catch-all concept to do something with something. Moreover, I think that the
way in which Convert invokes the converter object makes it more awkward to
use than necessary. I understand your analogy; my perspective however is
that you created a splendid autobahn but try to sell it as a generic
solution to connect A to B with potential applications in transportation,
communication and time-travel.

I can give my review more nuance by saying:

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.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov
alex <alexhighviz <at> hotmail.com> writes:
>> Vladimir Batov writes
> alex <alexhighviz <at> hotmail.com> 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<std::string >::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<class type_in, class type_out, class cnv>
>do_something(type_in in, type_out out)
>{ ...
>    convert<type_out>::from<type_in>(in, cnv);
>}
>

template<class type_in, class type_out, class converter>
void do_something(type_in in, converter cnv)
{ ...
    type_out out = convert<type_out>::from<type_in>(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<type_in, type_out, converter>(value_in)

Clearly my chosen order of template-type arguments can be improved to allow
for type deduction.




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov
Vladimir Batov <vb.mail.247 <at> gmail.com> writes:
> alex <alexhighviz <at> hotmail.com> 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<std::string >::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?




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

alex-2


>-----Original Message-----
>From: Boost [mailto:[hidden email]] On Behalf Of Vladimir
>Batov
>Sent: 17 May 2014 00:54
>To: [hidden email]
>Subject: Re: [boost] Boost review of the Convert library is ongoing
>
>Vladimir Batov <vb.mail.247 <at> gmail.com> writes:
>> alex <alexhighviz <at> hotmail.com> 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<std::string >::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?



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

alex-2
In reply to this post by Vladimir Batov
>Vladimi Batov writes
>> alex <alexhighviz <at> hotmail.com> writes:
>


>TODO: Provide "rich and advanced" stringstream_based converter.

I actually already consider the stringstream_based converter rich and
advanced, my suggestion was that you would incorporate this behaviour into
what you call the "contract" for conversion

>
>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.
>

These are not specific bells and whistles I want. 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. Conversion to/from string requires a different contract than
conversion between types, encryption or conversion from Celsius to
Fahrenheit. 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.

> force a *contract* between the user (service consumer) and the converter
developer
>(service provider).

My problem is that the contract hardly specifies anything, and what it does
specify is only partly necessary.

> 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.

To me it sounds a lot like Boost Property Map that also seems to derives its
value from being a useful contract, rather than a clever implementation.

>
>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. 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.

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. 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;
} ;

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: Boost review of the Convert library is ongoing

Vladimir Batov-5

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.




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