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 09:21:27

I forgot to answer the standard questions, so here they are:

What is your evaluation of the design?
>

It is good: the converter concept has been adequately identified,
considering performance. The responsibilities (the conversion itself an the
usage conveniences) have been correctly separated.

What is your evaluation of the implementation?
>

It is satisfactory.

What is your evaluation of the documentation?
>

I must admit that had it not been for the email exchanges and discussions
here in the group, I would have troubles figuring out what the library is
capable of from the documentation. I mean, every piece of information is
there, but the focus on the comparisons with lexical_cast is distracting.
The first thing I want to know about the conversion library is: is it a
ware of different locales and if I can easily (without performance
implications) check the conversion failure. I would expect the initial page
to be short(er) and give answers to my questions.

Apart form the above remark, I believe that the documentation contains
every piece of information that is needed.

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

Very useful! There is no standard satisfactory way of converting a string
to an int/float in C++ community.

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

I played with the pre-review versions on VC10. Basic examples worked fine.

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

I studied the documentation of this and the previous versions. Looked at
the implementation of the facade of pre-review versions and had a short
look at the current.

Are you knowledgeable about the problem domain?
>

I often need to convert, and had occasionally need to write my own simple
conversion functions

Regards,
&rzej

2014-05-17 22:33 GMT+02:00 Andrzej Krzemienski <akrzemi1_at_[hidden]>:

> Hi,
> 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.
>
> This is a good and useful library. This is what I needed for a long while:
> a tool that allows me to convert a string to an int or float, but that
> would not treat the failure to convert as something unusual, and that would
> be locale-aware. Convert library offers this. It also offers other things
> that I fail to appreciate, but I accept that it is supposed to satisfy more
> expectations than just mine.
>
> Your choice of the generic type converter:
>   bool operator()(TypeIn const&, TypeOut&);
> I think it is the right choice: this is the signature that does not impose
> any run-time overhead. Unlike boost::convert<TypeOut>::result, which does
> impose some run-time overhead of copying (or at least moving) the TypeOut.
> But that's acceptable. If the conversion needs to be super fast, one can
> fall back to using the converter directly.
>
> I appreciate the stringstream-based converter most. About the ability to
> plug other converters, I am just not convinced. Why would I use other
> converters? I know, lexical_cast is supposed to be faster in some cases,
> but if I was interested in this performance I would have used lexical_cast
> directly. The alleged ability to use this library to change the encoding of
> the strings or to convert between "related" types like ptime and time_t and
> Time -- I am not convinced: do you expect such conversions to fail and to
> report such failure the same way as you report the impossibility of
> interpreting a string as a float?
>
> I personally do not like this trick to force me to type this "from" in
> "convert<int>::from". 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. But I do not
> intend to argue over it. You wrote the (good) library, so you have the
> privilege to pick the name.
>
> I am also not convinced about the potential of the 'facade' part. I can
> see that you can see the potential in it. But to me separating the library
> into the facade and the stringstream converter adds no practical value. But
> the stringstream converter itself is enough to stand for the usefulness
> of library.
>
> 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.
>
> Regards,
> &rzej
>