www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - d-money ctor(string) has bug, be careful! EUR("1.23456") ==> ctor ==>

reply mw <mingwu gmail.com> writes:
Hi,

(if you are not a user of this package: 
https://code.dlang.org/packages/money

you can stop reading now.

I'm a D newbie, not sure if this the best place to send this 
alert, but this library package is about money and has a relative 
large user base:

Download Stats:

6 downloads today
122 downloads this week
966 downloads this month
9300 downloads total

I think it worth attention from a big audience group).


The issue found is here:

https://github.com/qznc/d-money/issues/11

----------------------------------------------------------------------------------------
$ cat m.d
/+dub.sdl:
dependency "money" version="~>2.3.1"
+/

import std.stdio;
import money;

alias EUR = currency!("EUR");

void main() {
   EUR x = EUR( 1.23456 );
   EUR y = EUR("1.23456");
   writeln(x);
   writeln(y);
   assert(x == y);
}

$ dub m.d
1.2346EUR
3.3456EUR
core.exception.AssertError m.d(15): Assertion failure
----------------
??:? _d_assertp [0x52b005]
m.d:15 _Dmain [0x4dec36]
Program exited with code 1

The reason is pow10(x) doesn't handle negative x at all.

https://github.com/qznc/d-money/blob/master/source/money.d#L64

when called from

https://github.com/qznc/d-money/blob/master/source/money.d#L107

with x = -1

BTW, why use a recursive function instead of a simple loop to 
calc pow10?
May 21
parent reply mw <mingwu gmail.com> writes:
On Friday, 22 May 2020 at 05:26:45 UTC, mw wrote:
 The issue found is here:

 https://github.com/qznc/d-money/issues/11
For anyone who's using this package, just submitted the PR to fix it: https://github.com/qznc/d-money/pull/12
May 24
parent reply Basile B. <b2.temp gmx.com> writes:
On Sunday, 24 May 2020 at 17:06:43 UTC, mw wrote:
 On Friday, 22 May 2020 at 05:26:45 UTC, mw wrote:
 The issue found is here:

 https://github.com/qznc/d-money/issues/11
For anyone who's using this package, just submitted the PR to fix it: https://github.com/qznc/d-money/pull/12
there's also parsing that is not super compliant EUR y = EUR("1.000.000,00"); writeln(y); // F!
May 24
parent reply mw <mingwu gmail.com> writes:
On Sunday, 24 May 2020 at 18:15:09 UTC, Basile B. wrote:
 On Sunday, 24 May 2020 at 17:06:43 UTC, mw wrote:
 On Friday, 22 May 2020 at 05:26:45 UTC, mw wrote:
 The issue found is here:

 https://github.com/qznc/d-money/issues/11
For anyone who's using this package, just submitted the PR to fix it: https://github.com/qznc/d-money/pull/12
there's also parsing that is not super compliant EUR y = EUR("1.000.000,00"); writeln(y); // F!
Added comment here on git: https://github.com/qznc/d-money/pull/12#issuecomment-633272988 ===================================== The parsing regex used internally is a very simple one: https://github.com/qznc/d-money/blob/master/source/money.d#L98 I don't want to enhanced it to be a full sscanf(). This is just a maintenance bug fix.
May 24
parent reply Basile B. <b2.temp gmx.com> writes:
On Sunday, 24 May 2020 at 18:30:38 UTC, mw wrote:
 On Sunday, 24 May 2020 at 18:15:09 UTC, Basile B. wrote:
 On Sunday, 24 May 2020 at 17:06:43 UTC, mw wrote:
 On Friday, 22 May 2020 at 05:26:45 UTC, mw wrote:
[...]
For anyone who's using this package, just submitted the PR to fix it: https://github.com/qznc/d-money/pull/12
there's also parsing that is not super compliant EUR y = EUR("1.000.000,00"); writeln(y); // F!
Added comment here on git: https://github.com/qznc/d-money/pull/12#issuecomment-633272988 ===================================== The parsing regex used internally is a very simple one: https://github.com/qznc/d-money/blob/master/source/money.d#L98 I don't want to enhanced it to be a full sscanf(). This is just a maintenance bug fix.
No this is an issue on itw own. The thousands and the decimal separator should be template parameters or customizable in some way.
May 24
parent mw <mingwu gmail.com> writes:
On Sunday, 24 May 2020 at 18:54:55 UTC, Basile B. wrote:
 On Sunday, 24 May 2020 at 18:30:38 UTC, mw wrote:
 On Sunday, 24 May 2020 at 18:15:09 UTC, Basile B. wrote:
 On Sunday, 24 May 2020 at 17:06:43 UTC, mw wrote:
 On Friday, 22 May 2020 at 05:26:45 UTC, mw wrote:
[...]
For anyone who's using this package, just submitted the PR to fix it: https://github.com/qznc/d-money/pull/12
there's also parsing that is not super compliant EUR y = EUR("1.000.000,00"); writeln(y); // F!
Added comment here on git: https://github.com/qznc/d-money/pull/12#issuecomment-633272988 ===================================== The parsing regex used internally is a very simple one: https://github.com/qznc/d-money/blob/master/source/money.d#L98 I don't want to enhanced it to be a full sscanf(). This is just a maintenance bug fix.
No this is an issue on itw own. The thousands and the decimal separator should be template parameters or customizable in some way.
As said, this is maintenance fix, I don't want to change the way how the original package is designed before, and add template parameters. But for your case, I added filter out commas ',': https://github.com/qznc/d-money/pull/12/commits/ff7aaa2b70dc91e2f93a5a26ecd04185de273f3a auto y = EUR("1,000.000,00"); auto z = EUR("1000"); // writeln(y); assert(y == z); BTW, the new ctor throw out exception on bad input it cannot recognize: money.ParseError !isNumeric: 1.000.000,00(105): Parse error is correct behavior.
May 24