[Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

John LeMoyne Castle jlc at mail2lee.com
Tue Nov 30 01:35:39 PST 2010


OOo Basic has had issues with its fixed point Currency type for a long time.  
I think the Basic Currency type deficiencies create a bar to adoption in the
minds of business users.  
Nothing says 'run away' like mangling the money numbers. 

Currency is presently implemented with a 64bit struct of 2 Int32s and the
math is done by the tools/bigint class. 
The attached patches replace the code to implement the Currency type with an
ISO C99 compiler supported 64 bit integer. 
In fact, the 64 bit int was already a member of the Sbx core data union.  
Main patch makes 1/2 kLOC reduction in sbx.

http://nabble.documentfoundation.org/file/n1991648/0001-Basic-Currency-64bit-impl-fixes-i31001-to--libs-core.patch.tar.gz
0001-Basic-Currency-64bit-impl-fixes-i31001-to--libs-core.patch.tar.gz 

http://nabble.documentfoundation.org/file/n1991648/0001-Basic-Currency-type-native-64bit-implemen-components.patch
0001-Basic-Currency-type-native-64bit-implemen-components.patch 

http://nabble.documentfoundation.org/file/n1991648/CurTestODS.ods.tar.gz
CurTestODS.ods.tar.gz 

The attached CurTestODS.ods spreadsheet Basic program CurTestRunAll
demonstrates the errors from the old implementation: 
  --  flagrantly wrong values from string inputs
  --  hugely wrong values from MDAS calculation
(where flagrantly, hugely == 10+ orders of magnitude off) 
  --  no-op (returning one of the operands instead of doing the operation)
  --  incorrect overflow from operations on bad string input values
  --  different errors and error rates on different platforms (in LibO Beta
3 on Ubuntu10.04 and WinVista)

The errors appear to start when results reach ~2.15e5 [(2^31-1)/10,000].
They continue up through the Currency upper limit of ~1e15. 
The errors affect all of the basic MDAS (*/+-) math operations mostly(?) add
and subtract.
I didn't troubleshoot the errors thoroughly because the offending code was
either: 
ripped out and replaced with simple C++ operations on the compiler supported
64 bit int, or 
removed by rework of the Compute, Get and Put routines in sbx/*.

The test program is CurTestODS.ods :: Standard :: CurTestRunAll
CurTestRunAll tests number pairs from the CurTestMeta sheet by: 
  -2x-  reading the numbers in as strings or doubles 
  -4x-  performing all MDAS (*/+-) operations 
 -16x-  for all Basic operand type combinations of integer(16b), long (32b),
double (64b) and currency 
for 128 tests on each number pair each test producing a Currency type
result.  
Expected values are calculated by a parallel calculation in doubles to
produce an expected Currency type result.  
Anticipated Basic run-time errors (overflow and div by zero) are identified
and passed. 
Unexpected BRT errors are counted and output as errors.

All of the PostFix result sheets show a great reduction in the number of
errors over Beta3 when the new implementation is used.  

The 4 remaining "errors" in the Demo result set show both the strength and
the weakness of the fixed point Currency type relative to the double
precision floating point type.  These type differences pre-exist by
definition and will not go away in any implementation.  Where more
fractional digits are needed the Double is more accurate, but when more
significant digits are needed the Currency type is more accurate.

What else it does: 
The fix needed to include corrections to the
currency-get-value-from-string-literal because it was a source of bad
values.  Users have requested in OOo issues to get the ability to use string
literals in their Locale format as set in the options.  The new string
getter relaxes the requirement that string literal be in en-US format (i.e.
"1,000,000.00") - it will now accept "1.000.000,00" if the Locale allows it
while always accepting the Basic standard en-US format.  The getter will
also always accept (always ignore) spaces within a number string so "1 000
000.00 00" is accepted.  The Locale read was lifted from another of the
Currency string input functions.  If this needs rework or reversion let me
know.

I thought this would be easy - and it was easy and fun - it was just a real
Big Easy... 

All contributions licensed per TDF standard:  MPL 1.1 / GPLv3+ / LGPLv3+
 
--LeMoyne (JLCastle)

-- 
View this message in context: http://nabble.documentfoundation.org/PATCH-Basic-Currency-Issues-i31001-to-i107277-tp1991648p1991648.html
Sent from the Dev mailing list archive at Nabble.com.


More information about the LibreOffice mailing list