Add currency validator #368

regularmike · 2015-02-24T18:23:58Z

As per suggestion, based the isCurrency validator on isMobilePhone, but used currency codes instead of locales because it seemed more reasonable, for example, to have a EUR validator that covers some slight variations instead of using all locales in the eurozone separately. Reliable information on formatting conventions seems difficult to find, so I based the RegEx's on ASP.NET/C# formatted strings with various CultureInfo instances. Added mocha tests as well.

coveralls · 2015-02-24T18:30:49Z

Coverage decreased (-0.27%) to 97.17% when pulling 3a19a0c on regularmike:master into bab1c14 on chriso:master.

chriso · 2015-02-25T03:36:40Z

validator.js

+        'CNY': /^¥?\-?([0-9]{1,3})?((,[0-9]{3})*|([0-9]{3})*)(\.[0-9]{2})?$/,
+        'ZAR': /^(R|R )?\-?([0-9]{1,3})?(( [0-9]{3})*|([0-9]{3})*)(,[0-9]{2})?$/,
+        'AUD': /^\$?\-?([0-9]{1,3})?((,[0-9]{3})*|([0-9]{3})*)(\.[0-9]{2})?$/,
+        'HKD': /^\$?\-?([0-9]{1,3})?((,[0-9]{3})*|([0-9]{3})*)(\.[0-9]{2})?$/,


AUD, HKD and USD have the same regex patterns. Can you pull it out into a variable instead of repeating it?

This matches a leading comma, e.g. ,123 or $-,123.

Yup, was trying to modify to accept ".10". I see a few other issues with spaces and leading zeros. Will fix tomorrow and use a variable for dollar pattern. I thought maybe I would come across some variations with the different dollar formats but I haven't seen any yet.

coveralls · 2015-02-25T14:41:58Z

Coverage decreased (-0.27%) to 97.18% when pulling 268f3d0 on regularmike:master into bab1c14 on chriso:master.

regularmike · 2015-02-25T15:56:02Z

Looks like some of these match empty string and the currency symbol by itself. Need to add some lookaheads, perhaps. Will update soon. I know I've been a bit overeager.

regularmike · 2015-02-25T17:28:48Z

I think this is fairly solid now. Added more tests for edge cases like the ones you mentioned and fixed a few others that I found like solitary currency symbol, empty string, and single space. Have a look when you get a chance.

coveralls · 2015-02-25T19:02:45Z

Coverage decreased (-0.27%) to 97.18% when pulling 0dd8611 on regularmike:master into bab1c14 on chriso:master.

chriso · 2015-02-26T02:41:21Z

Thanks. Couple more edge cases I found:

$- => true
- => true
$10,123 => false

What about this?

/^(?!\$?\-?$)\$?\-?(0|[1-9](\d*|\d{0,2}(,\d{3})*))(\.\d{2})?$/

coveralls · 2015-02-26T03:39:31Z

Coverage decreased (-0.27%) to 97.18% when pulling 59f6cbb on regularmike:master into bab1c14 on chriso:master.

regularmike · 2015-02-26T03:39:33Z

Well, two edge cases and one glaring typical case. Man, it's been a while since I've done any non-trivial regular expressions. Your version looks good. Adjusted the others accordingly, and added those test cases and other similar ones for all currencies.

chriso · 2015-02-26T13:38:28Z

Two more: '- €', '-€'. Almost there 😉

That euro one is getting quite complicated. Do they really accept the currency symbol on either side?

regularmike · 2015-02-26T14:43:08Z

Nice catch. Unfortunately they do accept it like that in certain places. I
was looking at the ones in the mobile phone list in C#, which had some
slight variations, so I looked into it more. This page doesn't match what
.NET is doing exactly, but I think it shows all the cases that we are
trying to cover:

http://www.evertype.com/standards/euro/formats.html

Testing in C# confirms that the euro sign can be on either end, but I
haven't actually seen one that doesn't use a space before the currency
symbol at the end.

10123.25.ToString("c", new CultureInfo("el-GR")) => 10.123,25 €
10123.25.ToString("c", new CultureInfo("fr-FR")) => 10 123,25 €
10123.25.ToString("c", new CultureInfo("en-IE")) => €10,123.25
10123.25.ToString("c", new CultureInfo("it-IT")) => € 10.123.25

At this point I'm thinking it would be better to do this by
language/culture string like the mobile phones. It might seem more natural
to use USD than en-US, but I think each regex would become easier and more
reliable. I imagine you're having the same thought. However, if we do that
it seems like we should make the list much more comprehensive, at least
including all the eurozone countries. Should we include any others that we
haven't already?

On Thu, Feb 26, 2015 at 8:38 AM, chriso notifications@github.com wrote:

Two more: '- €', '-€'. Almost there [image: 😉]

That euro one is getting quite complicated. Do they really accept the
currency symbol on either side?

—
Reply to this email directly or view it on GitHub
#368 (comment).

chriso · 2015-02-27T01:56:51Z

An even better alternative might be to generate the regex based on user input.

Something along the lines of

function currencyRegex(options) {
    var symbol = '\\' + options.symbol + '?'
      , negative = '\\-?'
      , amount_without_sep = '[1-9]\\d*'
      , amount_with_sep = '[1-9]\\d{0,2}(\\' + options.thousands + '\\d{3})*'
      , valid_amounts = ['0', amount_without_sep, amount_with_sep]
      , amount = '(' + valid_amounts.join('|') + ')?'
      , decimal = '(\\' + options.decimal + '\\d{2})?$'
      , prefix = symbol + negative;
    return new RegExp(
        '^' +
        '(?!' + prefix + '$)' + // ensure there's an amount and/or decimal
        prefix +
        amount +
        decimal +
        '$'
    );
}

validator.isCurrency = function (str, options) {
    return currencyRegex(options).test(str);
}

validator.isCurrency('$10,123', {symbol: '$', thousands: ',', decimal: '.'});
// => true

It's then quite easy to customize the validator. For example, you could add an option to allow whitespace, another to require a currency symbol and another to exclude negative values. You could also default each option to USD, since many locales share the same parameters.

regularmike · 2015-02-27T03:29:13Z

I like this, as long as there is no significant demand for validating
without having any knowledge of the format. And I suppose there may not be.
This seems like the most flexible solution so far. I'll keep the current
tests, more or less, but just arrange the appropriate validator before each
set.

The msdn article you originally referenced also points out that the
negative sign could be before or after the currency symbol, or even after
the digits, but of course the default will be at the front of the string so
most users won't have to specify that option. We should also support
parentheses for negatives since it's the preferred format for financial
applications in the US. Not sure if that should be the default, though.
Will let you know when I have the tests working with this solution. Thanks.

https://msdn.microsoft.com/en-us/goglobal/bb688126.aspx

On Thu, Feb 26, 2015 at 8:56 PM, chriso notifications@github.com wrote:

An even better alternative might be to generate the regex based on user
input.

Something along the lines of

function currencyRegex(options) {
var symbol = '' + options.symbol + '?'
, negative = '-?'
, amount_without_sep = '[1-9]\d_'
, amount_with_sep = '[1-9]\d{0,2}(' + options.thousands + '\d{3})_'
, amount = '(' + ['0', amount_without_sep, amount_with_sep].join('|') + ')?'
, decimal = '(' + options.decimal + '\d{2})?$'
, prefix = symbol + negative;
return new RegExp(
'^' +
'(?!' + prefix + '$)' + // ensure there's an amount and/or decimal
prefix +
amount +
decimal +
'$'
);
}
validator.isCurrency = function (str, options) {
return currencyRegex(options).test(str);
}

validator.isCurrency('$10,123', {symbol: '$', thousands: ',', decimal: '.'});// => true

It's then quite easy to customize the validator. For example, you could
add an option to allow whitespace, another to require a currency symbol and
another to exclude negative values. You could also default each option to
USD, since many locales share the same parameters.

—
Reply to this email directly or view it on GitHub
#368 (comment).

coveralls · 2015-03-02T00:30:57Z

Coverage decreased (-0.09%) to 97.35% when pulling 15122c2 on regularmike:master into bab1c14 on chriso:master.

regularmike · 2015-03-02T00:53:05Z

OK, I've implemented it in the new, customizable way. A few things to note:

Default is en-US format but with negative sign instead of parens. Parens is the standard for display but for entry I think negative sign is more appropriate.
One of the options I came up with was allow_negative_sign_placeholder, which means a space is allowed only for a non-negative value, which I noticed is how the South African Rand (en-ZA) is formatted, e.g. R 123 but R-123
That msdn article shows Netherlands euros (nl-NL) with the negative sign after the digits. I don't think that's the official format, and .NET is not doing it that way currently. This option is still supported (negative_sign_after_digits) but might not be needed by anyone as far as I can tell.
No more than one consecutive space is ever allowed anywhere, and when a space is allowed it's always optional. Not sure if it would be worth adding an option for enforcing that space.
Tried to be pretty thorough with tests but there is now a huge number of combinations of options. I think we have the most common ones covered, but anyway I think it would make more sense to look at formats that may not be similar to any of the ones listed below (if there are any) than to try to test a large number of arbitrary combinations.
Finally, once this is ready I'll be happy to squash commits and send a cleaner pull request, but I figure it might take some more passes.

UK: -£10,123.25
Ireland: -€10,123.25
Italy: -€ 10.123,25
Greece: -10.123,25 €
France: -10 123,25 €
Australia: -$10,123.25
China: ¥-10,123.25
South Africa: R-10 123,25
Denmark: kr. -10.123,25
Netherlands: € -10.123,25
US: ($10,123.25)
CA: -$10,123.25

chriso · 2015-03-02T09:04:10Z

Looks great, thanks 👍

Want to clean up the commits and then I'll merge?

coveralls · 2015-03-02T16:28:32Z

Coverage decreased (-0.09%) to 97.35% when pulling e9a4701 on regularmike:master into bab1c14 on chriso:master.

regularmike · 2015-03-02T16:44:50Z

OK, all set. Also updated README.

coveralls · 2015-03-02T16:46:24Z

Coverage decreased (-0.09%) to 97.35% when pulling 8364c12 on regularmike:master into bab1c14 on chriso:master.

chriso · 2015-03-03T02:07:48Z

Perfect. Thanks again.

Add currency validator

chriso · 2015-03-03T02:13:28Z

Available in v3.33.0.

regularmike · 2015-03-03T14:14:40Z

No problem. It was very educational. And happy to contribute to something I use all the time. Here to help if anybody brings up anything that we overlooked.

chriso reviewed Feb 25, 2015
View reviewed changes

add currency validator

8364c12

regularmike force-pushed the master branch from e9a4701 to 8364c12 Compare March 2, 2015 16:43

chriso added a commit that referenced this pull request Mar 3, 2015

Merge pull request #368 from regularmike/master

c3fe6f3

Add currency validator

chriso merged commit c3fe6f3 into validatorjs:master Mar 3, 2015

chriso mentioned this pull request Mar 3, 2015

Add USD($) validator #360

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add currency validator #368

Add currency validator #368

regularmike commented Feb 24, 2015

coveralls commented Feb 24, 2015

chriso Feb 25, 2015

chriso Feb 25, 2015

regularmike Feb 25, 2015

coveralls commented Feb 25, 2015

regularmike commented Feb 25, 2015

regularmike commented Feb 25, 2015

coveralls commented Feb 25, 2015

chriso commented Feb 26, 2015

coveralls commented Feb 26, 2015

regularmike commented Feb 26, 2015

chriso commented Feb 26, 2015

regularmike commented Feb 26, 2015

chriso commented Feb 27, 2015

regularmike commented Feb 27, 2015

coveralls commented Mar 2, 2015

regularmike commented Mar 2, 2015

chriso commented Mar 2, 2015

coveralls commented Mar 2, 2015

regularmike commented Mar 2, 2015

coveralls commented Mar 2, 2015

chriso commented Mar 3, 2015

chriso commented Mar 3, 2015

regularmike commented Mar 3, 2015

Add currency validator #368

Add currency validator #368

Conversation

regularmike commented Feb 24, 2015

coveralls commented Feb 24, 2015

chriso Feb 25, 2015

Choose a reason for hiding this comment

chriso Feb 25, 2015

Choose a reason for hiding this comment

regularmike Feb 25, 2015

Choose a reason for hiding this comment

coveralls commented Feb 25, 2015

regularmike commented Feb 25, 2015

regularmike commented Feb 25, 2015

coveralls commented Feb 25, 2015

chriso commented Feb 26, 2015

coveralls commented Feb 26, 2015

regularmike commented Feb 26, 2015

chriso commented Feb 26, 2015

regularmike commented Feb 26, 2015

chriso commented Feb 27, 2015

regularmike commented Feb 27, 2015

coveralls commented Mar 2, 2015

regularmike commented Mar 2, 2015

chriso commented Mar 2, 2015

coveralls commented Mar 2, 2015

regularmike commented Mar 2, 2015

coveralls commented Mar 2, 2015

chriso commented Mar 3, 2015

chriso commented Mar 3, 2015

regularmike commented Mar 3, 2015