Vicente BOTET wrote:
De : "Gordon Woodhull"
The interface of the proposed Convert library is not simple enough. To start out with the most annoying thing, I don't like "::from" at all. The replacement should look as close to lexical_cast as possible.
So, why the extra '::from'? Even though I disagree with the lazy evaluation (see below), even that doesn't seem to require this weird static member syntax. Is '::from' just so that '::result' will not feel lonely?
For the rest of this review, I'm going to refer to convert::from(s), but please keep in mind that I really think it should be something like string_convert(s).
I wanted to add more on the this direction. Vladimir, note that maybe the syntax proposed below could be not correct in C++, but I hope you will get the idea.
What do you think of using
* Instead of
int i1 = convert::from(str); // Throws if the conversion fails
That should be convert<int>::from(str).
the following
int i1 = string_convert(str); // Throws if the conversion fails
You haven't specified the target type. You'd need: int I = string_convert<int>(str); which doesn't read well.
* Instead of
int i2 = convert::from(str, -1); // Returns -1 if the conversion fails
the following
int i2 = string_convert(defaults_to(str, -1)); // Returns -1 if the conversion fails
defaultts_to(T,U) is a function that generates a wrapper to T and default value U.
You have the same problems here, of course. Adding the wrapper really looks bad. I'd prefer something more like this: int I = convert<int>::from(str, default_(-1)); if you wish to make it explicit.
* Instead of
int i = convert::from(hex_str, -1) >> std::hex;
the following
int i = string_convert(defaults_to(as_istream(hex_str) >> std::hex, -1);
as_istream is a function that convert a type to an input stream (provided the type is output streamable).
I see that you're trying to make "as_istream(hex_str) >> std::hex" look like a normal stream that plugs into your string_convert function, but it looks very ugly to me.
* Instead of
string d1 = string_convert::from(d)(locale_ = rus_locale)
std::setprecision(4) std::scientific;
the following
string d1 = string_convert(as_istream(d) >> set_locale(rus_locale) >> std::setprecision(4)
std::scientific;
here set_locale can be a manipulator.
With that locale, you could also have: string s = convert<string>::from(d)
set_local(rus_locale) >> std::setprecision(4) std::scientific;
I understand that there are things hiding behind a façade: - convert<string>::from(d) returns a convert<string>::result - convert<string>::from(d) acts like a std::istream WRT manipulators - convert<string>::from() returns a convert<string>::converter I also get that convert<string>::result converts implicitly to a string but can also be queried via a safe-bool and directly for the string, but adding lots of explicit wrappers into the mix just obfuscates the desired conversion operation. It makes things more obvious, but syntactically obese. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.