Re: [boost] Convert library -- Andrzej's review

Subject: Re: [boost] Convert library -- Andrzej's review
From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2014-05-18 17:48:02

2014-05-18 15:16 GMT+02:00 Vladimir Batov <vb.mail.247_at_[hidden]>:

> Andrzej Krzemienski <akrzemi1 <at> gmail.com> writes:
>
> > This is the review of the Convert library. My vote is a CONDITIONAL YES:
> > this library should be accepted into Boost subject to conditions
> described
> > at the end of this message.
> > ...
> > If I was only passing my string as an argument, this
> > would be fine, but when I am also passing the converter object, it reads
> > that I am converting from both the converter and the string.
>
> Yes, I remember you suggesting defaulting the converter parameter to the
> std::sstream-based and my initial intention to do just that. I then backed
> off as I realized the create-converter-once-use it-many-times can be easily
> lost. If we could come up with something like
>
> boost::convert::use(some_converter);
>
> int v = convert<int>::from(str); // some_converter used
>
> That might address your concern, right? Any crazy tricks to achieve that
> without inheritance?
>

No; no globals, please. My remark is only about the choice of the name
("::from"): not the interface. In the other place you are suggesting to
drop the "::from". This would satisfy my taste. The converter should be
passed as the argument to the facade, or at least be the member of the
state-full facade. Your choice is the right one.

> > My condition on the acceptance of the library is that its performance
> > should be improved as indicated by others, in particular, provide move
> > semantics for boost::convert<TypeOut>::result (on compilers with rvalue
> > references). It is enough that you commit to doing it in the future.
>
> My intention, in fact, is to ditch convert<>::result for boost::optional
> given that you'll be pushing it into 1.56, right?
>

Well, at this point there are chances that all the necessary changes to
Boost.Optional will not make their way to 1.56.0 release. Currently move
semantics are ready, but function value() and value_or() are still missing
and my velocity has shrunk since my daughter was born. It is also not
certain when 1.56 is scheduled to be released. So I recommend that for 1.56
you have some plan B: either stick to your own type, or accept
Boost.Optional with the limited (for the time being) interface.

Regards,
&rzej