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
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
2014-05-17 22:33 GMT+02:00 Andrzej Krzemienski <akrzemi1_at_[hidden]>:
> 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.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk