Re: [boost] Formal Review Request: Boost.Convert

Subject: Re: [boost] Formal Review Request: Boost.Convert
From: Vladimir Batov (batov_at_[hidden])
Date: 2009-02-24 04:59:02

>> transform (
>>   s.begin(),
>>   s.end(),
>>   back_inserter(i),
>>   convert<int, string>(string(), -1)
>>       >> throw_t() >> std::hex);
>
> I believe that says "convert to int from string, throw on error,  format 
> as hex." But what does string() and -1 signify in that case?
>
> And if you'll allow me to nitpick, it seems you've mixed up the order  of 
> the function parameters here. Should it read as follows?
>
> convert<int, string>(-1, string())

Yes, I typed it in a hurry and messed it up. Sorry. It meant to be

 transform (
   s.begin(),
   s.end(),
   back_inserter(i),
   convert<string, int>(string(), -1)
       >> throw_t() >> std::hex);

I.e. we have strings. We want to convert them to ints. So, we say, 
convert<string, int>.

> And since it seems that -1 and string() serve no purpose here, can we 
> possible get a refinement to the following?

In fact, string() and -1 do serve a purpose. Namely, -1 is the 
default/failure parameter to be used if conversion fails. string() is a 
nuisance, I agree, but it has to be in front of -1 as the signature is 
convert(TypeIn const&, TypeOut const&). But you are correct, if we do not 
need the default value (as with convert_to()) we should be able to do as you 
indicated below. Will add this.

> transform (
>   s.begin(),
>   s.end(),
>   back_inserter(i),
>   convert<int, string>() >> throw_t() >> std::hex);
>
> I'm sure you meant to say here:
>
> unsigned i = convert_to<unsigned>(str);
>
> Correct? That makes sense.

I did mean to say that. Thank you. convert_to always requires the type we 
are converting to.

> From looking at the source code, it seems like the first one is just  a 
> typo.

Yes, it was. I hate when that happens. Trying to explain and instead confuse 
people. Frustrating.

> But you see how easily things can get mixed up. I'd be happier  picking 
> one interface, either convert or convert_to or convert_from to  keep 
> things clear. Though that might make others unhappy, I don't know.

I understand. I am of an opinion that we should not be deciding for people 
what they are to use... especially for such a diverse group as Boosters. 
We'll provide them with convert, convert_to, convert_from which I hope will 
cover all the preferences.

> ... when I see >> and std::hex on one line, my  mind immediately goes to 
> streams because std::hex is a stream  manipulator. And in this case, the 
> stream that's being manipulated  isn't exposed to the user, it's an 
> implementation detail. Using stream  manipulators here may be nice, but 
> it's a bit of a case of  implementation leaking into the interface.

Hmm, I am not sure I see it that way. To me those manipulators are merely 
building blocks/components. If they fit some other purpose, I'll use them 
without hesitation. However, I tend to agree that it somewhat gives away the 
implementation detail. Although come to think of it I am not sure it's such 
a bad thing as I'd expect that to make the user feel more comfortable as 
he'll know what's ticking inside.

> I'm not saying that's  entirely bad in this case because using stream 
> manipulators does seem  to solve the problem nicely, but it is a bit weird 
> (at least to me  anyway) seeing them used this way.

Yes, I think I know what you mean.. and, gosh, haven't I felt that way while 
looking at many Boost libraries. I am sure this little piece won't even get 
into the first ten in this category. What do you think? ;-)

> I could get used to this interface though, nice job so far. Thank you  for 
> the effort.

Actually it was very little of my own effort. I merely followed the path of 
the least resistance and implemented what people were suggesting/discussing. 
What's come out so far has nothing to do what I foolishly stepped with. :-) 
But still thanks for the kind words anyway. :-)

Best,
V.