Currency validation support #4430

olegL1337 · 2021-12-16T12:03:03Z

Add Currency format to TextBoxExtensions #4188

#4188
So I added Currency option to ValidationType enum and modified ValidateTextBox metod with adding new option to switch-case clause. Because I could not modify StringExtensions to add isCurrency mehtod, I decided to simply add modifications in such switch-case clause.

Regex pattern I used: @"(^\d*\.\d{2}$)"
Mathes: $100.00, $100, $10.25
Non-Matches: 100., $10.233, $10.

ghost · 2021-12-16T12:03:07Z

Thanks olegL1337 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

net-foundation-cla · 2021-12-16T12:03:15Z

All CLA requirements met.

net-foundation-cla · 2021-12-16T12:03:15Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ olegL1337 sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Arlodotexe · 2021-12-16T17:56:37Z

Microsoft.Toolkit.Uwp.UI/Extensions/TextBox/TextBoxExtensions.Regex.Internals.cs

+                    // @"(^\d*\.\d{2}$)" regex pattern to detect currency sign with currency value
+                    // Mathes: $100.00, $100, $10.25
+                    // Non-Matches: 100., $10.233, $10.
+                    regexMatch = Regex.IsMatch(textBox.Text, @"(^\d*\.\d{2}$)");


We should handle currency symbols and decimal separators other than USD. Luckily, dotnet has this functionality built in and we don't need to fiddle with Regex.

System.Globalization.NumberFormatInfo should have everything we need for this.

Firing up an empty console app, here's what we have to work with.

string FormatCurrency(string value) { // Parse the entered data into a decimal, allowing existing currency symbols. decimal.TryParse(value, NumberStyles.Currency, out var amount); // Format the decimal into a human readable string for the current culture. return amount.ToString("C2", CultureInfo.CurrentCulture); } // Stress testing USD FormatCurrency("500"); // $500.00 FormatCurrency("5"); // $5.00 FormatCurrency("5.00"); // $5.00 FormatCurrency(".05"); // $0.05 FormatCurrency(".5"); // $0.50 FormatCurrency("$.5"); // $0.50 // With other cultures FormatCurrency("5"); // nl-NL, € 5,00 FormatCurrency("5"); // fr-FR, 5,00 € FormatCurrency("5"); // ja-JP, ¥5.00 FormatCurrency("5"); // en-GB, £5.00

michael-hawker

Thanks @olegL1337 for the PR. Agree with @Arlodotexe that we want to make sure to handle localized scenarios. It'd be good to put some localized examples in some unit tests to make sure we guard against changes in the future as well.

Also FYI @NaftoliOst who was interested in helping with this feature too in case they want to review or have any suggestions.

michael-hawker · 2021-12-16T18:31:56Z

Microsoft.Toolkit.Uwp.UI/Extensions/TextBox/TextBoxExtensions.Regex.Data.cs

+            /// <summary>
+            /// Currency validation
+            /// </summary>
+            Currency,
+


Should be added at the end so not a breaking change.

NaftoliOst · 2021-12-16T23:45:26Z

Yes, I wrote the code for this, but didn't get around to testing it, which is why I haven't submitted a PR.

This is the Regex I used to match all currencies:

+        /// <summary>
+        /// Regular expression for matching a decimal with or without a currency character.
+        /// </summary>
+        internal const string CurrencyRegex = @"^\p{Sc}?\d+\.?\d*$";

michael-hawker · 2021-12-17T22:11:04Z

Thanks for chiming in @NaftoliOst! I think as @Arlodotexe called out, if there's actual .NET APIs which can do this validation without RegEx, it'd be great to try using those instead and help guard us against localization/regional issues.

Arlodotexe · 2021-12-17T22:13:59Z

Regex is a tricky thing to globalize. Not all currencies use a decimal as their separator (if any separator), many place the currency symbol in different places with different spacing.

Since we're just checking if a string is valid as a currency, we don't need to use regex. Try this:

    // Parse the entered data into a decimal, allowing existing currency symbols. Uses current UI culture.
    regexMatch = decimal.TryParse(value, NumberStyles.Currency, out _);

May also need to rename the regexMatch bool to isValid or something :)

edit: @michael-hawker beat me to it while I was spell checking 😄

NaftoliOst · 2021-12-20T11:50:54Z

I wasn't aware of the .NET APIs for currency validation with localized formats, thanks for pointing that out @michael-hawker and @Arlodotexe. It definitely make more sense to make use of that ready built method that takes into account local formats and has stood the test of time.

I would also suggest putting the code in an IsCurrency extension method in the StringExtensions.cs file and then call that method here in the TextBoxExtensions.Regex.Internals.cs file
regexMatch = textBox.Text.IsCurrency();

That would be consistent with the other validation checks ( IsDecimal() IsPhoneNumber() etc.) and allows the IsCurrency method to be used in future for other use cases, should the need arise, without having to rewrite the code.

Arlodotexe · 2022-01-13T17:47:12Z

Microsoft.Toolkit.Uwp.UI/Extensions/TextBox/TextBoxExtensions.Regex.Internals.cs

+                    // @"(^\d*\.\d{2}$)" regex pattern to detect currency sign with currency value
+                    // Mathes: $100.00, $100, $10.25
+                    // Non-Matches: 100., $10.233, $10.
+                    regexMatch = Regex.IsMatch(textBox.Text, @"^\$( )*\d*(.\d{1,2})?$");


Until we can reference this new extension method, let's use the .NET Api directly so we can close this off :)

Suggested change

regexMatch = Regex.IsMatch(textBox.Text, @"^\$( )*\d*(.\d{1,2})?$");

// Parse the entered data into a decimal, allowing existing currency symbols. Uses current UI culture.

regexMatch = decimal.TryParse(value, NumberStyles.Currency, out _);

The TryParse method doesn't accept 3 parameters, there needs to be an IFormatProvider parameter as well.

I've written the code for this with CultureInfo.CurrentCulture as the IFormatProvider like you did in the PR for the new extension method (unless @olegL1337 is still planning on updating this PR)

My only question is, it this the best way to validate the currencies? with the IFormatProvider set to CultureInfo.CurrentCulture the validation will only pass for the user's local currency. Is there a way of including all currency cultures so that any currency will pass as valid?

Afaik, there isn't a CultureInfo that represents "all" cultures. However, we can get all available cultures and check against each one separately.

static decimal? TryParseCurrency(string str) { foreach (var culture in CultureInfo.GetCultures(CultureTypes.AllCultures)) { if (decimal.TryParse(str, NumberStyles.Currency, culture, out var amount)) return amount; } return null; }

See it in action: https://dotnetfiddle.net/HW8wZ4

Got it!
So do you think it would be better to implement it like that? The trade off would be the efficiency, particularly when it needs to loop though all of the cultures at each keystroke of user input. Is that something you think we should be concerned about @Arlodotexe?

BTW I've submitted a PR here that closes this issue if @olegL1337 isn't planning on going further with this PR

This feels this needs more options.

Perhaps we add a way to provide the current or a specific culture for ValidationType.Currency, and add an additional ValidationType.AnyCurrency if they'd like to check all cultures. @michael-hawker what do you think?

michael-hawker · 2022-02-03T21:24:36Z

@olegL1337 going to close this PR as we haven't heard from you and it has been picked up by @NaftoliOst now. Thanks! See #4469 for now.

currency validation support

fa1878b

ghost requested review from michael-hawker and azchohfi December 16, 2021 12:03

regex fix :)

392b783

Arlodotexe self-requested a review December 16, 2021 17:18

Arlodotexe reviewed Dec 16, 2021

View reviewed changes

michael-hawker requested changes Dec 16, 2021

View reviewed changes

ghost added the needs author feedback 📝 label Dec 16, 2021

michael-hawker added this to the 8.0 milestone Dec 16, 2021

michael-hawker added the extensions ⚡ label Dec 16, 2021

Arlodotexe mentioned this pull request Dec 20, 2021

Add IsCurrency to StringExtensions. CommunityToolkit/dotnet#72

Open

Arlodotexe reviewed Jan 13, 2022

View reviewed changes

michael-hawker closed this Feb 3, 2022

michael-hawker removed this from the 8.0 milestone Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Currency validation support #4430

Currency validation support #4430

olegL1337 commented Dec 16, 2021

ghost commented Dec 16, 2021

net-foundation-cla bot commented Dec 16, 2021 •

edited

net-foundation-cla bot commented Dec 16, 2021

Arlodotexe Dec 16, 2021 •

edited

michael-hawker left a comment •

edited

michael-hawker Dec 16, 2021

NaftoliOst commented Dec 16, 2021

michael-hawker commented Dec 17, 2021

Arlodotexe commented Dec 17, 2021 •

edited

NaftoliOst commented Dec 20, 2021

Arlodotexe Jan 13, 2022

NaftoliOst Jan 24, 2022

Arlodotexe Jan 26, 2022

NaftoliOst Jan 26, 2022

Arlodotexe Jan 29, 2022

michael-hawker commented Feb 3, 2022

	regexMatch = Regex.IsMatch(textBox.Text, @"^\$( )\d(.\d{1,2})?$");
	// Parse the entered data into a decimal, allowing existing currency symbols. Uses current UI culture.
	regexMatch = decimal.TryParse(value, NumberStyles.Currency, out _);

Currency validation support #4430

Currency validation support #4430

Conversation

olegL1337 commented Dec 16, 2021

Add Currency format to TextBoxExtensions #4188

ghost commented Dec 16, 2021

net-foundation-cla bot commented Dec 16, 2021 • edited

net-foundation-cla bot commented Dec 16, 2021

Arlodotexe Dec 16, 2021 • edited

Choose a reason for hiding this comment

michael-hawker left a comment • edited

Choose a reason for hiding this comment

michael-hawker Dec 16, 2021

Choose a reason for hiding this comment

NaftoliOst commented Dec 16, 2021

michael-hawker commented Dec 17, 2021

Arlodotexe commented Dec 17, 2021 • edited

NaftoliOst commented Dec 20, 2021

Arlodotexe Jan 13, 2022

Choose a reason for hiding this comment

NaftoliOst Jan 24, 2022

Choose a reason for hiding this comment

Arlodotexe Jan 26, 2022

Choose a reason for hiding this comment

NaftoliOst Jan 26, 2022

Choose a reason for hiding this comment

Arlodotexe Jan 29, 2022

Choose a reason for hiding this comment

michael-hawker commented Feb 3, 2022

net-foundation-cla bot commented Dec 16, 2021 •

edited

Arlodotexe Dec 16, 2021 •

edited

michael-hawker left a comment •

edited

Arlodotexe commented Dec 17, 2021 •

edited