-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/13/2010 04:53 AM, Adam Merz wrote:
The changes have just been uploaded to the Sandbox (haven't updated the Vault yet). Please take a look and let me know if you see any problems with the implementation. There shouldn't be.
Looks good to me -- the compilation errors were solved, which was my only real concern. A few nitpicks though, since you asked: ;-]
Functions templated on charT taking std::basic_string<charT> should be templated on charT, traitsT, allocT and take std::basic_string<charT, traitsT, allocT>.
Okay, done.
xint::detail::nan_text should return 'std::basic_string<charT> const&' rather than just 'std::basic_string<charT>'. This way callers (many integer_t constructors as well as xint::to_string variants and stream operators) can call it like 'std::basic_string<charT> const& tnan = detail::nan_text<charT>();' and avoid making hundreds/thousands of pointless copies of the string.
That makes sense. Done.
For functions taking 'charT const*, charT**', this is a bit C-ish; obviously this is pedantic/stylistic concern, but charT*& is preferred over charT**.
It's a bit C-ish because it was based on a C function. :-) There's no similar function that would confuse the compiler (or the user), so I've changed it, but I'm ambivalent about it... having to add the address-of operator was a clue that it was an out-parameter. Without it, you can't tell that it is unless you look at the source code or documentation. A minor quibble though.
The functions templated on charT are fine, but for the functions specifically overloaded on std::string/std::wstring (e.g. to_string, to_wstring), consider also providing additional overloads for char16_t and char32_t based on BOOST_NO_CHAR16_T and BOOST_NO_CHAR32_T.
to_string and to_wstring have identical implementations except for the character type they hardcode in. Refactor these into a function template so there's a single common implementation, and have outer functions defer to the template.
I've made a new template function, to_stringtype, that takes a type to make the string from. to_string and to_wstring are now just thin shells around it. I'm not sure how often the other two types will be used (and I doubt anyone is at this point). I'm also not sure what I'd call those functions... to_char16_string seems esthetically unpleasant. It's the kind of thing that, if I ran across it in an otherwise well-designed library that I wanted to use, I'd write a function or macro to rename it and hide its ugliness. So I propose leaving it as it is. If someone needs that capability, to_stringtype will provide it, and they can write whatever wrapper function or macro they want around it. Combining messages...
integer_t constructors taking 'const charT *, charT **' don't compile, due to const correctness issues (assigning a charT const* to a charT*). convert.hpp line 140 works around it with const_cast, but the signature should just be changed to 'const charT *, const charT **' (or better yet 'const charT *, const charT *&', as previously mentioned).
I wondered about that, but I guess I got distracted before I could find a satisfying fix for it. In any case, it's fixed now.
raw_integer line 131 & convert.hpp line 103, from_string takes char** rather than charT**.
Oops... sorry, I was in a bit of a hurry, and there was no test to point that out. Both of those problems are now fixed.
std::size_t should be used universally instead of size_t.
As a defensive measure, it makes sense, though I'd question the sanity of any developer who made his own different size_t type. Done. These changes have been uploaded to the Sandbox now. If you have the time to spare, please check them and ensure that I haven't made a mistake. And thanks for the feedback, I really appreciate it. - -- Chad Nelson Oak Circle Software, Inc. * * * -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwVDRMACgkQp9x9jeZ9/wSfdwCgqDsSmDfXiL/YkeWRkQP6ve4J sJ4An3BMnuwx0UFCD7nvJfVOup2EpCFA =XxGb -----END PGP SIGNATURE-----