-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 05/04/2010 02:23 PM, Stewart, Robert wrote:
The integer member function template assignment operator could benefit from the advice in <http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/>. There may well be other member functions that should be changed as well.
I read it when you first mentioned it (though not in detail), but I'm deferring everything related to that until I determine whether copy-on-write is going to be used in the final version of the library. If it is, then passing by value is cheap in all cases, and is probably best; if not, then it might be better to leave most functions using const references instead, because I'm not convinced that the compiler will have a chance to optimize the copies away in most cases.
If the compiler elides the argument copying, then the COW logic won't be invoked. It's a win with or without COW.
That could well be, but I'll need to prove it to myself before I use that, and I haven't had the time to do so (and probably won't in the next few days at least, there's a lot of restructuring to do on it). [...]
The "not recommended, inefficient" comment for postincrement and postdecrement sounds too harsh. [...]
Not as harsh as not implementing them at all, which is what n1692 recommended. I prefer to implement all of the operators, and leave the decision to the person using the library, with a warning so that he knows to think before using them.
I think part of what you elided was a suggestion that information regarding the inefficiency be put where you can give it fair treatment, not in a terse, somewhat misleading docstring. [...]
Perhaps I misunderstood your recommendation. But I didn't have any more to say on the subject (I assume that anyone who doesn't know why postincrement/postdecrement would be less efficient than their pre- cousins and saw the warning would look it up), and the docstring is the only thing people will see unless they specifically look at the documentation for that function, so I believe it's better to leave it there.
sqr() means, I suppose, square, since you have sqrt(), too. Why not spell it out and remove doubt?
I had a lot of functions, including both of those, spelled out in earlier iterations of the library. I changed them to match the conventions of similar functions in the C and C++ libraries, on the assumption that people would find them easier to remember that way.
What does sqr() do?
sqr's a number, of course. ;-) Anyone familiar with sqrt(double) in the standard library, or sqrt in XInt, should find it to be intuitive naming. I don't expect that it will often need to be used directly anyway.
If you need to parse out the characters that are actually part of a number, like strtol, the stream functions will do that. At least for base 8, 10, and 16 numbers, which I expect will be the main ones that people will use anyway.
I'd prefer to have both forms. That is, if I call the overload which takes a non-const reference to an iterator or size_t, I can determine where the parsing left off and not get an exception. If I assume everything should parse, I'd use the current overload.
Noted. Easy enough to do, I'll just move the code from operator>> and expand on it.
An overload for char const * would be appropriate so as to avoid forcing the creation of a std::string when not already available.
I was under the impression that a char* would be efficiently converted to a const std::string& when needed. If it's less efficient than I was led to believe, then I'll certainly add an overload.
std::string construction from a char * implies free store allocation and copying.
So there isn't any compiler magic that automatically re-uses the existing allocation and just acts as a wrapper around it, for cases like that? How disappointing. Easy enough to add that overload then.
It looks as if you haven't taken advantage of building some of the relational operators on the others. For example, >= is !<. That is, many of the operators should be inlined and only a few need to be implemented.
I'm not sure I see your point. They all consist of a single function call and an int comparison, so the compiler will probably inline them regardless.
I didn't see the implementation of those, so I assumed that they were declarations of non-inlined functions.
I believe they are, but I also believe that the compiler is smart enough to change them to inlined functions. It wouldn't be any major difficulty to make that explicit though.
Nevertheless, this is another case of only having to worry about the implementation of a core set of functions and leave the others in terms of those.
bool operator==(const integer &num1, const integer &num2) { return compare(num1, num2)==0; } bool operator!=(const integer& num1, const integer& num2) { return compare(num1, num2)!=0; } bool operator<(const integer& num1, const integer& num2) { return compare(num1, num2)<0; } bool operator>(const integer& num1, const integer& num2) { return compare(num1, num2)>0; } bool operator<=(const integer& num1, const integer& num2) { return compare(num1, num2)<=0; } bool operator>=(const integer& num1, const integer& num2) { return compare(num1, num2)>=0; } I think you can see the appeal of doing it that way. :-) I might be missing a trick there, but I don't think so... compare returns as soon as it can tell what the answer is; it only has to pursue it to the bitter end when the arguments are equal down to the lowest digit_t. I don't think dedicated functions for each operator could be made any more efficient.
s/template function/function template/
Where do you see them? grep doesn't find the string "template function" anywhere in the source, headers, or documentation.
Search case insensitively. I think it was "Template function" actually.
Ah, found them.
And out of curiosity, do they have different meanings that I'm not aware of, or is this just the commonly-used phrasing?
The correct terminology is "function template" and "class template." IOW, they are templates for generating functions or classes, they are not functions or classes.
Good enough. Though that's a distinction that I'll never be able to remember, so forgive any conversational lapses.
Commentary, like "efficiently" in the to() docstring, should be left for broader library documentation.
Why? It's far more efficient than the method that would almost certainly otherwise be used, which is to stream it out and stream it back to the new type, and I wanted to make that clear. I'm also not sure what broader library documentation you're talking about.
First, I'm assuming that Doxygen generated documentation will be for reference and not for tutorial, rationale, etc. content. That is the "broader library documentation" to which I refer.
That seems like an arbitrary distinction.
Second, when describing what a function does, the word "efficiently" isn't helpful. If you prefer, you can discuss the efficiency aspects in the rest of the docstring (not part of the brief line).
As with the postincrement example, I didn't have anything further to say on the subject -- certainly nothing requiring even a full sentence -- and "efficiently" serves as good shorthand for "more efficiently than other commonly-used methods". It also tells the casual browser the purpose for its existence, so he's more likely to use it for such conversions than automatically fall back on something like lexical_cast.
exceptions::too_big sounds like std::range_error to me.
The wording on std::range_error, in Josuttis' "The C++ Standard Library", is [...]
The name fits nicely, even if the description suggests something slightly different. Arguably, your internal computation determined that the value exceeded the range of the target type, so the exception type is fitting.
In the end, there's no harm in having your own type, given that it does derive from range_error. It allows calling code to be more specific if warranted.
That, and the self-imposed constraint that the library only throw exceptions that are declared in the boost::xint::exception namespace. (Why? Because it seems discordant to me to have a library that only *almost* always throws exceptions from a particular namespace, and it was almost no additional work to make it fit my sense of rightness. In other words, because I can. ;-) ) - -- 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/ iEYEARECAAYFAkvguTIACgkQp9x9jeZ9/wRPgQCgkKZE/XGpZ+E9HO0X8I83l/Tm 3VkAn1Rzh5Nup7l/k7ORSXWDF17T5Nug =22Ix -----END PGP SIGNATURE-----