Re: [boost] [xint] Third release is ready, requesting preliminary review

Subject: Re: [boost] [xint] Third release is ready, requesting preliminary review
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2010-05-05 07:56:08

Chad Nelson wrote:
> On 05/04/2010 02:23 PM, Stewart, Robert wrote:
>
> >>> The "not recommended, inefficient" comment for postincrement and
> >>> postdecrement sounds too harsh. [...]
[snip]
> 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.

I expected that you'd discuss the actual overhead of the post- operators so users could make an informed decision.  Yes, we all know that the post- operators are less efficient, but in your case, because of the memory allocation and large amount of data that may be copied, it can be worse than one might assume, because of the work your class must do.

> >>> sqr() means, I suppose, square, since you have sqrt(), too.  Why
> >>> not spell it out and remove doubt?
[snip]
> > 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.

I still don't understand.  Is "sqr" an alternative spelling of "sqrt" or does it mean "square?"  I know of sqrt(), but I've never seen sqr() before, so your assertion is false.  I found no manpage for it and a cursory search of the Internet turned up nothing on it.

> > 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.

A string class other than std::string could be designed to allocate memory or just point to external memory, according to how it is constructed, but that's not how std::string works.

> > 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.

Those are trivial, and all based upon a single function, so you do get the reuse to which I referred.  However, there is an inherent inefficiency here, though it may be miniscule relative to the comparison code.  A compare() function must return one of three values: less than zero, zero, or greater than zero.  That implies additional logic that isn't necessary when computing <, ==, etc.  (Splitting those apart means duplicating portions of the logic in compare(), of course.)

BTW, should you implement compare() using memcmp()?  I would expect that function to be optimal as it is usually implemented in assembler and takes great advantage of platform characteristics.

> > 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.

I contend that "efficiently" is meaningless then.  It's a QoI detail, your textual claim has no effect on the code, and it's not quantifiable.  It certainly doesn't tell me that one operator is to be preferred over another because the one is described as "efficient" and the other isn't.

> >>> exceptions::too_big sounds like std::range_error to me.
[snip]
> > 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. ;-) )

That's a nice rationale.  Be sure to put it in your docs (assuming it isn't).

_____
Rob Stewart                           robert.stewart_at_[hidden]
Software Engineer, Core Software      using std::disclaimer;
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.