[boost] Re: Thoughts on fixdec

From: Bill Seymour (bill-at-the-office_at_[hidden])
Date: 2003-07-16 07:55:13

Daryle Walker wrote:
>
> I think this library should be accepted.
>

Thanks.

>
> 1.	We have two math-related namespaces already.  The boost::math 
> namespace leans to theoretical work and boost::numeric leans to 
> hard-core computation.
>

I don't know that there's anything particularly theoretical about
decimal representations; and the library's target audience is not
those users who do "hard-code computation," which I understand to
mean numerical programming (in the sense of scientific and
statical programming).

It's not a major issue for me; and I'll do whatever the majority
wants.

>
> You should probably pick one of those namespaces (numeric?) and
> possibly rename the class to "fixed_decimal". 
>

I'm reticent to require the user to type the extra "fixed_" in
every declaration of such an object.

>
> 2.	There is a <boost/limits.hpp> header to give deficient systems
> the std::numeric_limits class template (and pass through if <limits>
> exists).
>

I had that in a previous version but wasn't satisfied with it.  I can't
remember why; so I'll look at the old code and see what it was that
I didn't like.

>
> 3.	Instead of playing around with determining the significand type,
> just #include <boost/cstdint.hpp>,  ...
>

I do.

>
> ... use boost::intmax_t and be done with it.
>

You're right.  My mind got stuck on int_least64_t; and I completely
forgot about intmax_t.  Also, that might solve the <boost/limits.hpp>
problem (whatever that was) if numeric_limits<intmax_t>::is_specialized
and numeric_limits<intmax_t>::digits10 is a constant expression.  I'll
check that out.

>
> 4.	What, no (safe-)Boolean conversion?
>

Ack!  You're right!

>
> 5.	The conversion from "int" needs to check for overflow.
> The conversion from "double" needs to check also for underflow.
> The conversion from strings needs to check also for bad formats.
>

But that would require some users to pay for something they don't
need.  Users who need that level of correctness can write their
own checks; and others (probably most of the library's target
audience) get better efficiency.

>
> 6.	There should only be conversions from strings, and _no_
> mixed operations with strings.
>

In the absence of decimal literals, I think it's easier for the
user to be able to use strings as pseudo-literals in all contexts;
and I don't see how the mixed operations do any harm.  In particular,
I'm willing to give the user credit for knowing that

        my_decimal += "The quick brown fox...";

doesn't make any sense.

>
> 7.	Does the scale need to be first in all constructors?  If it
> can be placed second, then converting constructors can be made
> and most of the mixed operator functions can go away.
>
> [ctor examples]
>

I'd want to see use cases for all those conversions before I agreed
to add them.  Remember, the library's target audience is folk who
are writing financial software.  It's not intended as a general-
purpose number-crunching library.

>
> 8.	The "round_down" and "round_up" functions don't seem clear enough.  
>

They're intended to be clear to accountants who deal only with
non-negative debits and non-negative credits; and whether a value
is a debit or a credit is orthogonal to the mathematical notion
of sign.

Remember the instructions on your tax return form that tell you,
if line x is less than line y, subtract line x from line y and
write the answer over here; otherwise subtract line y from line x
and write the answer over there?  Sure, that's a howler for
mathematicians; but it makes perfect sense to accountants. 8-)

>
> 9.	What, no "operator <<" nor "operator >>"?  (The shift would
> be a power of ten.)
>

Those operators don't exist for the floating-point types, and with
good reason IMO.  Their use is best reserved for twiddling bits,
not for scaling.  Even for binary integers, i << 1 is not generally
the same thing as i * 2 except on 2's-complement boxes.

>
> 10.	The class is no longer a template, but some of the wording
> in the code and docs act like it's still a template.  (When you're
> rewording, there's some HTML errors to fix.)
>

Could you point them out?

>
> 11.  ...
>
> ...
>
> class MyNum
> {
> public:
>      //...
>      bool is_negative() const;
>      //...
> };
>
> ... would save time over doing a (possible) conversion from zero
> and a comparison.
>

But (my_decimal < 0) doesn't do a conversion; and it lets the user
use decimals the way they would use other arithmetic types.

>
> 12.	[use pword to save rounding mode I/O state]
>

Yes, much better.  I'll play with that.

>
> 13.	You have strange (regular) assignment semantics.  You try to
> change the receiver's logical value to the source's logical value,
> but retain the receiver's original scale (internally shifting the
> new significand and doing any rounding if needed).  ... you will
> get strange effects if you try to change containers of "decimal"
> through assignment.
>

I thought it would be less surprising if a decimal object's scale
were immutable.  This matches prior art (the BCD type on IBM
mainframes, for example) in which the scale is part of the type.

You're right about container<decimal>; but remember that this class
isn't intended for number-crunching; so I don't really care about
assigning matrices, etc.

>
> 14.	If you have a "scale" member function to return the current
> object's scale; you should have something like a "significand" member
> function to return the object's mantissa.  This will allow copies of
> objects that retain just one aspect of their source.
>
>
> 15.	It took me a while to figure the following out, so maybe you
> should put it in simpler language.  Objects of this type retain a
> copy of the given value, rounded (via the given mode) and kept to
> a precision of the given scale number of (base-ten) fractional digits.
> The value is stored as that scale and the (integral) number of
> 10**(-scale) units needed to add up to the value.
>

I'm not sure what you mean.

>
> I guess that the scale needs to be non-negative?
>

Yes, and that's pointed out in the documentation, but maybe not
loudly enough.

>
> 16. ... the "[]" is bound to the object on the left (by C++'s parsing
> rules), and not to the operator.  You just adjust the spacing and wish
> that the brackets were associated with the operator on the right.
>

Yes, and that's pointed out in the documentation with a box around it;
and there's a rather long bit (maybe even too long) that explains how
the whole business came into being.

>
> 17.	Do you really need to support VC++5 ...?
>

Yes, I do, because that's what I'm stuck with at work (for what are
euphemistically called "non-technical reasons").  I understand
completely that such support is not required for Boost; but absence
of a requirement isn't a requirement of absence; and I don't see
that the additional support does any harm.  (Users have to hack
their visualc.hpp to let that version pass anyway.)

Thanks for the input.

--Bill Seymour