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 _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost