[ 
https://issues.apache.org/jira/browse/GROOVY-12147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095456#comment-18095456
 ] 

ASF GitHub Bot commented on GROOVY-12147:
-----------------------------------------

Copilot commented on code in PR #2688:
URL: https://github.com/apache/groovy/pull/2688#discussion_r3562836208


##########
src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java:
##########
@@ -3466,6 +3469,103 @@ public static BigInteger toBigInteger(final 
CharSequence self) {
         return new BigInteger(self.toString().trim());
     }
 
+    /**
+     * Parses a CharSequence into a Number using the given locale, honouring
+     * locale-specific grouping and decimal symbols. Unlike {@link 
#toBigDecimal(CharSequence)},
+     * parsing is lenient (a leading numeric prefix is accepted) and 
grouping-aware.
+     * <pre class="groovyTestCase">
+     * assert '1.234,5'.toNumber(Locale.GERMANY) == 1234.5
+     * </pre>
+     *
+     * @param self   a CharSequence
+     * @param locale the locale defining the number symbols
+     * @return the parsed Number
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toNumber(final CharSequence self, final Locale 
locale) {
+        try {
+            return 
NumberFormat.getNumberInstance(locale).parse(self.toString());
+        } catch (ParseException e) {
+            throw new NumberFormatException("Unparseable number: \"" + self + 
"\"");
+        }

Review Comment:
   The new parsing methods wrap ParseException in NumberFormatException but 
drop the original cause, making failures harder to diagnose (stack traces lose 
the underlying ParseException details). Consider chaining the cause via 
initCause while keeping the public exception type the same.



##########
src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java:
##########
@@ -3466,6 +3469,103 @@ public static BigInteger toBigInteger(final 
CharSequence self) {
         return new BigInteger(self.toString().trim());
     }
 
+    /**
+     * Parses a CharSequence into a Number using the given locale, honouring
+     * locale-specific grouping and decimal symbols. Unlike {@link 
#toBigDecimal(CharSequence)},
+     * parsing is lenient (a leading numeric prefix is accepted) and 
grouping-aware.
+     * <pre class="groovyTestCase">
+     * assert '1.234,5'.toNumber(Locale.GERMANY) == 1234.5
+     * </pre>
+     *
+     * @param self   a CharSequence
+     * @param locale the locale defining the number symbols
+     * @return the parsed Number
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toNumber(final CharSequence self, final Locale 
locale) {
+        try {
+            return 
NumberFormat.getNumberInstance(locale).parse(self.toString());
+        } catch (ParseException e) {
+            throw new NumberFormatException("Unparseable number: \"" + self + 
"\"");
+        }
+    }
+
+    /**
+     * Parses a CharSequence into a Number using the currency format of the 
default locale.
+     * The result is an exact {@link BigDecimal}.
+     *
+     * @param self a CharSequence
+     * @return the parsed Number (a BigDecimal)
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toCurrencyNumber(final CharSequence self) {
+        return toCurrencyNumber(self, Locale.getDefault());
+    }
+
+    /**
+     * Parses a CharSequence into a Number using the currency format of the 
given locale.
+     * The result is an exact {@link BigDecimal}.
+     * <pre class="groovyTestCase">
+     * assert '$1,234.50'.toCurrencyNumber(Locale.US) == 1234.50g
+     * </pre>
+     *
+     * @param self   a CharSequence
+     * @param locale the locale defining the currency format
+     * @return the parsed Number (a BigDecimal)
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toCurrencyNumber(final CharSequence self, final 
Locale locale) {
+        NumberFormat nf = NumberFormat.getCurrencyInstance(locale);
+        if (nf instanceof DecimalFormat) ((DecimalFormat) 
nf).setParseBigDecimal(true);
+        try {
+            return nf.parse(self.toString());
+        } catch (ParseException e) {
+            throw new NumberFormatException("Unparseable currency: \"" + self 
+ "\"");
+        }
+    }

Review Comment:
   toCurrencyNumber currently relies on NumberFormat.parse(String), which can 
succeed on a numeric prefix and ignore trailing non-currency text. Also, the 
Javadoc promises an exact BigDecimal, but if the returned Number isn't a 
BigDecimal (e.g., non-DecimalFormat implementation), callers may get a 
different type. Consider trimming input, enforcing full-string consumption via 
ParsePosition, and normalizing the return value to BigDecimal.



##########
src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java:
##########
@@ -3466,6 +3469,103 @@ public static BigInteger toBigInteger(final 
CharSequence self) {
         return new BigInteger(self.toString().trim());
     }
 
+    /**
+     * Parses a CharSequence into a Number using the given locale, honouring
+     * locale-specific grouping and decimal symbols. Unlike {@link 
#toBigDecimal(CharSequence)},
+     * parsing is lenient (a leading numeric prefix is accepted) and 
grouping-aware.
+     * <pre class="groovyTestCase">
+     * assert '1.234,5'.toNumber(Locale.GERMANY) == 1234.5
+     * </pre>
+     *
+     * @param self   a CharSequence
+     * @param locale the locale defining the number symbols
+     * @return the parsed Number
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toNumber(final CharSequence self, final Locale 
locale) {
+        try {
+            return 
NumberFormat.getNumberInstance(locale).parse(self.toString());
+        } catch (ParseException e) {
+            throw new NumberFormatException("Unparseable number: \"" + self + 
"\"");
+        }
+    }
+
+    /**
+     * Parses a CharSequence into a Number using the currency format of the 
default locale.
+     * The result is an exact {@link BigDecimal}.
+     *
+     * @param self a CharSequence
+     * @return the parsed Number (a BigDecimal)
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toCurrencyNumber(final CharSequence self) {
+        return toCurrencyNumber(self, Locale.getDefault());
+    }
+
+    /**
+     * Parses a CharSequence into a Number using the currency format of the 
given locale.
+     * The result is an exact {@link BigDecimal}.
+     * <pre class="groovyTestCase">
+     * assert '$1,234.50'.toCurrencyNumber(Locale.US) == 1234.50g
+     * </pre>
+     *
+     * @param self   a CharSequence
+     * @param locale the locale defining the currency format
+     * @return the parsed Number (a BigDecimal)
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toCurrencyNumber(final CharSequence self, final 
Locale locale) {
+        NumberFormat nf = NumberFormat.getCurrencyInstance(locale);
+        if (nf instanceof DecimalFormat) ((DecimalFormat) 
nf).setParseBigDecimal(true);
+        try {
+            return nf.parse(self.toString());
+        } catch (ParseException e) {
+            throw new NumberFormatException("Unparseable currency: \"" + self 
+ "\"");
+        }
+    }
+
+    /**
+     * Parses a CharSequence into a Number using the percent format of the 
default locale.
+     * The percent value is unscaled (divided by 100).
+     *
+     * @param self a CharSequence
+     * @return the parsed Number
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toPercentNumber(final CharSequence self) {
+        return toPercentNumber(self, Locale.getDefault());
+    }
+
+    /**
+     * Parses a CharSequence into a Number using the percent format of the 
given locale.
+     * The percent value is unscaled (divided by 100).
+     * <pre class="groovyTestCase">
+     * assert '50%'.toPercentNumber(Locale.US) == 0.5
+     * </pre>
+     *
+     * @param self   a CharSequence
+     * @param locale the locale defining the percent format
+     * @return the parsed Number
+     * @throws NumberFormatException if the CharSequence cannot be parsed
+     *
+     * @since 6.0.0
+     */
+    public static Number toPercentNumber(final CharSequence self, final Locale 
locale) {
+        try {
+            return 
NumberFormat.getPercentInstance(locale).parse(self.toString());
+        } catch (ParseException e) {
+            throw new NumberFormatException("Unparseable percent: \"" + self + 
"\"");
+        }
+    }

Review Comment:
   toPercentNumber currently uses NumberFormat.parse(String), which can accept 
a valid percent prefix and ignore trailing junk. If the intent is to fail when 
the whole CharSequence isn't a valid percent, consider trimming and using 
ParsePosition to ensure the entire input is consumed before returning the 
parsed value.





> Add locale-aware number, currency and percent formatting/parsing to the GDK
> ---------------------------------------------------------------------------
>
>                 Key: GROOVY-12147
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12147
>             Project: Groovy
>          Issue Type: Improvement
>          Components: groovy-jdk
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>
> h2. Summary
> Groovy can already format and parse numbers, but only in a locale-invariant 
> way. {{String.format(locale, ...)}} / {{sprintf}} cover locale-aware 
> _grouped/decimal_ output, and {{"1234.50".toBigDecimal()}} covers _canonical_ 
> parsing — but there is no idiomatic way to:
> * format a number as *locale currency* ({{"€1.234,50"}}) or *locale percent* 
> ({{"94,5 %"}}), or
> * parse a *locale-formatted* number, currency or percent string back to a 
> {{Number}}.
> These are exactly the cases {{java.util.Formatter}} ({{%f}}, {{%%}}) 
> structurally cannot express (no currency conversion; {{%%}} is a literal 
> sign, so no ÷100 scaling and wrong symbol placement in locales such as 
> Turkish {{%94,5}} or German {{94,5 %}}). This issue adds a small set of GDK 
> extension methods that wrap {{java.text.NumberFormat}} to close that gap.
> h2. Motivation
> * {{StringGroovyMethods}} conversions ({{toInteger}}, {{toDouble}}, 
> {{toBigDecimal}}, {{isNumber}}, …) are locale-invariant by construction 
> ({{new BigDecimal(str)}} / {{Double.valueOf(str)}}) — they reject grouping 
> separators, foreign decimal marks, currency symbols and percent signs.
> * {{sprintf}}/{{Formatter}} handles grouped/decimal numbers and localized 
> {{%t}} dates, so no new date/time or {{sprintf}} surface is needed.
> * The only genuinely missing capability is currency/percent formatting and 
> locale-aware parsing — a handful of thin {{NumberFormat}} wrappers.
> h2. Proposed API
> *Formatting* — extension methods on {{Number}} (in {{DefaultGroovyMethods}}):
> {code:java}
> public static String toCurrencyString(Number self)
> public static String toCurrencyString(Number self, Locale locale)
> public static String toPercentString(Number self)
> public static String toPercentString(Number self, Locale locale)
> {code}
> *Parsing* — extension methods on {{CharSequence}} (in 
> {{StringGroovyMethods}}, alongside {{toBigDecimal}}):
> {code:java}
> public static Number toNumber(CharSequence self, Locale locale)
> public static Number toCurrencyNumber(CharSequence self)
> public static Number toCurrencyNumber(CharSequence self, Locale locale)
> public static Number toPercentNumber(CharSequence self)
> public static Number toPercentNumber(CharSequence self, Locale locale)
> {code}
> h2. Reference implementation
> {code:java}
> // --- DefaultGroovyMethods (Number) ---
> public static String toCurrencyString(Number self) {
>     return NumberFormat.getCurrencyInstance().format(self);
> }
> public static String toCurrencyString(Number self, Locale locale) {
>     return NumberFormat.getCurrencyInstance(locale).format(self);
> }
> public static String toPercentString(Number self) {
>     return NumberFormat.getPercentInstance().format(self);
> }
> public static String toPercentString(Number self, Locale locale) {
>     return NumberFormat.getPercentInstance(locale).format(self);
> }
> // --- StringGroovyMethods (CharSequence) ---
> public static Number toNumber(CharSequence self, Locale locale) {
>     try {
>         return NumberFormat.getNumberInstance(locale).parse(self.toString());
>     } catch (ParseException e) {
>         throw new NumberFormatException("Unparseable number: \"" + self + 
> "\"");
>     }
> }
> public static Number toCurrencyNumber(CharSequence self) {
>     return toCurrencyNumber(self, Locale.getDefault());
> }
> public static Number toCurrencyNumber(CharSequence self, Locale locale) {
>     NumberFormat nf = NumberFormat.getCurrencyInstance(locale);
>     if (nf instanceof DecimalFormat) ((DecimalFormat) 
> nf).setParseBigDecimal(true); // exact money
>     try {
>         return nf.parse(self.toString());
>     } catch (ParseException e) {
>         throw new NumberFormatException("Unparseable currency: \"" + self + 
> "\"");
>     }
> }
> public static Number toPercentNumber(CharSequence self) {
>     return toPercentNumber(self, Locale.getDefault());
> }
> public static Number toPercentNumber(CharSequence self, Locale locale) {
>     try {
>         return 
> NumberFormat.getPercentInstance(locale).parse(self.toString()); // unscales 
> /100
>     } catch (ParseException e) {
>         throw new NumberFormatException("Unparseable percent: \"" + self + 
> "\"");
>     }
> }
> {code}
> h2. Design notes
> * *Exception convention:* {{NumberFormat.parse}} throws a checked 
> {{java.text.ParseException}}; the existing conversion family throws unchecked 
> {{NumberFormatException}} (e.g. {{new BigDecimal(str)}}). The parse methods 
> catch and rethrow as {{NumberFormatException}} to stay consistent. No cause 
> is chained, matching the current family; message style mirrors the JDK's 
> {{For input string}} form without relying on the non-public 
> {{NumberFormatException.forInputString}}.
> * *Money precision:* currency parsing sets {{setParseBigDecimal(true)}} so 
> amounts come back as exact {{BigDecimal}} rather than lossy {{Double}}. The 
> cast is {{instanceof}}-guarded because {{getCurrencyInstance}} is only 
> contractually a {{NumberFormat}}.
> * *Lenient vs strict:* unlike {{toBigDecimal}} (strict, whole-string), 
> {{NumberFormat.parse}} is lenient (leading numeric prefix, grouping-aware). 
> This is intentional and another reason not to retrofit the existing {{toXxx}} 
> methods.
> * *Naming:* {{to<Flavor><ReturnType>}} — pairs the parse side 
> ({{toCurrencyNumber}}/{{toPercentNumber}}) with the format side 
> ({{toCurrencyString}}/{{toPercentString}}), groups by flavor in IDE 
> autocomplete, and the suffix still names the return type in the spirit of 
> {{toBigDecimal}}. Bare {{toCurrency}} is avoided (reads like it returns a 
> {{java.util.Currency}}).
> h2. Scope
> *In scope:* 4 formatting + 5 parsing methods (9 total), all thin 
> {{java.text.NumberFormat}} wrappers.
> *Explicitly out of scope* (already covered or judged not worth the surface):
> * {{sprintf}}/{{printf}} {{Locale}} overloads on {{Object}} — would add 
> methods to every metaclass; {{String.format(locale, ...)}} already covers 
> grouped/decimal output.
> * {{java.time}} {{format}}/{{parse}} {{Locale}} variants — {{%t}} + 
> {{String.format}} cover the common cases.
> * A locale-invariant {{parseNumber}} — redundant with 
> {{toBigDecimal}}/{{toDouble}}.
> h2. Example
> {code:groovy}
> import java.util.Locale
> def de = Locale.GERMANY
> assert 1234.5.toCurrencyString(de)      == '1.234,50 €'
> assert 0.945.toPercentString(de)        == '94,5 %'
> assert '1.234,50 €'.toCurrencyNumber(de) == 1234.50G   // BigDecimal
> assert '94,5 %'.toPercentNumber(de)      == 0.945
> assert '1.234,50'.toNumber(de)           == 1234.5
> {code}
> h2. Tests
> * Round-trip format→parse for representative locales (US, Germany, France, 
> Turkey — Turkey to cover leading {{%}} placement).
> * Currency parse returns {{BigDecimal}}.
> * Percent parse applies ÷100 scaling.
> * Malformed input throws {{NumberFormatException}}.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to