Re: [boost] [convert] Version in the Vault ready for review.

Subject: Re: [boost] [convert] Version in the Vault ready for review.
From: Vladimir Batov (batov_at_[hidden])
Date: 2009-03-26 09:24:40

> From: "Stewart, Robert" <Robert.Stewart_at_[hidden]>
>> Vladimir.Batov_at_[hidden] wrote:

Rob, thank you for your continuous help and involvement. Pouring over the 
docs and implementation takes considerable time and effort and I much 
appreciate your doing so.

> ...
>> The current interface can be summed up as with as little as
>>
>>     int i = convert<int>::from(str); // Throws on failure
>>     int i = convert<int>::from(str, -1); // Returns -1 on failure
>>
> There's room to improve the document, of course, but it is a very nice 
> start.
> The result type's documentation is rather hidden, unfortunately, as it
> appears in the "Conversion-Failure Check" section.
> It should be prominently featured in one of the early sections.

Thanks, I'll look into addressing it.

> I wonder about making the result returning interface explicit.
> That is, change the two interfaces shown above to return
> TypeOut and add a new call, something like the following,
> that returns a result instance:
>
>   convert<int>::result result(convert<int>::try_from(str));

Andrew Troschinetz mentioned that it'd be nice settling on only one 
interface and I was very happy when the interface shrunk to just one

    int i = convert<int>::from(...

Therefore, at the moment my preference would be to try hard keeping that 
only entry point and to try to extend the functionality by other means if 
needed.

> That eliminates all conversion, template deduction,
> and temporary object overhead issues for the first two calls.

I am not sure I understand as temporaries seem necessary as I'd like to be 
able to do

double d01 = convert<double>::from(str);
double d02 = convert<double>::from(str)
        >> new_locale
        >> std::setprecision(4)
        >> std::scientific;

To have the above I seem to need a temporary (in the implementation it is 
convert_detail::implementation<TypeIn, TypeOut>) to delay the actual 
conversion.

> It also makes explicit when there's a result object.

I am honestly confused as I do not believe there are separate cases when a 
convert<>::result object exists or not. The conversion specification is 
collected into a convert_detail::implementation temporary. Then, the 
temporary can equally return the "naked" conversion value (like "int") or 
convert<int>::result depending on the user preference:

int i1 = convert<int>::from(str, 0);
convert<int>::result i2 = convert<int>::from(str, 0);

I might be missing something but at present I fail to see benefits of 
additional "try_from".

> I think the result type's conversion functions should throw an exception 
> if !good().
> Then, there's no need for your dothrow:
>
>   convert<direction>::result result(convert<direction>::try_from(str, 
> up_dir));
>   direction dir(result); // throws exception if !result.good()

Unfortunately, that will make my life quite difficult... unless I wrap such 
a convert() into another interface eliminating those exceptions. :-) First, 
my company's policy is very unfavorable towards exceptions so I'd like to 
have that no-throw flexibility available for myself and anyone in a similar 
situation. Secondly, my deployment pattern is such that I read dozens of 
config. parameters and try converting them. If (more like "when") a 
conversion fails, I log a message for that failed parameter, use the default 
and move on. Like

convert<type1>::result p1 = convert<type1>::from(str1, def1);
...
convert<typeN>::result pN = convert<typeN>::from(strN, defN);

if (!p1.good()) message("bad str1");
...
if (!pN.good()) message("bad strN");
... proceed with whatever parameters I've got.

The above is admittedly uninspiring but I found people to be comfortable 
with it. Throwing exceptions in seem to complicate the issue considerably.

On the other hand I see your point as *not* throwing in

    direction dir(result);

allows the conversion failure to slip through unnoticed. I am not sure how 
serious it is. I am kind of leaning towards these two

int i1 = convert<int>::from(str, 0);
convert<int>::result i2 = convert<int>::from(str, 0);

behaving the *same* with the second line only adding the *ability* to check 
the failure condition (it admittedly fits my usage pattern as mentioned 
above). After all, I provided the default for a reason. If I wanted the 
throwing behavior, I would not bother providing the default and, therefore, 
would use the throwing:

int i1 = convert<int>::from(str);

> I dislike the nothrow idea since it implicitly provides a default of 
> TypeOut()
> and there's already an interface for specifying a default.
> I'd rather folks wrote this:
>
>   int i(convert<int>::from("not an int", 0));
>
> than this:
>
>   int i(convert<int>::from("not an int") >> boost::nothrow);

Yes, I think it is a very sensible observation. I added that nothrow only 
for symmetry sake but with two lines above side-by-side the latter seems 
obfuscated and plain ugly. I've removed it from the doc. and will think how 
to take advantage of no nothrow in the implementation.

> Your locale example fails to break a line:

Thanks.

> I have some issues with the implementation, but I'll defer to another 
> time.

I'd most likely be very interested in any implementation issues you might 
see. Although, if that is not an actual bug, then I could probably wait a 
little longer. :-)

Thanks again,
V.