Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fix for #230 #231

Closed
wants to merge 1 commit into from

4 participants

@muxcmux
Collaborator

define spec and implement parsing currencies with different thousands_separators and decimal_marks. issue #230

@semmons99
Owner

Hey @muxcmux,

The problem we're trying to fix here is that when parsing a string, it should respect the given currencies rules for decimal markers and thousands separators and not try to second guess it.

@muxcmux
Collaborator

Wouldn't that introduce backward incompatibilities? For example, this:

"100.37 EUR"      => Money.new(100_37, "EUR")     ,

would no longer be valid

@semmons99
Owner

I always have a hard time deciding what to do here.

Anyone @RubyMoney/money-devs @RubyMoney/money-admins care to weigh in?

@semaperepelitsa
Collaborator

The formatting rules do not depend on currency, they depend on locale. In US English you write "€10,370.55" and in Russian - "10 370,55 €".

This doesn't matter when you are dealing with a local currency only. But it is not uncommon to have an additional one.

@mikaelwikman
Collaborator

Yes, I would agree with @semaperepelitsa. The thousands and decimal separators are entirely decided by locale. For example, in Swedish you would always write "," before the fractional part. I.e Swedish 10,37 is written "10.37" in Ruby. This is, of course, a major pain in the ass since the numpad period button won't work unless the keyboard layout is changed to "en/us".

@muxcmux
Collaborator

@mikaelwikman I couldn't agree more, but that seems to raise more questions than it answers. In such case, why do we need to keep information on thousands separator and the decimal sign? This should be locale-specific, like @semaperepelitsa suggested.

@mikaelwikman
Collaborator

Indeed. We might need to rethink all current functionality that involves the thousands separator and fractal sign. We could probably do some automatic mapping from the current settings by using Country gem to figure out the locale, and use this by default. This way we'd still be backwards compatible. After that the logical course of action would be to add locale to the interfaces and let it fall-back to the one defined in the configuration.
I'm currently swamped with work, all scheduled up to end of January. So someone else would need to take a crack at it :)

@semmons99 semmons99 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 27, 2012
  1. @muxcmux

    define spec and implement parsing currencies with different thousands…

    muxcmux authored
    …_separators and decimal_marks. issue #230
This page is out of date. Refresh to see the latest.
View
63 lib/money/money/parsing.rb
@@ -282,61 +282,20 @@ def extract_cents(input, currency = Money.default_currency)
major, minor = num.gsub(decimal_mark, '').split(thousands_separator)
min = 0 unless min
when 1
- # we can't determine if the comma or period is supposed to be a decimal_mark or a thousands_separator
- # e.g.
- # 1,00 - comma is a thousands_separator
- # 1.000 - period is a thousands_separator
- # 1,000 - comma is a decimal_mark
- # 1,000,000 - comma is a decimal_mark
- # 10000,00 - comma is a thousands_separator
- # 1000,000 - comma is a thousands_separator
-
- # assign first decimal_mark for reusability
- decimal_mark = used_decimal_marks.first
-
- # When we have identified the decimal mark character
- if decimal_char == decimal_mark
+ if used_decimal_marks.first == decimal_char
+ # the character is a decimal mark, so we can safely
+ # assume that digits before the decimal mark are majors
+ # and digits after it are minors
major, minor = num.split(decimal_char)
-
else
- # decimal_mark is used as a decimal_mark when there are multiple instances, always
- if num.scan(decimal_mark).length > 1 # multiple matches; treat as decimal_mark
- major, minor = num.gsub(decimal_mark, ''), 0
+ # the character must be a thousands separator
+ # but we need to make sure that this is the intention
+ if (num.split(currency.thousands_separator).last.length == 3)
+ # yes, we can safely assume that this was the intention indeed
+ major, minor = num.gsub(currency.thousands_separator, ''), 0
else
- # ex: 1,000 - 1.0000 - 10001.000
- # split number into possible major (dollars) and minor (cents) values
- possible_major, possible_minor = num.split(decimal_mark)
- possible_major ||= "0"
- possible_minor ||= "00"
-
- # if the minor (cents) length isn't 3, assign major/minor from the possibles
- # e.g.
- # 1,00 => 1.00
- # 1.0000 => 1.00
- # 1.2 => 1.20
- if possible_minor.length != 3 # thousands_separator
- major, minor = possible_major, possible_minor
- else
- # minor length is three
- # let's try to figure out intent of the thousands_separator
-
- # the major length is greater than three, which means
- # the comma or period is used as a thousands_separator
- # e.g.
- # 1000,000
- # 100000,000
- if possible_major.length > 3
- major, minor = possible_major, possible_minor
- else
- # number is in format ###{sep}### or ##{sep}### or #{sep}###
- # handle as , is sep, . is thousands_separator
- if decimal_mark == '.'
- major, minor = possible_major, possible_minor
- else
- major, minor = "#{possible_major}#{possible_minor}", 0
- end
- end
- end
+ # nope, the input intends the thousands separator as a decimal mark
+ major, minor = num.split(currency.thousands_separator)
end
end
else
View
4 spec/core_extensions_spec.rb
@@ -64,6 +64,10 @@
"100 EUR" => Money.new(100_00, "EUR") ,
"100.37 EUR" => Money.new(100_37, "EUR") ,
"100,37 EUR" => Money.new(100_37, "EUR") ,
+ "10,370 EUR" => Money.new(1_037, "EUR") ,
+ "10.370 EUR" => Money.new(1_037_000, "EUR") ,
+ "10.370 USD" => Money.new(1_037, "USD") ,
+ "10,370 USD" => Money.new(1_037_000, "USD") ,
"100,000.00 USD" => Money.new(100_000_00, "USD") ,
"100.000,00 EUR" => Money.new(100_000_00, "EUR") ,
"1,000 USD" => Money.new(1_000_00, "USD") ,
View
23 spec/money/parsing_spec.rb
@@ -296,6 +296,29 @@
it "correctly treats pipe marks '|' in input (regression test)" do
Money.extract_cents('100|0').should == Money.extract_cents('100!0')
end
+
+ it "treats the thousands_separator as such if and only if there are three digits after it" do
+ eur = Money::Currency.find(:eur)
+ usd = Money::Currency.find(:usd)
+
+ Money.extract_cents('100.00', eur).should == 10_000
+ Money.extract_cents('100.0', eur).should == 10_000
+ Money.extract_cents('100,00', eur).should == 10_000
+ Money.extract_cents('100,0', eur).should == 10_000
+ Money.extract_cents('100.0000', eur).should == 10_000
+ Money.extract_cents('100,0000', eur).should == 10_000
+ Money.extract_cents('100,00', usd).should == 10_000
+ Money.extract_cents('100,0', usd).should == 10_000
+ Money.extract_cents('100.00', usd).should == 10_000
+ Money.extract_cents('100.0', usd).should == 10_000
+ Money.extract_cents('100.0000', usd).should == 10_000
+ Money.extract_cents('100,0000', usd).should == 10_000
+
+ Money.extract_cents('100.000', eur).should == 10_000_000
+ Money.extract_cents('100,000', eur).should == 10_000
+ Money.extract_cents('100,000', usd).should == 10_000_000
+ Money.extract_cents('100.000', usd).should == 10_000
+ end
end
context "given the same inputs to .parse and .from_*" do
Something went wrong with that request. Please try again.