Re: [boost] Formal Review Request: Boost.Convert
Hi, I would like to propose removing the default value for the second argument of convert. I believe it may be insecure. Let me explain it by exemple. What did I mean when I wrote the following code: string str = readInput(); int men = convert_to<string, int>(str); 1. I meant: "if conversion is impossible, return 0", because I read the documentation thoroughly. 2. I am a novice and didn't really think about the situation where someone types a non-int string. In case of the first option it is all fine, but if the latter is the case, it may have been better to throw an exception to the user. I would propose, rather than having a default "fallback" value to provide two overloads for convert: one with two parameters where you can provide the fallback value, and the other, one-parameter, that would indicate that you want to throw an exception on failure. Yo do not loose much of your original interface, because you can write more explicitly: int men = convert_to<string, int>(str, 0); I find it better because: 1. It forces the writer to type more clearly his intentions. (Or explicitly confirm that he is aware that the string may not be convertible to int.) 2. Releaves you from providing the throw_t argument altogether. This in turn removes the possibility of writing: convert<string, int>(str, -1) >> throw_t(); // What does -1 mean here? Regards, &rzej
int men = convert_to<string, int>(str);
Just to clarify. You probably meant one of the below: int men = convert_to<int>(str); int men = convert_from<string, int>(str); int men = convert<string, int>(str);
would propose, rather than having a default "fallback" value to provide two overloads for convert: one with two parameters where you can provide the fallback value, and the other, one-parameter, that would indicate that you want to throw an exception on failure. Yo do not loose much of your original interface, because you can write more explicitly:
int men = convert_to<string, int>(str, 0);
Yes, I tend ot agree. Requiring an explicit default seems clearer and less ambiguous. Once you mentioned that int men = convert_to<int>(str); looks very unsafe to me. It's highly unlikely 0 (or any default-constructed object) would be a good failure indicatior... especially snicked in "under the table".
I find it better because: 1. It forces the writer to type more clearly his intentions. (Or explicitly confirm that he is aware that the string may not be convertible to int.) 2. Releaves you from providing the throw_t argument altogether. This in turn removes the possibility of writing:
convert<string, int>(str, -1) >> throw_t(); // What does -1 mean here?
That "-1" indeed looks kind of out of place, doesn't it? :-) And 'yes', your arguments make a lot of sense to me. So, to summarize we are moving to the original lexical_cast behavior int i = convert_to<int>(str) // throws on failure int i = convert_from<string, int>(str, -1) // returns -1 on failure int i = convert_from(str, -1) // the same as above and throw_t scrapped. Any objections? V.
convert<string, int>(str, -1) >> throw_t(); // What does -1 mean here?
That "-1" indeed looks kind of out of place, doesn't it? :-)
Unfortunately, there are classes that do not have default constructors (I have a few). For them we need direction dir = convert<srting, direction>(str, direction_up); Those classes *might* like to have throwing behavior. Therefore, for those we need direction dir = convert<srting, direction>(str, direction_up) >> throw_t();
int i = convert_to<int>(str) // throws on failure int i = convert_from<string, int>(str, -1) // returns -1 on failure int i = convert_from(str, -1) // the same as above
Done as above.
and throw_t scrapped. Any objections?
I seem to have to bring it back for the case I described above. V.
participants (2)
-
Andrzej Krzemienski -
Vladimir Batov