| -----Original Message----- | From: [EMAIL PROTECTED] | [mailto:[EMAIL PROTECTED] Behalf Of Jens Maurer | Sent: Wednesday, July 09, 2003 5:07 PM | To: [EMAIL PROTECTED]; [EMAIL PROTECTED] | Subject: [boost] Formal Review: fixed-point decimal library
Overall I vote to accept this in the Boost library. It appears to meet the needs of both COBOL programmers and Bean Counters :-) More detailed and more expert comments have been received, but from a brief perusal of the documentation and a build of the trydec test program using MSVC 7.1, I have a few minor observations and comments: 1 There are some warnings using 'strict' which are probably cast avoidable: fixed_decimal.hpp(442) : warning C4389: '==' : signed/unsigned mismatch fixed_decimal.hpp(446) : warning C4018: '<' : signed/unsigned mismatch fixed_decimal.hpp(450) : warning C4018: '>' : signed/unsigned mismatch 2 Using namespace std; is needed for more than cout. Also namespace fixdec would be more helpfully replaced by "using numeric::decimal;" This makes it obvious what is being demonstrated. 3 The sample program output could helpfully be added and made more obvious, for example by changing to output: numeric_limits<decimal>::is_specialized is true numeric_limits<decimal>::radix is 10 numeric_limits<decimal>::digits 9 numeric_limits<decimal>::digits10 9 numeric_limits<decimal>::epsilon() 0.000000001 and cout << setprecision(2) << showbase << dp // 2052.00 << " " << showmoney << dp //$2,052.00 << " " << showintl << dp << "\n" // USD2,052.00 << noshowmoney << noshowintl << dn // -1,026.00 << " " << showmoney << dn << " " // -$1,026.00 << showintl << dn << "\n"; // USD-1,026.00 And adding the complete output as a comment is very helpful. /* Output: ... numeric_limits<decimal>::is_specialized is true ... */ 4 I agree that decimal implies fixed so fixdec is a not very nice name - spare a thought for those whose native language is not english. I believe the best namespace is numeric. I don't think this will deter even COBOL programmers! 5 Overall a much better demo program would be valuable to sell this to average users. For example, all the examples in the html doc might be included in trydec.cpp. The default C locale doesn't produce interesting results, so adding cout.imbue(locale("English_United States.1252")); (macroized for other OS?) would be more 'interesting' if dollar-centric. 6 A test program is needed (promised but ...). Comments make tests a very valuable learning tool. 7 The documentation is mainly pitched at a too high a level for users and mixes detailed rationale with user info. The 'Boost Standard' format might be more suitable? 8 The synopsis might be commented better, for example: decimal(int, int); // scale, value 9 I really don't like the name scale at all. Nor deffrac, decscale and iosprec. Can't we do better than this, even if longer? These are going to be very visible in user code, but not at all obvious meaning. 10 You could usefully add an example of explicit limits for the common 32 bit system max amount is $21,474,836.47 - OK if you are invoicing from a hardware store, but for 64 bit ?92,233,720,368,547,758.07 may suit Enron-type accountants better. Can you give some clues to the probable performance cost of using 64 bits on X86 systems? Hope this will help you get wide use of this useful code. Paul _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost