Thread overview
d-money ctor(string) has bug, be careful! EUR("1.23456") ==> ctor ==> 3.3456EUR
May 22
mw
May 24
mw
May 24
Basile B.
May 24
mw
May 24
Basile B.
May 24
mw
May 22
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 24
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
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
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
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
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.